Wrong display for fees

Hi Aporie,

Again, thanks a lot for this patch.
After applying it, I did a very deep testing of all the functionalities involving fees, and you can find below the summary of my tests.
To note it goes far beyond my own needs (fees would be limited to Escrows on my site), but I hope it helps you (as you helped me with the patches :-))

Note: I have applied the patch manually, as it is the only mean I'm able to use. I have attached the resulting file, just in case I did a mistake applying it which would explain some of the failed tests.

Please do not hesitate to tell me if you have any question.

Thanks again,

1- Transfer (setup in module configuration: 5% with min 0.50)

Test case:

1 Transfer of 12$ to activate %fees (5% - should be 0.60) and 1 Transfer of 8$ to activate fixed min fees (should be 0.50)

Results:

- Transfer works when fees are calculated with % - OK
- Transfer works when fees are calculated with the fixed min - OK
- Fees are correctly calculated for %fees and fixed min fees - OK
- Message displayed with the correct amount of fees for %fees and fixed min fees - OK
- Users balances (for both issuer and recipient) are correctly calculated with the correct amount of fees for %fees and fixed min fees (for the issuer) - OK
- Transaction views are also correct for both issuer and recipient (--> See screenshot Transfer) - OK

Specific use case (transaction issuer is Admin):
When transferring money from the admin account to a user, the message after transaction displays an amount of fees, whereas no fees apply in that case (fees are subtracted from admin account, but credited on admin account at the same time, resulting in 0 fee, which is the normal behaviour). The message could be misleading for the admin.
Also, transaction views show the amount of fees and net amount for incoming transfers on the recipient views (whereas only brut amount should be shown here, as recipient do not care about the fees and total amount paid by the issuer). But this is solved by a simple view custo (hiding these columns) that every admin can do.

2- Escrow (setup in module configuration: 10% with min 0.60)

Test case:

1 Escrow of 15$ to activate %fees (10% - should be 1.50) and 1 Escrow of 5$ to activate fixed min fees (should be 0.60)

Results:

- Escrow works when fees are calculated with % - OK
- Escrow works when fees are calculated with the fixed min - OK
- Fees are incorrectly calculated for %fees and fixed min fees. Indeed, Escrow uses the Transfer fees settings instead of the Esccrow fees settings (in this test set: 5% with min 0.50 instead of 10% with min 0.60) (--> See screenshot Escrow). When no fees is set for Transfer, no fees are considered by the Escrow transaction - FAILED
- Message displayed with the correct amount of fees for %fees and fixed min fees (WARNING: Correct means here that it corresponds to the transaction effectively done, but the fees amount itself is wrong since the transaction does not use the Escrow settings but the Transfer settings, cf above) - OK
- User balances (for both issuer and recipient) correctly calculated with the correct amount of fees for %fees and fixed min fees (for the issuer) (WARNING: Correct means here that it corresponds to the transaction effectively done, but the fees amount itself is wrong since the transaction does not use the Escrow settings but the Transfer settings, cf above) - OK
- Transaction views are also correct for both issuer and recipient (--> See screenshot Escrow) (WARNING: Correct means here that it corresponds to the transaction effectively done, but the fees amount itself is wrong since the transaction does not use the Escrow settings but the Transfer settings, cf above) - OK
- Escrow status (Pending or Completed) working well and correctly displayed in Transactions views - OK

Specific use case (transaction issuer is Admin):
Here it is very strange, because the behaviour is not the same as for the Transfer. Fees are indeed subtracted from the admin account, whereas they should not. From a site balance perspective, this is not a normal behaviour, the fees amount being now lost in the nature (not paid to anybody or?) and the overall balance between users and the site becomes wrong).
Note: Again, Transfer settings are used here for the fees calculation instead of the Escrow settings.

3- Deposit (setup in module configuration: 15% with min 0.70)

Test case:

1 deposit of 2000$ to activate %fees (15% - should be 300$) and 1 deposit of 2$ to activate fixed min fees (should be 0.70)

Results:

- Deposit works when fees are calculated with % - OK
- Deposit doesn't work when fees are calculated with the fixed min (checkout process not completed. The site always redirects back to order summary when clicking on "Pay and complete purchase") - FAILED
- For %fees, fees amount is correctly calculated - OK
- For fixed min fees, fees amount is correctly calculated (seen in order summary, but transaction does not work, cf above)- OK
- No site message is displayed, the user is redirected to a page where he gets a summary of his deposit. However, this summary doesn't mention the fees he just paid (only the brut amount). Would be better maybe for the user experience to get a site message including fees amount, and being redirected to another page (the complete page is not so useful, and it is the only transaction behaving this way) - FAILED
- In the Order Summary and in Transaction views, fees are added to brut amount, whereas for a deposit they should be subtracted (--> see screenshots Deposit 1 and Deposit 2)- FAILED
- Fees are ignored for the user balance calculation. They are neither added or subtracted, user balance is updated only with brut amount (--> see screenshot Deposit - user balance) - FAILED

