Page MenuHomekolab.org

VAT modes
ClosedPublic

Authored by machniak on Feb 17 2023, 4:15 PM.

Details

Reviewers
vanmeeuwen
Group Reviewers
Restricted Project
Commits
rKd14052d8c047: VAT modes

Diff Detail

Repository
rK kolab
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

machniak requested review of this revision.Feb 17 2023, 4:15 PM
machniak created this revision.
machniak updated this revision to Diff 11630.Feb 18 2023, 8:52 AM
  • Fix some regressions
machniak updated this revision to Diff 11636.Feb 20 2023, 11:21 AM
  • taxRate() tests
machniak updated this revision to Diff 11642.Feb 20 2023, 4:13 PM
  • Add data:import:tax-rates command
machniak updated this revision to Diff 11654.Feb 22 2023, 1:08 PM
  • Small corrections
machniak updated this revision to Diff 11660.Feb 22 2023, 2:48 PM
  • Add some cli commands for tax rates

Two other points:

  • Instead of storing the vat with the payment I would store a foreign key to the vat entry with the payment (that said, I don't have a case where the current implementation doesn't work).
  • Not a blocker, but we'll likely require some UI changes as well for the payment process.
src/app/Documents/Receipt.php
134

Not related, but is this expected in here?.

157

Should the rate be hardcoded here?

158

Use ceil here as well. I would create a a function in Payment to calculate the amount (and potentially not store base_amount at all, unless necessary).

181

use ceil

src/app/Http/Controllers/API/V4/Admin/WalletsController.php
87

I would just call the methods instead of assembling a string.

src/app/Http/Controllers/API/V4/PaymentsController.php
511

I think we should be using ceil instead of round, so we always cover at least the tax,

src/app/Payment.php
12

How about amount_after_taxes, or taxed_amount, or wallet_amount?

src/app/Providers/Payment/Coinbase.php
355

I would prefer passing the $payment to credit, and then extract the correct amount there.
I'm not yet sure why we can't just calculate the base_amount/wallet_amount from amount + tax_rate there).

src/app/Providers/PaymentProvider.php
211–212

Rather call the actual method instead of assembling a string that is then invoked as a method, IMO.

src/config/app.php
250

I would prefer a string "taxed_at_source", "not_taxed" (or something like that).

As for the need for base_amount... We could get rid of it if we never support change of app.vat.mode on a working system. We could also store the mode instead. Also, having a calculated value that can be "directly" used for balance-related operations is handy. E.g. for Income chart we don't have to calculate the value in a sql query.

src/app/Documents/Receipt.php
134

Fake data is here so we can produce a test output of a receipt, using template:render command

src/app/Providers/Payment/Coinbase.php
355

Accepting \App\Payment in these Wallet methods might be a good idea.

Instead of storing the vat with the payment I would store a foreign key to the vat entry with the payment (that said, I don't have a case where the current implementation doesn't work).

One thing that would currently be difficult is to show in the UI which tax rate was applied beyond the percentage. I think we should preserve the link to the db entry so we can e.g. show in some UI that the the tax rate from switzerland, active in 2023 was used for this payment. If we only have the percentage we loose the ability to do this (cleanly).

As for the need for base_amount... We could get rid of it if we never support change of app.vat.mode on a working system. We could also store the mode instead. Also, having a calculated value that can be "directly" used for balance-related operations is handy. E.g. for Income chart we don't have to calculate the value in a sql query.

I think it should be fine to change the mode. We store the applied tax with the existing payment, and never change that, so changing the mode should only apply to new payments. That having it calculated for SQL reasons is handy makes sense.

machniak updated this revision to Diff 11666.Feb 24 2023, 1:46 PM
  • Payments: tax_rate -> tax_rate_id, base_amount -> credit_amount
  • Fix some code errors
machniak updated this revision to Diff 11672.Feb 24 2023, 3:09 PM
  • FIx flaky tests
machniak updated this revision to Diff 11678.Feb 24 2023, 3:45 PM
  • TaxRate -> VatRate
  • Fix
machniak updated this revision to Diff 11684.Feb 24 2023, 4:18 PM
  • Cleanup
machniak updated this revision to Diff 11708.Mon, Feb 27, 2:10 PM
  • Credit_amount for refunds
machniak updated this revision to Diff 11714.Tue, Feb 28, 10:26 AM
  • More tests + small fixes
machniak retitled this revision from WIP: Taxes to VAT modes.Tue, Feb 28, 10:29 AM
vanmeeuwen accepted this revision.Tue, Feb 28, 11:48 AM
This revision is now accepted and ready to land.Tue, Feb 28, 11:48 AM
Closed by commit rKd14052d8c047: VAT modes (authored by machniak, committed by Jeroen van Meeuwen (Apheleia IT) <vanmeeuwen@apheleia-it.ch>). · Explain WhyTue, Feb 28, 11:49 AM
This revision was automatically updated to reflect the committed changes.