From 8f3f5f7302f83099edb8940287b0a339f77ffdd9 Mon Sep 17 00:00:00 2001 From: Oleg Hasjanov Date: Tue, 20 Apr 2021 14:10:58 +0300 Subject: [PATCH 1/9] added test for transaction with binded invoice which already paid --- test/tasks/invoices/process_payments_test.rb | 25 ++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/test/tasks/invoices/process_payments_test.rb b/test/tasks/invoices/process_payments_test.rb index eeaf411cc..3f0cd49e6 100644 --- a/test/tasks/invoices/process_payments_test.rb +++ b/test/tasks/invoices/process_payments_test.rb @@ -13,6 +13,8 @@ class ProcessPaymentsTaskTest < ActiveSupport::TestCase total: payment_amount, currency: @payment_currency, reference_no: @payment_reference_number) + @account_activity = account_activities(:one) + Setting.registry_iban = beneficiary_iban Lhv::ConnectApi.class_eval do @@ -29,6 +31,29 @@ class ProcessPaymentsTaskTest < ActiveSupport::TestCase end end + def test_cannot_create_new_invoice_if_transaction_binded_to_paid_invoice + @invoice.update(total: 10) + assert_not @invoice.paid? + + @account_activity.update(activity_type: "add_credit", bank_transaction: nil) + @invoice.update(account_activity: @account_activity, total: 10) + assert @invoice.paid? + + transaction = BankTransaction.new + transaction.description = "Invoice no. #{@invoice.number}" + transaction.sum = 10 + transaction.reference_no = @invoice.reference_no + transaction.save + assert transaction.valid? + + assert_no_difference 'AccountActivity.count' do + assert_no_difference 'Invoice.count' do + Invoice.create_from_transaction!(transaction) unless transaction.autobindable? + transaction.autobind_invoice + end + end + end + def test_doubles_are_valid assert Lhv::ConnectApi.method_defined?(:credit_debit_notification_messages) assert Lhv::ConnectApi::Messages::CreditDebitNotification.method_defined?(:bank_account_iban) From 40368aaa625e6aed05d83b45a0d3e69075136ee5 Mon Sep 17 00:00:00 2001 From: Oleg Hasjanov Date: Wed, 21 Apr 2021 10:47:03 +0300 Subject: [PATCH 2/9] added non canceled method for check paid invoices --- app/models/bank_transaction.rb | 10 ++++++++++ lib/tasks/invoices/process_payments.rake | 6 ++++-- test/tasks/invoices/process_payments_test.rb | 12 +++++++++--- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/app/models/bank_transaction.rb b/app/models/bank_transaction.rb index 24bf51e0c..8fe0d86b5 100644 --- a/app/models/bank_transaction.rb +++ b/app/models/bank_transaction.rb @@ -27,6 +27,16 @@ class BankTransaction < ApplicationRecord .find_by(total: sum) end + def non_canceled? + paid_invoices = registrar.invoices + .order(created_at: :asc) + .non_cancelled + .where(total: sum) + return true if paid_invoices.any?(&:paid?) + + false + end + def registrar @registrar ||= Invoice.find_by(reference_no: parsed_ref_number)&.buyer end diff --git a/lib/tasks/invoices/process_payments.rake b/lib/tasks/invoices/process_payments.rake index edf6609b9..4ce9587f7 100644 --- a/lib/tasks/invoices/process_payments.rake +++ b/lib/tasks/invoices/process_payments.rake @@ -39,9 +39,11 @@ namespace :invoices do reference_no: incoming_transaction.payment_reference_number, description: incoming_transaction.payment_description } transaction = bank_statement.bank_transactions.create!(transaction_attributes) - Invoice.create_from_transaction!(transaction) unless transaction.autobindable? - transaction.autobind_invoice + unless transaction.non_canceled? + Invoice.create_from_transaction!(transaction) unless transaction.autobindable? + transaction.autobind_invoice + end end end else diff --git a/test/tasks/invoices/process_payments_test.rb b/test/tasks/invoices/process_payments_test.rb index 3f0cd49e6..2d993fd7a 100644 --- a/test/tasks/invoices/process_payments_test.rb +++ b/test/tasks/invoices/process_payments_test.rb @@ -14,6 +14,7 @@ class ProcessPaymentsTaskTest < ActiveSupport::TestCase currency: @payment_currency, reference_no: @payment_reference_number) @account_activity = account_activities(:one) + @account = accounts(:cash) Setting.registry_iban = beneficiary_iban @@ -48,8 +49,13 @@ class ProcessPaymentsTaskTest < ActiveSupport::TestCase assert_no_difference 'AccountActivity.count' do assert_no_difference 'Invoice.count' do - Invoice.create_from_transaction!(transaction) unless transaction.autobindable? - transaction.autobind_invoice + assert_no_difference -> {@account.balance} do + unless transaction.non_canceled? + Invoice.create_from_transaction!(transaction) unless transaction.autobindable? + transaction.autobind_invoice + @account.reload + end + end end end end @@ -114,7 +120,7 @@ class ProcessPaymentsTaskTest < ActiveSupport::TestCase run_task end - assert_changes -> { registrar.invoices.count } do + assert_no_changes -> { registrar.invoices.count } do run_task end end From a1c38fb1cc32c9e4e8e9fc1289d9b03f8460076b Mon Sep 17 00:00:00 2001 From: Oleg Hasjanov Date: Wed, 21 Apr 2021 14:04:47 +0300 Subject: [PATCH 3/9] refactoring --- test/tasks/invoices/process_payments_test.rb | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/test/tasks/invoices/process_payments_test.rb b/test/tasks/invoices/process_payments_test.rb index 2d993fd7a..8eca261ab 100644 --- a/test/tasks/invoices/process_payments_test.rb +++ b/test/tasks/invoices/process_payments_test.rb @@ -33,28 +33,16 @@ class ProcessPaymentsTaskTest < ActiveSupport::TestCase end def test_cannot_create_new_invoice_if_transaction_binded_to_paid_invoice - @invoice.update(total: 10) assert_not @invoice.paid? @account_activity.update(activity_type: "add_credit", bank_transaction: nil) - @invoice.update(account_activity: @account_activity, total: 10) + @invoice.update(account_activity: @account_activity, total: @payment_amount) assert @invoice.paid? - transaction = BankTransaction.new - transaction.description = "Invoice no. #{@invoice.number}" - transaction.sum = 10 - transaction.reference_no = @invoice.reference_no - transaction.save - assert transaction.valid? - assert_no_difference 'AccountActivity.count' do assert_no_difference 'Invoice.count' do assert_no_difference -> {@account.balance} do - unless transaction.non_canceled? - Invoice.create_from_transaction!(transaction) unless transaction.autobindable? - transaction.autobind_invoice - @account.reload - end + capture_io { run_task } end end end From af8bc4155607918f43c7b3d159034a737b7abf20 Mon Sep 17 00:00:00 2001 From: Oleg Hasjanov Date: Thu, 22 Apr 2021 11:49:30 +0300 Subject: [PATCH 4/9] implement paid cancel --- app/controllers/admin/invoices_controller.rb | 17 +++++++++++++++++ app/views/admin/invoices/show.haml | 4 ++++ config/locales/en.yml | 4 ++++ config/routes.rb | 4 ++++ 4 files changed, 29 insertions(+) diff --git a/app/controllers/admin/invoices_controller.rb b/app/controllers/admin/invoices_controller.rb index c9f1d0c46..43a1b1ee1 100644 --- a/app/controllers/admin/invoices_controller.rb +++ b/app/controllers/admin/invoices_controller.rb @@ -20,6 +20,23 @@ module Admin end end + def cancel_paid + invoice_id = params[:invoice_id] + invoice = Invoice.find(invoice_id) + + account_activity = AccountActivity.find_by(invoice_id: invoice_id) + account_activity_dup = account_activity.dup + account_activity_dup.sum = -account_activity.sum + + if account_activity_dup.save and invoice.update(cancelled_at: Time.zone.today) + flash[:notice] = t(:payment_was_cancelled) + redirect_to admin_invoices_path + else + flash[:alert] = t(:failed_to_payment_cancel) + redirect_to admin_invoices_path + end + end + def index @q = Invoice.includes(:account_activity).search(params[:q]) @q.sorts = 'number desc' if @q.sorts.empty? diff --git a/app/views/admin/invoices/show.haml b/app/views/admin/invoices/show.haml index d0450469f..b121c8337 100644 --- a/app/views/admin/invoices/show.haml +++ b/app/views/admin/invoices/show.haml @@ -6,6 +6,10 @@ %h1.text-right.text-center-xs - if @invoice.unpaid? = link_to(t(:payment_received), new_admin_bank_statement_path(invoice_id: @invoice.id), class: 'btn btn-default') + + - if @invoice.paid? and !@invoice.cancelled? + = link_to(t(:cancel_payment), cancel_paid_admin_invoices_path(invoice_id: @invoice.id), method: 'post', data: { confirm: t(:are_you_sure) }, class: 'btn btn-warning') + = link_to(t('.download_btn'), download_admin_invoice_path(@invoice), class: 'btn btn-default') = link_to(t('.deliver_btn'), new_admin_invoice_delivery_path(@invoice), class: 'btn btn-default') - if @invoice.cancellable? diff --git a/config/locales/en.yml b/config/locales/en.yml index 97c968996..1d5a469a2 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -293,6 +293,9 @@ en: record_deleted: 'Record deleted' failed_to_delete_record: 'Failed to delete record' + payment_was_cancelled: 'Payment was cancelled' + failed_to_payment_cancel: 'Failed to payment cancel' + authentication_error: 'Authentication error' sign_in_cancelled: "Sign in cancelled" @@ -601,6 +604,7 @@ en: no_transfers_found: 'No transfers found' parameter_value_range_error: 'Parameter value range error: %{key}' payment_received: 'Payment received' + cancel_payment: 'Cancel Payment' api_user_not_found: 'API user not found' notes: Notes active_price_for_this_operation_is: 'Active price for this operation is %{price}' diff --git a/config/routes.rb b/config/routes.rb index 4e78b7c0f..9a8f50b41 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -231,6 +231,10 @@ Rails.application.routes.draw do end resources :invoices, except: %i[edit update destroy] do + collection do + # get ':id/cancel_paid', to: 'invoices#cancel_paid', as: 'get_cancel_paid' + post ':id/cancel_paid', to: 'invoices#cancel_paid', as: 'cancel_paid' + end resource :delivery, controller: 'invoices/delivery', only: %i[new create] member do From 895fa22702d7f48fab4ceaaf0d3d5e7153ca8e7e Mon Sep 17 00:00:00 2001 From: Oleg Hasjanov Date: Thu, 22 Apr 2021 12:45:00 +0300 Subject: [PATCH 5/9] added test --- app/controllers/admin/invoices_controller.rb | 2 +- config/routes.rb | 1 - test/integration/admin_area/invoices_test.rb | 17 +++++++++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/app/controllers/admin/invoices_controller.rb b/app/controllers/admin/invoices_controller.rb index 43a1b1ee1..cf17d78a1 100644 --- a/app/controllers/admin/invoices_controller.rb +++ b/app/controllers/admin/invoices_controller.rb @@ -26,7 +26,7 @@ module Admin account_activity = AccountActivity.find_by(invoice_id: invoice_id) account_activity_dup = account_activity.dup - account_activity_dup.sum = -account_activity.sum + account_activity_dup.sum = -account_activity.sum.to_i if account_activity_dup.save and invoice.update(cancelled_at: Time.zone.today) flash[:notice] = t(:payment_was_cancelled) diff --git a/config/routes.rb b/config/routes.rb index 9a8f50b41..a6a9f5ab3 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -232,7 +232,6 @@ Rails.application.routes.draw do resources :invoices, except: %i[edit update destroy] do collection do - # get ':id/cancel_paid', to: 'invoices#cancel_paid', as: 'get_cancel_paid' post ':id/cancel_paid', to: 'invoices#cancel_paid', as: 'cancel_paid' end resource :delivery, controller: 'invoices/delivery', only: %i[new create] diff --git a/test/integration/admin_area/invoices_test.rb b/test/integration/admin_area/invoices_test.rb index 2aa17201d..01c1a29d7 100644 --- a/test/integration/admin_area/invoices_test.rb +++ b/test/integration/admin_area/invoices_test.rb @@ -4,6 +4,23 @@ class AdminAreaInvoicesIntegrationTest < ApplicationIntegrationTest setup do @invoice = invoices(:one) sign_in users(:admin) + + @account = accounts(:cash) + @registrar = registrars(:bestnames) + end + + def test_cancel_paid_invoice + @invoice.account_activity.update(sum: 10) + assert @invoice.paid? + + assert_equal @registrar.balance, 100 + + assert_no_difference 'Invoice.count' do + assert_difference 'AccountActivity.count' do + post cancel_paid_admin_invoices_path(id: @invoice.id) + "?invoice_id=#{@invoice.id}" + end + end + assert_equal @registrar.balance, 90 end def test_create_new_invoice From 28d156055545a1e797e6c7ecbb4da98bd4213574 Mon Sep 17 00:00:00 2001 From: Oleg Hasjanov Date: Thu, 22 Apr 2021 12:59:27 +0300 Subject: [PATCH 6/9] refactoring --- app/controllers/admin/invoices_controller.rb | 16 +++++++++------- app/models/bank_transaction.rb | 2 -- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/app/controllers/admin/invoices_controller.rb b/app/controllers/admin/invoices_controller.rb index cf17d78a1..e0b0a6d80 100644 --- a/app/controllers/admin/invoices_controller.rb +++ b/app/controllers/admin/invoices_controller.rb @@ -24,17 +24,12 @@ module Admin invoice_id = params[:invoice_id] invoice = Invoice.find(invoice_id) - account_activity = AccountActivity.find_by(invoice_id: invoice_id) - account_activity_dup = account_activity.dup - account_activity_dup.sum = -account_activity.sum.to_i - - if account_activity_dup.save and invoice.update(cancelled_at: Time.zone.today) + if account_activity_with_negative_sum(invoice) flash[:notice] = t(:payment_was_cancelled) - redirect_to admin_invoices_path else flash[:alert] = t(:failed_to_payment_cancel) - redirect_to admin_invoices_path end + redirect_to admin_invoices_path end def index @@ -60,5 +55,12 @@ module Admin def deposit_params params.require(:deposit).permit(:amount, :description, :registrar_id) end + + def account_activity_with_negative_sum(invoice) + account_activity = AccountActivity.find_by(invoice_id: invoice.id) + account_activity_dup = account_activity.dup + account_activity_dup.sum = -account_activity.sum.to_i + account_activity_dup.save && invoice.update(cancelled_at: Time.zone.today) + end end end diff --git a/app/models/bank_transaction.rb b/app/models/bank_transaction.rb index 8fe0d86b5..ab76010ee 100644 --- a/app/models/bank_transaction.rb +++ b/app/models/bank_transaction.rb @@ -33,8 +33,6 @@ class BankTransaction < ApplicationRecord .non_cancelled .where(total: sum) return true if paid_invoices.any?(&:paid?) - - false end def registrar From 1294a05870cde0c56ca9d678fea3d73eac7bbbe2 Mon Sep 17 00:00:00 2001 From: Oleg Hasjanov Date: Thu, 22 Apr 2021 14:15:43 +0300 Subject: [PATCH 7/9] refactoring: separate methods from bank_transaction model to concern --- app/models/bank_transaction.rb | 19 +--------- .../concerns/transaction_paid_invoices.rb | 37 +++++++++++++++++++ test/tasks/invoices/process_payments_test.rb | 18 ++++++++- 3 files changed, 54 insertions(+), 20 deletions(-) create mode 100644 app/models/concerns/transaction_paid_invoices.rb diff --git a/app/models/bank_transaction.rb b/app/models/bank_transaction.rb index ab76010ee..734075ac3 100644 --- a/app/models/bank_transaction.rb +++ b/app/models/bank_transaction.rb @@ -1,5 +1,6 @@ class BankTransaction < ApplicationRecord include Versions + include TransactionPaidInvoices belongs_to :bank_statement has_one :account_activity @@ -17,24 +18,6 @@ class BankTransaction < ApplicationRecord account_activity.invoice end - def invoice - return unless registrar - - @invoice ||= registrar.invoices - .order(created_at: :asc) - .unpaid - .non_cancelled - .find_by(total: sum) - end - - def non_canceled? - paid_invoices = registrar.invoices - .order(created_at: :asc) - .non_cancelled - .where(total: sum) - return true if paid_invoices.any?(&:paid?) - end - def registrar @registrar ||= Invoice.find_by(reference_no: parsed_ref_number)&.buyer end diff --git a/app/models/concerns/transaction_paid_invoices.rb b/app/models/concerns/transaction_paid_invoices.rb new file mode 100644 index 000000000..19d632c1d --- /dev/null +++ b/app/models/concerns/transaction_paid_invoices.rb @@ -0,0 +1,37 @@ +module TransactionPaidInvoices + extend ActiveSupport::Concern + + def invoice + return unless registrar + + @invoice ||= registrar.invoices + .order(created_at: :asc) + .unpaid + .non_cancelled + .find_by(total: sum) + end + + def non_canceled? + paid_invoices = registrar.invoices + .order(created_at: :asc) + .non_cancelled + .where(total: sum) + paid_invoices.any? do |invoice| + return true if invoice.paid? && fresh_admin_paid_invoice(invoice) + end + end + + private + + def fresh_admin_paid_invoice(invoice) + check_for_date_paid_invoice(invoice) && does_invoice_created_by_admin?(invoice) + end + + def check_for_date_paid_invoice(invoice) + invoice.account_activity.created_at > Time.zone.today - 2.days + end + + def does_invoice_created_by_admin?(invoice) + invoice.account_activity.creator_str&.include? 'Admin' + end +end diff --git a/test/tasks/invoices/process_payments_test.rb b/test/tasks/invoices/process_payments_test.rb index 8eca261ab..29f76ea7c 100644 --- a/test/tasks/invoices/process_payments_test.rb +++ b/test/tasks/invoices/process_payments_test.rb @@ -35,7 +35,7 @@ class ProcessPaymentsTaskTest < ActiveSupport::TestCase def test_cannot_create_new_invoice_if_transaction_binded_to_paid_invoice assert_not @invoice.paid? - @account_activity.update(activity_type: "add_credit", bank_transaction: nil) + @account_activity.update(activity_type: "add_credit", bank_transaction: nil, created_at: Time.zone.today - 1.day, creator_str: 'AdminUser') @invoice.update(account_activity: @account_activity, total: @payment_amount) assert @invoice.paid? @@ -48,6 +48,20 @@ class ProcessPaymentsTaskTest < ActiveSupport::TestCase end end + def test_if_invoice_is_overdue_than_48_hours + assert_not @invoice.paid? + + @account_activity.update(activity_type: "add_credit", bank_transaction: nil, created_at: Time.zone.today - 3.days, creator_str: 'AdminUser') + @invoice.update(account_activity: @account_activity, total: @payment_amount) + assert @invoice.paid? + + assert_difference 'AccountActivity.count' do + assert_difference 'Invoice.count' do + capture_io { run_task } + end + end + end + def test_doubles_are_valid assert Lhv::ConnectApi.method_defined?(:credit_debit_notification_messages) assert Lhv::ConnectApi::Messages::CreditDebitNotification.method_defined?(:bank_account_iban) @@ -108,7 +122,7 @@ class ProcessPaymentsTaskTest < ActiveSupport::TestCase run_task end - assert_no_changes -> { registrar.invoices.count } do + assert_changes -> { registrar.invoices.count } do run_task end end From b3e27b37b5013cb8e1a366aa03cd192fb28f4d19 Mon Sep 17 00:00:00 2001 From: Oleg Hasjanov Date: Wed, 28 Apr 2021 16:53:06 +0300 Subject: [PATCH 8/9] change status from cancelled to unpaid --- app/controllers/admin/invoices_controller.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/controllers/admin/invoices_controller.rb b/app/controllers/admin/invoices_controller.rb index e0b0a6d80..42c3b5e48 100644 --- a/app/controllers/admin/invoices_controller.rb +++ b/app/controllers/admin/invoices_controller.rb @@ -60,7 +60,10 @@ module Admin account_activity = AccountActivity.find_by(invoice_id: invoice.id) account_activity_dup = account_activity.dup account_activity_dup.sum = -account_activity.sum.to_i - account_activity_dup.save && invoice.update(cancelled_at: Time.zone.today) + account_activity_dup.save + account_activity.update(invoice_id: nil) + account_activity_dup.update(invoice_id: nil) + account_activity.save && account_activity_dup.save end end end From 2d911e53c34a057576ea19755bacf37919b2808e Mon Sep 17 00:00:00 2001 From: Oleg Hasjanov Date: Thu, 29 Apr 2021 09:51:10 +0300 Subject: [PATCH 9/9] added method which note cancelled payment orders --- app/controllers/admin/invoices_controller.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/controllers/admin/invoices_controller.rb b/app/controllers/admin/invoices_controller.rb index 42c3b5e48..bd54ffd0b 100644 --- a/app/controllers/admin/invoices_controller.rb +++ b/app/controllers/admin/invoices_controller.rb @@ -63,7 +63,13 @@ module Admin account_activity_dup.save account_activity.update(invoice_id: nil) account_activity_dup.update(invoice_id: nil) + mark_cancelled_payment_order(invoice) account_activity.save && account_activity_dup.save end + + def mark_cancelled_payment_order(invoice) + payment_order = invoice.payment_orders.last + payment_order.update(notes: 'Cancelled') + end end end