Specific use case (transaction issuer is Admin):
Here also, the behaviour is like for the Escrow. Fees are indeed added to the admin/site account, whereas an admin should not pay fees to himself when he wants to add money in the economy of his site.
This is even more strange that when a user does a deposit, no fees are deducted from his deposited amount! It should be the contrary.
Note: Again, Transfer settings are used here for the fees calculation instead of the Escrow settings.

4- Withdrawal (setup 20% - min 0.80)

Test case:

1 Withdrawal of 16$ to activate %fees (20% - should be 3.20) and 1 withdrawal of 2$ to activate fixed min fees (should be 0.80)

Results:

- Withdrawal works when fees are calculated with % - OK
- Withdrawal works when fees are calculated with the fixed min - OK
- Fees are correctly calculated for %fees and fixed min fees - OK
- Message displayed with the correct amount of fees for %fees and fixed min fees - OK
- Transaction views is correct, however the column title "Total due" is misleading. From an admin perspective, total due would mean what has to be transferred to the user, but fees are not paid to the user, on the contrrary they are paid by the user to the site, deducted from the requested amount (--> See screenshot Withdrawal) - OK
- User balance is correctly updated with fees - OK
- Site balance is correctly updated with the fees amount- OK

No problem at all, and I'm glad it helps.

Reworking on this today, I think you can maybe ignore the two following failed tests for deposit:

- In the Order Summary and in Transaction views, fees are added to brut amount, whereas for a deposit they should be subtracted (--> see screenshots Deposit 1 and Deposit 2)- FAILED
- Fees are ignored for the user balance calculation. They are neither added or subtracted, user balance is updated only with brut amount (--> see screenshot Deposit - user balance) - FAILED

I think it is indeed a wrong interpretation from my side of how the module works, now I've tried again I think it is working OK.
My expectation was indeed that users enter an amount (2000$ in my use case), and from this amount fees are subtracted (so 1700$ should go to the user balance, and 300$ to admin). In other words, I expected the user entering a net amount, and being credited with a brut amount.
However, I think now that like for other transactions the user is entering the brut amount, and fees are added on top. This is why I had the feeling that the user balance did not consider the fees (the credit was 2000$ and not 1700$).
What made me change my mind is that I saw today that my admin balance is well credited with 300$ of fees, and yes the orders summary tells the user that he will pay 2300$...

So I need maybe to dig a bit more on this, I will try to confirm you when everything's clear in my mind (it was late yesterday when I did test that part :-)).

Just tell me if/how I can help further.

Hi Aporie,

Again, what a quick feedback! You can take time also for your daytime job :-)
Unfortunately, after applying the patch, I have an error (The website encountered an unexpected error. Please try again later.) while trying to make a deposit.

Error log shows the following:

Error: Call to a member function hasPermission() on null in Drupal\commerce_funds\Services\FeesManager->calculateOrderFee() (line 94 of ...\modules\contrib\commerce_funds\src\Services\FeesManager.php)

This prevents me to do any test, as all balances are 0, so impossible to make any transaction.

Any idea where this may come from?

Thanks,

Of course I did it :-)

I also thought I wrongly applied the patch so I uninstalled and reinstalled everything, then reapplies the patch but with the same error in the end.

Also my escrow rule is broken now with a logged error saying that "context method is not a valid context".

dev version was working!!!
So maybe I did some mistakes while applying the patch... apologies if it is the case.

Results of today:

Deposit:

Works perfectly fine when %fees is used, but still blocking when min fixed fees are used. But I understand this bug may not come from your module (to note that I do not intend myself to use deposit fees at all, so it will not be a blocking point for me, but could be for others)
To note that now I have the following message displayed: You have canceled checkout at [payment gateway] but may resume the checkout process here when you are ready. I had no message before (as far as I remember).

Transfer:

Works perfectly well!

Escrow:

Works perfectly well!

Withdrawal:

Works perfectly well!

Specific use case (transaction issuer is Admin):

Works perfectly well!

As a summary, everything looks fine which is awesooooooooooooooooooooooome :-)

Thanks mate for the reactivity/efficiency!!!