From bef3118649e69184ff9edb3ce7be3d6db67343ba Mon Sep 17 00:00:00 2001 From: Sylvain Date: Thu, 6 Dec 2018 18:26:01 +0100 Subject: [PATCH] [ongoing] refactor user.subscriptions to save history TODO: - refactor subscription.save_with_payment (stripe) - offer free days - extend the subscription - renew a subscription - buy subscription + reservation --- .../api/subscriptions_controller.rb | 62 ++-- app/models/invoice.rb | 2 +- app/models/notification_type.rb | 4 +- app/models/subscription.rb | 287 +++++++++--------- app/models/user.rb | 8 +- ...ubscription_extension_after_reservation.rb | 8 +- app/services/subscriptions/subscribe.rb | 17 ++ app/services/users_credits/manager.rb | 88 +++--- .../subscriptions/renew_as_admin_test.rb | 58 +++- .../subscriptions/renew_as_user_test.rb | 36 ++- test/test_helper.rb | 5 + 11 files changed, 319 insertions(+), 256 deletions(-) create mode 100644 app/services/subscriptions/subscribe.rb diff --git a/app/controllers/api/subscriptions_controller.rb b/app/controllers/api/subscriptions_controller.rb index cd6db0454..fd097fd02 100644 --- a/app/controllers/api/subscriptions_controller.rb +++ b/app/controllers/api/subscriptions_controller.rb @@ -1,7 +1,7 @@ class API::SubscriptionsController < API::ApiController include FablabConfiguration - before_action :set_subscription, only: [:show, :edit, :update, :destroy] + before_action :set_subscription, only: %i[show edit update destroy] before_action :authenticate_user! def show @@ -12,15 +12,12 @@ class API::SubscriptionsController < API::ApiController if fablab_plans_deactivated? head 403 else - if current_user.is_admin? - @subscription = Subscription.find_or_initialize_by(user_id: subscription_params[:user_id]) - @subscription.attributes = subscription_params - is_subscribe = @subscription.save_with_local_payment(true, coupon_params[:coupon_code]) - else - @subscription = Subscription.find_or_initialize_by(user_id: current_user.id) - @subscription.attributes = subscription_params - is_subscribe = @subscription.save_with_payment(true, coupon_params[:coupon_code]) - end + method = current_user.is_admin? ? :local : :stripe + user_id = current_user.is_admin? ? subscription_params[:user_id] : current_user.id + + @subscription = Subscription.new(subscription_params) + is_subscribe = Subscribe.new(method, user_id).pay_and_save(@subscription, coupon_params[:coupon_code], true) + if is_subscribe render :show, status: :created, location: @subscription else @@ -44,31 +41,30 @@ class API::SubscriptionsController < API::ApiController end private - # Use callbacks to share common setup or constraints between actions. - def set_subscription - @subscription = Subscription.find(params[:id]) - end - # Never trust parameters from the scary internet, only allow the white list through. - def subscription_params - params.require(:subscription).permit(:plan_id, :user_id, :card_token) - end + # Use callbacks to share common setup or constraints between actions. + def set_subscription + @subscription = Subscription.find(params[:id]) + end - def coupon_params - params.permit(:coupon_code) - end + # Never trust parameters from the scary internet, only allow the white list through. + def subscription_params + params.require(:subscription).permit(:plan_id, :user_id, :card_token) + end - def subscription_update_params - params.require(:subscription).permit(:expired_at) - end + def coupon_params + params.permit(:coupon_code) + end - # TODO refactor subscriptions logic and move this in model/validator - def valid_card_token?(token) - begin - Stripe::Token.retrieve(token) - rescue Stripe::InvalidRequestError => e - @subscription.errors[:card_token] << e.message - false - end - end + def subscription_update_params + params.require(:subscription).permit(:expired_at) + end + + # TODO refactor subscriptions logic and move this in model/validator + def valid_card_token?(token) + Stripe::Token.retrieve(token) + rescue Stripe::InvalidRequestError => e + @subscription.errors[:card_token] << e.message + false + end end diff --git a/app/models/invoice.rb b/app/models/invoice.rb index 5c3a5f263..3f70920d0 100644 --- a/app/models/invoice.rb +++ b/app/models/invoice.rb @@ -169,7 +169,7 @@ class Invoice < ActiveRecord::Base def is_subscription_invoice? invoice_items.each do |ii| - return true if ii.subscription and !ii.subscription.is_expired? + return true if ii.subscription and !ii.subscription.expired? end false end diff --git a/app/models/notification_type.rb b/app/models/notification_type.rb index 6a43c4819..428bfd164 100644 --- a/app/models/notification_type.rb +++ b/app/models/notification_type.rb @@ -2,7 +2,7 @@ class NotificationType include NotifyWith::NotificationType # DANGER: dont remove a notification type!!! - notification_type_names %w( + notification_type_names %w[ notify_admin_when_project_published notify_project_collaborator_to_valid notify_project_author_when_collaborator_valid @@ -42,5 +42,5 @@ class NotificationType notify_admin_export_complete notify_member_about_coupon notify_member_reservation_reminder - ) + ] end diff --git a/app/models/subscription.rb b/app/models/subscription.rb index dae9d6d4f..9bcc044e7 100644 --- a/app/models/subscription.rb +++ b/app/models/subscription.rb @@ -13,163 +13,150 @@ class Subscription < ActiveRecord::Base attr_accessor :card_token # creation - after_save :notify_member_subscribed_plan, if: :is_new? - after_save :notify_admin_subscribed_plan, if: :is_new? + after_save :notify_member_subscribed_plan, if: :new? + after_save :notify_admin_subscribed_plan, if: :new? after_save :notify_partner_subscribed_plan, if: :of_partner_plan? # Stripe subscription payment # @params [invoice] if true then subscription pay itself, dont pay with reservation # if false then subscription pay with reservation def save_with_payment(invoice = true, coupon_code = nil) - if valid? - begin - customer = Stripe::Customer.retrieve(user.stp_customer_id) - invoice_items = [] + return unless valid? - unless coupon_code.nil? - @coupon = Coupon.find_by(code: coupon_code) - if not @coupon.nil? and @coupon.status(user.id) == 'active' - total = plan.amount + begin + customer = Stripe::Customer.retrieve(user.stp_customer_id) + invoice_items = [] - discount = 0 - if @coupon.type == 'percent_off' - discount = (total * @coupon.percent_off / 100).to_i - elsif @coupon.type == 'amount_off' - discount = @coupon.amount_off - else - raise InvalidCouponError - end + unless coupon_code.nil? + @coupon = Coupon.find_by(code: coupon_code) + raise InvalidCouponError if @coupon.nil? || @coupon.status(user.id) != 'active' - invoice_items << Stripe::InvoiceItem.create( - customer: user.stp_customer_id, - amount: -discount, - currency: Rails.application.secrets.stripe_currency, - description: "coupon #{@coupon.code} - subscription" - ) - else - raise InvalidCouponError - end + total = plan.amount + + discount = 0 + if @coupon.type == 'percent_off' + discount = (total * @coupon.percent_off / 100).to_i + elsif @coupon.type == 'amount_off' + discount = @coupon.amount_off + else + raise InvalidCouponError end - # only add a wallet invoice item if pay subscription - # dont add if pay subscription + reservation - if invoice - @wallet_amount_debit = get_wallet_amount_debit - if @wallet_amount_debit != 0 - invoice_items << Stripe::InvoiceItem.create( - customer: user.stp_customer_id, - amount: -@wallet_amount_debit, - currency: Rails.application.secrets.stripe_currency, - description: "wallet -#{@wallet_amount_debit / 100.0}" - ) - end - end - - new_subscription = customer.subscriptions.create(plan: plan.stp_plan_id, source: card_token) - # very important to set expired_at to nil that can allow method is_new? to return true - # for send the notification - # TODO: Refactoring - update_column(:expired_at, nil) unless new_record? - self.stp_subscription_id = new_subscription.id - self.canceled_at = nil - self.expired_at = Time.at(new_subscription.current_period_end) - save! - - UsersCredits::Manager.new(user: self.user).reset_credits if expired_date_changed - - # generate invoice - stp_invoice = Stripe::Invoice.all(customer: user.stp_customer_id, limit: 1).data.first - if invoice - invoc = generate_invoice(stp_invoice.id, coupon_code) - # debit wallet - wallet_transaction = debit_user_wallet - if wallet_transaction - invoc.wallet_amount = @wallet_amount_debit - invoc.wallet_transaction_id = wallet_transaction.id - end - invoc.save - end - # cancel subscription after create - cancel - return true - rescue Stripe::CardError => card_error - clear_wallet_and_goupon_invoice_items(invoice_items) - logger.error card_error - errors[:card] << card_error.message - return false - rescue Stripe::InvalidRequestError => e - clear_wallet_and_goupon_invoice_items(invoice_items) - # Invalid parameters were supplied to Stripe's API - logger.error e - errors[:payment] << e.message - return false - rescue Stripe::AuthenticationError => e - clear_wallet_and_goupon_invoice_items(invoice_items) - # Authentication with Stripe's API failed - # (maybe you changed API keys recently) - logger.error e - errors[:payment] << e.message - return false - rescue Stripe::APIConnectionError => e - clear_wallet_and_goupon_invoice_items(invoice_items) - # Network communication with Stripe failed - logger.error e - errors[:payment] << e.message - return false - rescue Stripe::StripeError => e - clear_wallet_and_goupon_invoice_items(invoice_items) - # Display a very generic error to the user, and maybe send - # yourself an email - logger.error e - errors[:payment] << e.message - return false - rescue => e - clear_wallet_and_goupon_invoice_items(invoice_items) - # Something else happened, completely unrelated to Stripe - logger.error e - errors[:payment] << e.message - return false + invoice_items << Stripe::InvoiceItem.create( + customer: user.stp_customer_id, + amount: -discount, + currency: Rails.application.secrets.stripe_currency, + description: "coupon #{@coupon.code} - subscription" + ) end - end - end - # @params [invoice] if true then subscription pay itself, dont pay with reservation - # if false then subscription pay with reservation - def save_with_local_payment(invoice = true, coupon_code = nil) - if valid? + # only add a wallet invoice item if pay subscription + # dont add if pay subscription + reservation + if invoice + @wallet_amount_debit = get_wallet_amount_debit + if @wallet_amount_debit != 0 + invoice_items << Stripe::InvoiceItem.create( + customer: user.stp_customer_id, + amount: -@wallet_amount_debit, + currency: Rails.application.secrets.stripe_currency, + description: "wallet -#{@wallet_amount_debit / 100.0}" + ) + end + end + + new_subscription = customer.subscriptions.create(plan: plan.stp_plan_id, source: card_token) # very important to set expired_at to nil that can allow method is_new? to return true # for send the notification # TODO: Refactoring update_column(:expired_at, nil) unless new_record? - self.stp_subscription_id = nil + self.stp_subscription_id = new_subscription.id self.canceled_at = nil - set_expired_at - if save - UsersCredits::Manager.new(user: self.user).reset_credits if expired_date_changed - if invoice - @wallet_amount_debit = get_wallet_amount_debit + self.expired_at = Time.at(new_subscription.current_period_end) + save! - # debit wallet - wallet_transaction = debit_user_wallet + UsersCredits::Manager.new(user: self.user).reset_credits if expired_date_changed - if !self.user.invoicing_disabled? - invoc = generate_invoice(nil, coupon_code) - if wallet_transaction - invoc.wallet_amount = @wallet_amount_debit - invoc.wallet_transaction_id = wallet_transaction.id - end - invoc.save - end + # generate invoice + stp_invoice = Stripe::Invoice.all(customer: user.stp_customer_id, limit: 1).data.first + if invoice + invoc = generate_invoice(stp_invoice.id, coupon_code) + # debit wallet + wallet_transaction = debit_user_wallet + if wallet_transaction + invoc.wallet_amount = @wallet_amount_debit + invoc.wallet_transaction_id = wallet_transaction.id end - return true - else - return false + invoc.save end - else + # cancel subscription after create + cancel + return true + rescue Stripe::CardError => card_error + clear_wallet_and_goupon_invoice_items(invoice_items) + logger.error card_error + errors[:card] << card_error.message + return false + rescue Stripe::InvalidRequestError => e + clear_wallet_and_goupon_invoice_items(invoice_items) + # Invalid parameters were supplied to Stripe's API + logger.error e + errors[:payment] << e.message + return false + rescue Stripe::AuthenticationError => e + clear_wallet_and_goupon_invoice_items(invoice_items) + # Authentication with Stripe's API failed + # (maybe you changed API keys recently) + logger.error e + errors[:payment] << e.message + return false + rescue Stripe::APIConnectionError => e + clear_wallet_and_goupon_invoice_items(invoice_items) + # Network communication with Stripe failed + logger.error e + errors[:payment] << e.message + return false + rescue Stripe::StripeError => e + clear_wallet_and_goupon_invoice_items(invoice_items) + # Display a very generic error to the user, and maybe send + # yourself an email + logger.error e + errors[:payment] << e.message + return false + rescue => e + clear_wallet_and_goupon_invoice_items(invoice_items) + # Something else happened, completely unrelated to Stripe + logger.error e + errors[:payment] << e.message return false end end + # @params [invoice] if true then only the subscription is payed, without reservation + # if false then the subscription is payed with reservation + def save_with_local_payment(invoice = true, coupon_code = nil) + return false unless valid? + + return false unless save + + UsersCredits::Manager.new(user: user).reset_credits if expired_date_changed + if invoice + @wallet_amount_debit = get_wallet_amount_debit + + # debit wallet + wallet_transaction = debit_user_wallet + + unless user.invoicing_disabled? + invoc = generate_invoice(nil, coupon_code) + if wallet_transaction + invoc.wallet_amount = @wallet_amount_debit + invoc.wallet_transaction_id = wallet_transaction.id + end + invoc.save + end + end + true + end + def generate_invoice(stp_invoice_id = nil, coupon_code = nil) coupon_id = nil total = plan.amount @@ -200,11 +187,11 @@ class Subscription < ActiveRecord::Base end def cancel - if stp_subscription_id.present? - stp_subscription = stripe_subscription - stp_subscription.delete(at_period_end: true) - update_columns(canceled_at: Time.now) - end + return unless stp_subscription_id.present? + + stp_subscription = stripe_subscription + stp_subscription.delete(at_period_end: true) + update_columns(canceled_at: Time.now) end def stripe_subscription @@ -212,7 +199,7 @@ class Subscription < ActiveRecord::Base end def expire(time) - if !is_expired? + if !expired? update_columns(expired_at: time, canceled_at: time) notify_admin_subscription_canceled notify_member_subscription_canceled @@ -222,7 +209,7 @@ class Subscription < ActiveRecord::Base end end - def is_expired? + def expired? expired_at <= Time.now end @@ -231,14 +218,15 @@ class Subscription < ActiveRecord::Base self.expired_at = expired_at if save - UsersCredits::Manager.new(user: self.user).reset_credits if !free_days + UsersCredits::Manager.new(user: user).reset_credits unless free_days notify_subscription_extended(free_days) return true end - return false + false end private + def notify_member_subscribed_plan NotificationCenter.call type: 'notify_member_subscribed_plan', receiver: user, @@ -289,8 +277,9 @@ class Subscription < ActiveRecord::Base end def expired_date_changed - p_value = self.previous_changes[:expired_at][0] + p_value = previous_changes[:expired_at][0] return true if p_value.nil? + p_value.to_date != expired_at.to_date and expired_at > p_value end @@ -298,7 +287,7 @@ class Subscription < ActiveRecord::Base # !expired_at_was.nil? and expired_at_changed? # end - def is_new? + def new? expired_at_was.nil? end @@ -308,18 +297,16 @@ class Subscription < ActiveRecord::Base def get_wallet_amount_debit total = plan.amount - if @coupon - total = CouponService.new.apply(total, @coupon, user.id) - end + total = CouponService.new.apply(total, @coupon, user.id) if @coupon wallet_amount = (user.wallet.amount * 100).to_i - return wallet_amount >= total ? total : wallet_amount + wallet_amount >= total ? total : wallet_amount end def debit_user_wallet - if @wallet_amount_debit.present? and @wallet_amount_debit != 0 - amount = @wallet_amount_debit / 100.0 - return WalletService.new(user: user, wallet: user.wallet).debit(amount, self) - end + return unless @wallet_amount_debit.present? || @wallet_amount_debit.zero? + + amount = @wallet_amount_debit / 100.0 + WalletService.new(user: user, wallet: user.wallet).debit(amount, self) end def clear_wallet_and_goupon_invoice_items(invoice_items) diff --git a/app/models/user.rb b/app/models/user.rb index c8ec16188..8d7ccc787 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -220,7 +220,7 @@ class User < ActiveRecord::Base ## used to allow the migration of existing users between authentication providers def generate_auth_migration_token - update_attributes(:auth_token => Devise.friendly_token) + update_attributes(auth_token: Devise.friendly_token) end ## link the current user to the given provider (omniauth attributes hash) @@ -241,7 +241,7 @@ class User < ActiveRecord::Base ## Merge the provided User's SSO details into the current user and drop the provided user to ensure the unity ## @param sso_user {User} the provided user will be DELETED after the merge was successful def merge_from_sso(sso_user) - # update the attibutes to link the account to the sso account + # update the attributes to link the account to the sso account self.provider = sso_user.provider self.uid = sso_user.uid @@ -264,7 +264,7 @@ class User < ActiveRecord::Base set_data_from_sso_mapping(field, value) unless field == 'user.email' && value.end_with?('-duplicate') end - # run the account transfert in an SQL transaction to ensure data integrity + # run the account transfer in an SQL transaction to ensure data integrity User.transaction do # remove the temporary account sso_user.destroy @@ -319,7 +319,7 @@ class User < ActiveRecord::Base end def notify_admin_when_user_is_created - if need_completion? and not provider.nil? + if need_completion? && !provider.nil? NotificationCenter.call type: 'notify_admin_when_user_is_imported', receiver: User.admins, attached_object: self diff --git a/app/services/subscription_extension_after_reservation.rb b/app/services/subscription_extension_after_reservation.rb index a11490d77..e5d5710cc 100644 --- a/app/services/subscription_extension_after_reservation.rb +++ b/app/services/subscription_extension_after_reservation.rb @@ -2,7 +2,8 @@ class SubscriptionExtensionAfterReservation attr_accessor :user, :reservation def initialize(reservation) - @user, @reservation = reservation.user, reservation + @user = reservation.user + @reservation = reservation end def extend_subscription_if_eligible @@ -13,9 +14,10 @@ class SubscriptionExtensionAfterReservation return false unless reservation.reservable_type == 'Training' return false if user.reservations.where(reservable_type: 'Training').count != 1 return false unless user.subscription - return false if user.subscription.is_expired? + return false if user.subscription.expired? return false unless user.subscribed_plan.is_rolling - return true + + true end def extend_subscription diff --git a/app/services/subscriptions/subscribe.rb b/app/services/subscriptions/subscribe.rb new file mode 100644 index 000000000..43924f192 --- /dev/null +++ b/app/services/subscriptions/subscribe.rb @@ -0,0 +1,17 @@ +class Subscribe + attr_accessor :payment_method, :user_id + + def initialize(payment_method, user_id) + @payment_method = payment_method + @user_id = user_id + end + + def pay_and_save(subscription, coupon, invoice) + subscription.user_id = user_id + if payment_method == :local + subscription.save_with_local_payment(invoice, coupon) + elsif payment_method == :stripe + subscription.save_with_payment(invoice, coupon) + end + end +end diff --git a/app/services/users_credits/manager.rb b/app/services/users_credits/manager.rb index befeee260..edc89dd62 100644 --- a/app/services/users_credits/manager.rb +++ b/app/services/users_credits/manager.rb @@ -1,7 +1,7 @@ require 'forwardable' module UsersCredits - class AlreadyUpdatedError < StandardError;end; + class AlreadyUpdatedError < StandardError; end class Manager extend Forwardable @@ -31,7 +31,8 @@ module UsersCredits end module Managers - class User # that class is responsible for reseting users_credits of a user + # that class is responsible for resetting users_credits of a user + class User attr_reader :user def initialize(user) @@ -46,7 +47,8 @@ module UsersCredits class Reservation attr_reader :reservation - def initialize(reservation, plan) # a plan can be passed to do a simulation (if user didn't have a subscription YET) + # a plan can be passed to do a simulation (if user didn't have a subscription YET) + def initialize(reservation, plan) @reservation = reservation @already_updated = false @plan = plan @@ -71,8 +73,10 @@ module UsersCredits private_constant :Reservation - class Machine < Reservation # that class is responsible for knowing how to update users_credit of a given user for a given reservation - def will_use_credits? # to known if a credit will be used in the context of the given reservation + # that class is responsible for knowing how to update users_credit of a given user for a given reservation + class Machine < Reservation + # to known if a credit will be used in the context of the given reservation + def will_use_credits? _will_use_credits?[0] end @@ -97,28 +101,30 @@ module UsersCredits end private - def _will_use_credits? - return false, 0 unless plan - if machine_credit = plan.machine_credits.find_by(creditable_id: reservation.reservable_id) - users_credit = user.users_credits.find_by(credit_id: machine_credit.id) - already_used_hours = users_credit ? users_credit.hours_used : 0 + def _will_use_credits? + return false, 0 unless plan - remaining_hours = machine_credit.hours - already_used_hours + if machine_credit = plan.machine_credits.find_by(creditable_id: reservation.reservable_id) + users_credit = user.users_credits.find_by(credit_id: machine_credit.id) + already_used_hours = users_credit ? users_credit.hours_used : 0 - free_hours_count = [remaining_hours, reservation.slots.size].min + remaining_hours = machine_credit.hours - already_used_hours - if free_hours_count > 0 - return true, free_hours_count, machine_credit - else - return false, free_hours_count, machine_credit - end + free_hours_count = [remaining_hours, reservation.slots.size].min + + if free_hours_count.positive? + return true, free_hours_count, machine_credit + else + return false, free_hours_count, machine_credit end - return false, 0 end + return false, 0 + end end - class Training < Reservation # same as class Machine but for Training reservation + # same as class Machine but for Training reservation + class Training < Reservation def will_use_credits? _will_use_credits?[0] end @@ -132,18 +138,19 @@ module UsersCredits end private - def _will_use_credits? - return false, nil unless plan - # if there is a training_credit defined for this plan and this training - if training_credit = plan.training_credits.find_by(creditable_id: reservation.reservable_id) - # if user has not used all the plan credits - if user.training_credits.where(plan: plan).count < plan.training_credit_nb - return true, training_credit - end + def _will_use_credits? + return false, nil unless plan + + # if there is a training_credit defined for this plan and this training + if training_credit = plan.training_credits.find_by(creditable_id: reservation.reservable_id) + # if user has not used all the plan credits + if user.training_credits.where(plan: plan).count < plan.training_credit_nb + return true, training_credit end - return false, nil end + return false, nil + end end class Event < Reservation @@ -151,12 +158,12 @@ module UsersCredits false end - def update_credits - end + def update_credits; end end class Space < Reservation - def will_use_credits? # to known if a credit will be used in the context of the given reservation + # to known if a credit will be used in the context of the given reservation + def will_use_credits? _will_use_credits?[0] end @@ -168,19 +175,20 @@ module UsersCredits super will_use_credits, free_hours_count, space_credit = _will_use_credits? - if will_use_credits - users_credit = user.users_credits.find_or_initialize_by(credit_id: space_credit.id) + return unless will_use_credits - if users_credit.new_record? - users_credit.hours_used = free_hours_count - else - users_credit.hours_used += free_hours_count - end - users_credit.save! + users_credit = user.users_credits.find_or_initialize_by(credit_id: space_credit.id) + + if users_credit.new_record? + users_credit.hours_used = free_hours_count + else + users_credit.hours_used += free_hours_count end + users_credit.save! end private + def _will_use_credits? return false, 0 unless plan @@ -192,7 +200,7 @@ module UsersCredits free_hours_count = [remaining_hours, reservation.slots.size].min - if free_hours_count > 0 + if free_hours_count.positive? return true, free_hours_count, space_credit else return false, free_hours_count, space_credit diff --git a/test/integration/subscriptions/renew_as_admin_test.rb b/test/integration/subscriptions/renew_as_admin_test.rb index e5006036f..228b92dd9 100644 --- a/test/integration/subscriptions/renew_as_admin_test.rb +++ b/test/integration/subscriptions/renew_as_admin_test.rb @@ -10,13 +10,13 @@ class Subscriptions::RenewAsAdminTest < ActionDispatch::IntegrationTest user = User.find_by(username: 'kdumas') plan = Plan.find_by(base_name: 'Mensuel tarif réduit') - VCR.use_cassette("subscriptions_admin_renew_success") do + VCR.use_cassette('subscriptions_admin_renew_success') do post '/api/subscriptions', { - subscription: { - plan_id: plan.id, - user_id: user.id - } + subscription: { + plan_id: plan.id, + user_id: user.id + } }.to_json, default_headers end @@ -39,10 +39,16 @@ class Subscriptions::RenewAsAdminTest < ActionDispatch::IntegrationTest # Check that the user benefit from prices of his plan printer = Machine.find_by(slug: 'imprimante-3d') - assert_equal 10, (printer.prices.find_by(group_id: user.group_id, plan_id: user.subscription.plan_id).amount / 100), 'machine hourly price does not match' + assert_equal 10, + (printer.prices.find_by(group_id: user.group_id, plan_id: user.subscription.plan_id).amount / 100), + 'machine hourly price does not match' # Check notification was sent to the user - notification = Notification.find_by(notification_type_id: NotificationType.find_by_name('notify_member_subscribed_plan'), attached_object_type: 'Subscription', attached_object_id: subscription[:id]) + notification = Notification.find_by( + notification_type_id: NotificationType.find_by_name('notify_member_subscribed_plan'), + attached_object_type: 'Subscription', + attached_object_id: subscription[:id] + ) assert_not_nil notification, 'user notification was not created' assert_equal user.id, notification.receiver_id, 'wrong user notified' @@ -50,7 +56,41 @@ class Subscriptions::RenewAsAdminTest < ActionDispatch::IntegrationTest invoice = Invoice.find_by(invoiced_type: 'Subscription', invoiced_id: subscription[:id]) assert_invoice_pdf invoice assert_equal plan.amount, invoice.total, 'Invoice total price does not match the bought subscription' - end -end \ No newline at end of file + test 'admin successfully offer free days' do + user = User.find_by(username: 'pdurand') + subscription = user.subscription + new_date = (1.month.from_now - 4.days).utc + + VCR.use_cassette('subscriptions_admin_offer_free_days') do + put "/api/subscriptions/#{subscription.id}", + { + subscription: { + expired_at: new_date.strftime('%Y-%m-%d %H:%M:%S.%9N Z'), + free: true + } + }.to_json, default_headers + end + + # Check response format & status + assert_equal 200, response.status, response.body + assert_equal Mime::JSON, response.content_type + + # Check that the subscribed plan was not altered + res_subscription = json_response(response.body) + assert_equal subscription.id, res_subscription[:id], 'subscription id has changed' + assert_equal subscription.plan_id, res_subscription[:plan_id], 'subscribed plan does not match' + assert_dates_equal new_date, res_subscription[:expired_at], 'subscription end date was not updated' + + # Check notification was sent to the user + notification = Notification.find_by( + notification_type_id: NotificationType.find_by_name('notify_member_subscription_extended'), + attached_object_type: 'Subscription', + attached_object_id: subscription[:id] + ) + assert_not_nil notification, 'user notification was not created' + assert_equal user.id, notification.receiver_id, 'wrong user notified' + end + +end diff --git a/test/integration/subscriptions/renew_as_user_test.rb b/test/integration/subscriptions/renew_as_user_test.rb index 8444102eb..89356107d 100644 --- a/test/integration/subscriptions/renew_as_user_test.rb +++ b/test/integration/subscriptions/renew_as_user_test.rb @@ -9,7 +9,7 @@ class Subscriptions::RenewAsUserTest < ActionDispatch::IntegrationTest test 'user successfully renew a subscription after it has ended' do plan = Plan.find_by(group_id: @user.group.id, type: 'Plan', base_name: 'Mensuel') - VCR.use_cassette("subscriptions_user_renew_success", :erb => true) do + VCR.use_cassette('subscriptions_user_renew_success', erb: true) do post '/api/subscriptions', { subscription: { @@ -21,7 +21,7 @@ class Subscriptions::RenewAsUserTest < ActionDispatch::IntegrationTest end # Check response format & status - assert_equal 201, response.status, "API does not return the expected status."+response.body + assert_equal 201, response.status, "API does not return the expected status. #{response.body}" assert_equal Mime::JSON, response.content_type # Check the correct plan was subscribed @@ -30,16 +30,24 @@ class Subscriptions::RenewAsUserTest < ActionDispatch::IntegrationTest # Check that the user has the correct subscription assert_not_nil @user.subscription, "user's subscription was not found" - assert (@user.subscription.expired_at > DateTime.now), "user's subscription expiration was not updated ... VCR cassettes may be outdated, please check the gitlab wiki" - assert_in_delta 5, (DateTime.now.to_i - @user.subscription.updated_at.to_i), 10, "user's subscription was not updated recently" + assert (@user.subscription.expired_at > DateTime.now), + "user's subscription expiration was not updated ... VCR cassettes may be outdated, please check the gitlab wiki" + assert_in_delta 5, + (DateTime.now.to_i - @user.subscription.updated_at.to_i), + 10, + "user's subscription was not updated recently" # Check that the credits were reset correctly assert_empty @user.users_credits, 'credits were not reset' # Check notifications were sent for every admins - notifications = Notification.where(notification_type_id: NotificationType.find_by_name('notify_admin_subscribed_plan'), attached_object_type: 'Subscription', attached_object_id: subscription[:id]) + notifications = Notification.where( + notification_type_id: NotificationType.find_by_name('notify_admin_subscribed_plan'), + attached_object_type: 'Subscription', + attached_object_id: subscription[:id] + ) assert_not_empty notifications, 'no notifications were created' - notified_users_ids = notifications.map {|n| n.receiver_id } + notified_users_ids = notifications.map(&:receiver_id) User.admins.each do |adm| assert_includes notified_users_ids, adm.id, "Admin #{adm.id} was not notified" end @@ -57,23 +65,23 @@ class Subscriptions::RenewAsUserTest < ActionDispatch::IntegrationTest previous_expiration = @user.subscription.expired_at.to_i - VCR.use_cassette("subscriptions_user_renew_failed") do + VCR.use_cassette('subscriptions_user_renew_failed') do post '/api/subscriptions', { - subscription: { - plan_id: plan.id, - user_id: @user.id, - card_token: 'invalid_card_token' - } + subscription: { + plan_id: plan.id, + user_id: @user.id, + card_token: 'invalid_card_token' + } }.to_json, default_headers end # Check response format & status - assert_equal 422, response.status, "API does not return the expected status."+response.body + assert_equal 422, response.status, "API does not return the expected status. #{response.body}" assert_equal Mime::JSON, response.content_type # Check the error was handled - assert_match /No such token/, response.body + assert_match /No such token/, response.body # Check that the user's subscription has not changed assert_equal previous_expiration, @user.subscription.expired_at.to_i, "user's subscription has changed" diff --git a/test/test_helper.rb b/test/test_helper.rb index f1dc3a4aa..f7c222f9d 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -120,6 +120,11 @@ class ActiveSupport::TestCase end end + def assert_dates_equal(expected, actual, msg = nil) + assert_not_nil actual, msg + assert_equal expected.to_date, actual.to_date, msg + end + private # Parse a line of text read from a PDF file and return the price included inside