From 8740c4971dc6d39f7dab856c8865b423490cb6c0 Mon Sep 17 00:00:00 2001 From: Sylvain Date: Fri, 17 Feb 2023 15:35:06 +0100 Subject: [PATCH] (bug) cannot cancel a subscription after offering free days --- CHANGELOG.md | 1 + .../api/slots_reservations_controller.rb | 2 +- .../api/subscriptions_controller.rb | 2 +- app/models/avoir.rb | 2 +- app/models/reservation.rb | 2 +- app/models/subscription.rb | 23 +------ app/services/payment_schedule_service.rb | 2 +- app/services/subscriptions/expire_service.rb | 38 +++++++++++ .../extension_after_reservation.rb} | 4 +- test/integration/subscriptions/cancel_test.rb | 68 +++++++++++++++++++ ...iption_extension_after_reservation_test.rb | 12 ++-- 11 files changed, 122 insertions(+), 34 deletions(-) create mode 100644 app/services/subscriptions/expire_service.rb rename app/services/{subscription_extension_after_reservation.rb => subscriptions/extension_after_reservation.rb} (86%) create mode 100644 test/integration/subscriptions/cancel_test.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index bb04378b4..caeefa934 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - Filter projects by status - Maximum validity period for trainings authorizations - Automatically cancel trainings with insufficient attendees +- Fix a bug: cannot cancel a subscription after offering free days - Fix a bug: event image updates are not reflected unless the browser's cache is purged - Fix a bug: schedules jobs are not launched at the right time - Fix a bug: unable to update the title of a training diff --git a/app/controllers/api/slots_reservations_controller.rb b/app/controllers/api/slots_reservations_controller.rb index 18a1534af..9ee95cbca 100644 --- a/app/controllers/api/slots_reservations_controller.rb +++ b/app/controllers/api/slots_reservations_controller.rb @@ -11,7 +11,7 @@ class API::SlotsReservationsController < API::ApiController def update authorize @slot_reservation if @slot_reservation.update(slot_params) - SubscriptionExtensionAfterReservation.new(@slot_reservation.reservation).extend_subscription_if_eligible + Subscriptions::ExtensionAfterReservation.new(@slot_reservation.reservation).extend_subscription_if_eligible render :show, status: :ok, location: @slot_reservation else render json: @slot_reservation.errors, status: :unprocessable_entity diff --git a/app/controllers/api/subscriptions_controller.rb b/app/controllers/api/subscriptions_controller.rb index 9414b57a2..05cf0de53 100644 --- a/app/controllers/api/subscriptions_controller.rb +++ b/app/controllers/api/subscriptions_controller.rb @@ -15,7 +15,7 @@ class API::SubscriptionsController < API::ApiController def cancel authorize @subscription - if @subscription.expire(Time.current) + if @subscription.expire render :show, status: :ok, location: @subscription else render json: { error: 'already expired' }, status: :unprocessable_entity diff --git a/app/models/avoir.rb b/app/models/avoir.rb index 09e718e3b..0f28ce5dc 100644 --- a/app/models/avoir.rb +++ b/app/models/avoir.rb @@ -17,7 +17,7 @@ class Avoir < Invoice end def expire_subscription - user.subscription.expire(Time.current) + user.subscription.expire end private diff --git a/app/models/reservation.rb b/app/models/reservation.rb index d75066ba5..c3a07e700 100644 --- a/app/models/reservation.rb +++ b/app/models/reservation.rb @@ -121,7 +121,7 @@ class Reservation < ApplicationRecord end def extend_subscription - SubscriptionExtensionAfterReservation.new(self).extend_subscription_if_eligible + Subscriptions::ExtensionAfterReservation.new(self).extend_subscription_if_eligible end def notify_member_create_reservation diff --git a/app/models/subscription.rb b/app/models/subscription.rb index f9667828e..f6a033f6c 100644 --- a/app/models/subscription.rb +++ b/app/models/subscription.rb @@ -29,15 +29,8 @@ class Subscription < ApplicationRecord generate_invoice(operator_profile_id).save end - def expire(time) - if expired? - false - else - update_columns(expiration_date: time, canceled_at: time) # rubocop:disable Rails/SkipsModelValidations - notify_admin_subscription_canceled - notify_member_subscription_canceled - true - end + def expire + Subscriptions::ExpireService.call(self) end def expired? @@ -78,18 +71,6 @@ class Subscription < ApplicationRecord attached_object: self end - def notify_admin_subscription_canceled - NotificationCenter.call type: 'notify_admin_subscription_canceled', - receiver: User.admins_and_managers, - attached_object: self - end - - def notify_member_subscription_canceled - NotificationCenter.call type: 'notify_member_subscription_canceled', - receiver: user, - attached_object: self - end - def notify_partner_subscribed_plan NotificationCenter.call type: 'notify_partner_subscribed_plan', receiver: plan.partners, diff --git a/app/services/payment_schedule_service.rb b/app/services/payment_schedule_service.rb index b27fd977d..dea50390e 100644 --- a/app/services/payment_schedule_service.rb +++ b/app/services/payment_schedule_service.rb @@ -170,7 +170,7 @@ class PaymentScheduleService end # cancel subscription subscription = payment_schedule.payment_schedule_objects.find { |pso| pso.object_type == Subscription.name }.subscription - subscription.expire(Time.current) + subscription.expire subscription.canceled_at end diff --git a/app/services/subscriptions/expire_service.rb b/app/services/subscriptions/expire_service.rb new file mode 100644 index 000000000..7eaa1adbf --- /dev/null +++ b/app/services/subscriptions/expire_service.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +# Expire the given subscription +class Subscriptions::ExpireService + class << self + # @param subscription [Subscription] + def call(subscription) + expiration = Time.current + if subscription.expired? + false + else + subscription.update_columns(expiration_date: expiration, canceled_at: expiration) # rubocop:disable Rails/SkipsModelValidations + subscription.offer_days.find_each do |od| + od.update(start_at: expiration, end_at: expiration) + end + notify_admin_subscription_canceled(subscription) + notify_member_subscription_canceled(subscription) + true + end + end + + private + + # @param subscription [Subscription] + def notify_admin_subscription_canceled(subscription) + NotificationCenter.call type: 'notify_admin_subscription_canceled', + receiver: User.admins_and_managers, + attached_object: subscription + end + + # @param subscription [Subscription] + def notify_member_subscription_canceled(subscription) + NotificationCenter.call type: 'notify_member_subscription_canceled', + receiver: subscription.user, + attached_object: subscription + end + end +end diff --git a/app/services/subscription_extension_after_reservation.rb b/app/services/subscriptions/extension_after_reservation.rb similarity index 86% rename from app/services/subscription_extension_after_reservation.rb rename to app/services/subscriptions/extension_after_reservation.rb index 5c2d46171..b6d9f77ab 100644 --- a/app/services/subscription_extension_after_reservation.rb +++ b/app/services/subscriptions/extension_after_reservation.rb @@ -2,7 +2,7 @@ # Extend the user's current subscription after his first training reservation if # he subscribed to a rolling plan -class SubscriptionExtensionAfterReservation +class Subscriptions::ExtensionAfterReservation attr_accessor :user, :reservation def initialize(reservation) @@ -25,7 +25,7 @@ class SubscriptionExtensionAfterReservation end def extend_subscription - user.subscription.update_columns( + user.subscription.update_columns( # rubocop:disable Rails/SkipsModelValidations expiration_date: reservation.slots_reservations.first.slot.start_at + user.subscribed_plan.duration ) end diff --git a/test/integration/subscriptions/cancel_test.rb b/test/integration/subscriptions/cancel_test.rb new file mode 100644 index 000000000..46f657699 --- /dev/null +++ b/test/integration/subscriptions/cancel_test.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require 'test_helper' + +module Subscriptions; end + +class Subscriptions::CancelTest < ActionDispatch::IntegrationTest + setup do + @admin = User.find_by(username: 'admin') + login_as(@admin, scope: :user) + end + + test 'admin cancel a subscription for a user' do + subscription = Subscription.find(1) + + patch "/api/subscriptions/#{subscription.id}/cancel", headers: default_headers + + # Check response format & status + assert_response :success + assert_equal Mime[:json], response.content_type + + # Check the subscription was canceled + subscription.reload + assert subscription.expiration_date < Time.current + assert subscription.canceled_at < Time.current + assert subscription.expired_at < Time.current + assert subscription.expired? + assert_nil subscription.user.subscribed_plan + + # Notifications + notifications = Notification.where(notification_type: NotificationType.find_by(name: 'notify_admin_subscription_canceled'), + attached_object: subscription) + notified_users_ids = notifications.map(&:receiver_id) + assert_not_empty notifications + assert(User.admins.map(&:id).all? { |admin| notified_users_ids.include?(admin) }) + + user_notification = Notification.where(notification_type: NotificationType.find_by(name: 'notify_member_subscription_canceled'), + attached_object: subscription) + assert_equal 1, user_notification.count + end + + test 'admin offer free days then cancel the subscription' do + subscription = Subscription.find(1) + new_date = 1.month.from_now.utc + + post '/api/local_payment/confirm_payment', + params: { + customer_id: subscription.user.id, + items: [{ free_extension: { end_at: new_date.strftime('%Y-%m-%d %H:%M:%S.%9N Z') } }] + }.to_json, headers: default_headers + + assert_response :success + + patch "/api/subscriptions/#{subscription.id}/cancel", headers: default_headers + + # Check response format & status + assert_response :success + assert_equal Mime[:json], response.content_type + + # Check the subscription was canceled + subscription.reload + assert subscription.expiration_date < Time.current + assert subscription.canceled_at < Time.current + assert subscription.expired_at < Time.current + assert subscription.expired? + assert_nil subscription.user.subscribed_plan + end +end diff --git a/test/services/subscription_extension_after_reservation_test.rb b/test/services/subscription_extension_after_reservation_test.rb index 07059562c..f544527a3 100644 --- a/test/services/subscription_extension_after_reservation_test.rb +++ b/test/services/subscription_extension_after_reservation_test.rb @@ -31,31 +31,31 @@ class SubscriptionExtensionAfterReservationTest < ActiveSupport::TestCase end test 'is eligible for extension because all conditions are met by default (test setup)' do - assert SubscriptionExtensionAfterReservation.new(@reservation_training).eligible_to_extension? + assert Subscriptions::ExtensionAfterReservation.new(@reservation_training).eligible_to_extension? end test 'not eligible if reservable is a machine' do @reservation_machine.save! - refute SubscriptionExtensionAfterReservation.new(@reservation_machine).eligible_to_extension? + assert_not Subscriptions::ExtensionAfterReservation.new(@reservation_machine).eligible_to_extension? end test "not eligible if user doesn't have subscription" do @user.subscriptions.destroy_all - refute SubscriptionExtensionAfterReservation.new(@reservation_training).eligible_to_extension? + assert_not Subscriptions::ExtensionAfterReservation.new(@reservation_training).eligible_to_extension? end test 'not eligible if subscription is expired' do @user.subscription.update!(expiration_date: 10.years.ago) - refute SubscriptionExtensionAfterReservation.new(@reservation_training).eligible_to_extension? + assert_not Subscriptions::ExtensionAfterReservation.new(@reservation_training).eligible_to_extension? end test "not eligible if plan attribute 'is_rolling' is false/nil" do @plan.update!(is_rolling: false) - refute SubscriptionExtensionAfterReservation.new(@reservation_training).eligible_to_extension? + assert_not Subscriptions::ExtensionAfterReservation.new(@reservation_training).eligible_to_extension? end test 'method extend_subscription' do - SubscriptionExtensionAfterReservation.new(@reservation_training).extend_subscription + Subscriptions::ExtensionAfterReservation.new(@reservation_training).extend_subscription assert_equal @reservation_training.slots_reservations.first.slot.start_at + @plan.duration, @user.subscription.expired_at end end