Page MenuHomekolab.org

Add possibility to change system currency
ClosedPublic

Authored by machniak on Aug 12 2021, 11:59 AM.

Details

Reviewers
mollekopf
Group Reviewers
Restricted Project
Commits
rK9c0964980d2f: Add possibility to change system currency
Summary

And use tenant-specific app.name also in PaymentsController

Test Plan

./phpunit

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.Aug 12 2021, 11:59 AM
machniak created this revision.
machniak planned changes to this revision.Aug 12 2021, 2:03 PM
machniak updated this revision to Diff 7720.Aug 12 2021, 3:36 PM
  • Add tests, fix small bugs
  • Use wallet currency in bonus/penalty dialog
machniak edited the summary of this revision. (Show Details)Aug 13 2021, 8:27 AM
machniak updated this revision to Diff 7726.Aug 13 2021, 8:37 AM
machniak edited the summary of this revision. (Show Details)
  • More tests

Looks good except for the array_merge question

src/app/Providers/Payment/Mollie.php
586

Can array_merge handle a non-existing $eurMethods in the $currency == 'EUR' case? Otherwise set to []

machniak planned changes to this revision.Aug 16 2021, 10:24 AM

I'm planning on updating this diff with a fix for stats (where the income chart should sum up eur and chf payments using conversion rates and present the results in chf).

src/app/Providers/Payment/Mollie.php
586

I don't get the question. If $currency == 'EUR' array_merge is not executed, because the first request to mollie will return them.

mollekopf accepted this revision.Aug 16 2021, 12:22 PM
machniak updated this revision to Diff 7732.Aug 16 2021, 12:23 PM
  • Support multi-currency in income calculation for stats
This revision is now accepted and ready to land.Aug 16 2021, 12:23 PM
mollekopf added inline comments.Aug 16 2021, 12:37 PM
src/app/Http/Controllers/API/V4/Admin/StatsController.php
369

This needs an implementation otherwise $addTenantScope above is never used, no?

388

Are the resellers users not in the resellers currency?

machniak added inline comments.Aug 16 2021, 1:00 PM
src/app/Http/Controllers/API/V4/Admin/StatsController.php
369

applyTenantScope() is implemented in API/V4/Reseller/StatsController. Here it can be empty (until we implement tenant selector for admin stats). For now admins work without the tenant scope, resellers work in their tenant scope.

388

I'm not sure I get the question. Admin work in system currency. Resellers work in their tenant currency, which is their wallet currency. Note that right now it does not really matter, as we do not have Income chart implementation for resellers (yet).

mollekopf accepted this revision.Aug 16 2021, 1:05 PM
This revision was automatically updated to reflect the committed changes.