[16.0][MIG] pos_customer_wallet #300

carmenbianca · 2023-07-11T14:13:55Z

Description

Odoo task (if applicable)

https://gestion.coopiteasy.be/web#id=10799&action=475&active_id=492&model=project.task&view_type=form&menu_id=536

Checklist before approval

Tests are present (or not needed).
Credits/copyright have been changed correctly.
Change log snippet is present.
(If a new module) Moving this to OCA has been considered.

codecov-commenter · 2023-09-07T09:49:12Z

Codecov Report

Merging #300 (8772350) into 16.0 (b640076) will decrease coverage by 3.22%.
Report is 19 commits behind head on 16.0.
The diff coverage is 86.00%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##             16.0     #300      +/-   ##
==========================================
- Coverage   97.73%   94.52%   -3.22%     
==========================================
  Files          12       26      +14     
  Lines         221      347     +126     
  Branches       30       39       +9     
==========================================
+ Hits          216      328     +112     
- Misses          1       15      +14     
  Partials        4        4

Files Changed	Coverage Δ
pos_customer_wallet/models/pos_session.py	`40.00% <40.00%> (ø)`
pos_customer_wallet/models/pos_config.py	`58.33% <58.33%> (ø)`
account_customer_wallet/tests/common.py	`94.11% <100.00%> (+0.17%)`	⬆️
pos_customer_wallet/__init__.py	`100.00% <100.00%> (ø)`
pos_customer_wallet/models/__init__.py	`100.00% <100.00%> (ø)`
pos_customer_wallet/models/pos_payment_method.py	`100.00% <100.00%> (ø)`
pos_customer_wallet/models/res_partner.py	`100.00% <100.00%> (ø)`
pos_customer_wallet/tests/__init__.py	`100.00% <100.00%> (ø)`
pos_customer_wallet/tests/common.py	`100.00% <100.00%> (ø)`
pos_customer_wallet/tests/test_balance.py	`100.00% <100.00%> (ø)`

... and 5 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

….config Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

…t payment method Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

…stead of settings Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

…ines This does not yet live-update in the POS. The window needs to be reloaded to see the updated balance. Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

… balance Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

…ner._invoice_total algorithm Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

…r account is set instead Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

…ly tree Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

Signed-off-by: Carmen Bianca BAKKER <[email protected]>

…thout perms Previously there was a permission error on trying to read the `account.bank.statement.line` model. Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

Signed-off-by: Carmen Bianca BAKKER <[email protected]>

Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

Signed-off-by: Carmen Bianca BAKKER <[email protected]>

Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

legalsylvain · 2023-09-15T13:31:14Z

Is it ready to review ?

carmenbianca · 2023-09-18T11:37:49Z

@legalsylvain yes, i think!

carmenbianca · 2023-09-18T13:33:22Z

Problem found during functional testing: The balance is not reduced when making POS payments with the child of a partner. Needs further investigation.

carmenbianca · 2023-09-18T13:54:01Z

Ignore previous comment. I was testing the wrong payment method ('Customer Account' instead of 'Customer Wallet')

I did find another bug, though. After closing the session, all changes to the customer wallet balance disappear, as if the POS session never happened.

carmenbianca · 2023-09-18T14:26:02Z

Interesting. The account.move.lines generated by closing the POS do not contain partner_ids. This is going to be painful to research.

legalsylvain · 2023-09-19T12:32:07Z

Hi @carmenbianca. Thanks for porting this module.

Just did some tests.

Globally it works !

Regarding your problem.

Interesting. The account.move.lines generated by closing the POS do not contain partner_ids. This is going to be painful to research.

It is partially wrong. for the payment account move, if you check the checkbox split_transactions AND set the outstanding account : it will :

A) force user to select a customer. (so part of the javascript can be removed, see comment inline)
B) set a partner in the account move. So it's OK. See :

Ex : payment of 5€ with wallet :

BUT : if you sale "wallet product", AFAIK, the account move does'nt contain the partner, if it is not possible to credit the wallet, saling wallet in the PoS.

Ex : credit 999€, saling wallet product :

Do you know how we could fix it ?

legalsylvain · 2023-09-19T12:16:14Z

pos_customer_wallet/static/src/js/Screens/PaymentScreen/PaymentScreen.js

+
+    const WalletPaymentScreen = (PaymentScreen_) =>
+        class extends PaymentScreen_ {
+            /* eslint-disable no-unused-vars */


Suggested change

/* eslint-disable no-unused-vars */

This is needed to satisfy eslint regarding the isForceValidate parameter. It's not smart enough to recognise that await super.validateOrder(...arguments) implies the use of isForceValidate.

legalsylvain · 2023-09-19T12:16:24Z

pos_customer_wallet/static/src/js/Screens/PaymentScreen/PaymentScreen.js

+             * @param {Boolean} isForceValidate - Passed to super.
+             * @returns {Boolean} Whether the order is valid.
+             */
+            async validateOrder(isForceValidate) {


Suggested change

async validateOrder(isForceValidate) {

async validateOrder() {

It doesn't seem right to me to adjust the function signature.

legalsylvain · 2023-09-19T12:16:32Z

pos_customer_wallet/static/src/js/Screens/PaymentScreen/PaymentScreen.js

+             * @returns {Boolean} Whether the order is valid.
+             */
+            async validateOrder(isForceValidate) {
+                /* eslint-enable no-unused-vars */


Suggested change

/* eslint-enable no-unused-vars */

legalsylvain · 2023-09-19T12:19:16Z

pos_customer_wallet/static/src/js/Screens/PaymentScreen/PaymentScreen.js

+                    if (payment_lines_qty > 0) {
+                        this.showPopup("ErrorPopup", {
+                            title: this.env._t("No customer selected"),
+                            body: this.env._t(
+                                "Cannot use customer wallet payment method without selecting a customer.\n\n Please select a customer or use a different payment method."
+                            ),
+                        });
+                        return;
+                    }


useless in V16.
check payment_method.split_transactions will do the job.

carmenbianca · 2023-09-20T09:45:13Z

Thanks for the tip on split_transactions @legalsylvain

BUT : if you sale "wallet product", AFAIK, the account move does'nt contain the partner, if it is not possible to credit the wallet, saling wallet in the PoS.

Ex : credit 999€, saling wallet product :

Do you know how we could fix it ?

I've been looking at this for a while. I'm not sure what a good solution is. When closing the session, it correctly gets the income account from the wallet product for each pos.order.line that sells a wallet product. However, the amount for every such pos.order.line is eventually accumulated/aggregated into a single partner-less account.move.line on the session's account move.

There exists no split_transactions that we can use here, and the code does not seem to have the right hooks for us to force such a behaviour.

Furthermore, the session's account move is on the POS journal, not on the customer wallet journal. If we were to succeed in splitting the lines on the session's account move, we might still run afoul of some expectations made in account_customer_wallet. Trivially fixed, I think, but important to keep in mind.

Some solutions that I can think of, but haven't thought through:

Like above, try to force split_transactions behaviour on the session's account move. There are no good hooks for this, so we might have to do it retroactively. I'm not sure I like the idea of retroactively adjusting account moves.
Do some creative accounting. After the POS session is closed, create enough account moves to solve the problem, reading from the pos.order.lines.
Don't do anything. Or rather, include the pos.order.lines (with wallet products) when calculating the balance. We already do some form of this for open pos sessions. Furthermore, the customer wallet is already credited the correct amount when closing the session.

I will expand later.

carmenbianca · 2023-09-21T08:09:14Z

Now is later.

All three options proposed above have some flaws.

A full example. Find below the account.move of the POS session, where Alice bought 800€ of wallet product, and Bob bought 200€:

Account	Partner	Debit	Credit
Account Receivable (PoS)		1000
Customer Wallet			1000

Option 1: Split on the pos session's account move

I've explained the flaws of option 1 fairly well. The account move is on the wrong journal, retroactively editing a closed account move seems like a bad idea, and there are no good hooks to make this happen.

But if we did make it happen, we would turn the example account move into this:

Account	Partner	Debit	Credit
Account Receivable (PoS)		1000
Customer Wallet	Alice		800
Customer Wallet	Bob		200

Option 2: Do creative accounting after the pos session closes

Option 2 is fine-ish, if a little verbose and difficult to retroactively comprehend. You have to undo the account.move.line against the customer wallet account and create new per-partner lines.

Given the example, we would create one or two new moves that look a little like this:

Two moves against a temporary account

Account	Partner	Debit	Credit
Temporary			1000
Customer Wallet		1000

and

Account	Partner	Debit	Credit
Temporary		1000
Customer Wallet	Alice		800
Customer Wallet	Bob		200

One move against the wallet account itself

Not sure if this is possible.

Account	Partner	Debit	Credit
Customer Wallet		1000
Customer Wallet	Alice		800
Customer Wallet	Bob		200

Option 3: Compute the balance from pos orders; don't touch the account moves any further

Option 3 seems fairly easy, but I'm not certain it's a good idea. One ramification is that you cannot uninstall pos_customer_wallet without data loss to the balances. This is not presently the case if all POS sessions are closed, I think.

But overall, I just feel like it would be (mis)using pos orders for the wrong purpose.

legalsylvain · 2023-10-03T20:22:01Z

Thanks a lot @carmenbianca for all the investigations !

Option 3
Well. I don't think option 3 is OK : For the reason you mentionned, about the uninstallation, but also because the information, regarding the credit should be in account.move.line. It's an important information. If some bridges are done with other accounting software, the data should be exported. (for exemple, in a software for accountant experts).

The account move is on the wrong journal,

I'm not sure. for me saling wallet product should go in sale journal. Don't you think ? I'm pretty sure it is the current behaviour in v12.

Regarding option 2, I think writing a single extra account move is OK in a accounting point of view.

For the time being, I think option 1 has my preference, but I read the code that prepare the account move lines, and it is just horrible. ;-) So maybe option 2 will be mandatory.

What is your deadline for this developpement ? we could make a little code sprint during the oca days?

regards.

carmenbianca · 2023-10-04T09:45:18Z

@legalsylvain We can definitely sprint on this at OCA Days!

I agree with you on option 3.

carmenbianca · 2024-05-17T17:08:58Z

@legalsylvain I wrote an implementation for this finally! It was tricky to get right, but it implements option 1.

It needs heaps more tests before this can be merged, but I thought I'd notify you.

base_suspend_security's functionality is wrapped into Odoo's sudo by default

This change was also done in account_customer_wallet. It simplifies things. This change is necessary for the 16.0 migration, but is not included in its commit for clarity's sake. Signed-off-by: Carmen Bianca BAKKER <[email protected]>

…t computation The pos session does not create these anymore. It creates pos.payment records instead. Signed-off-by: Carmen Bianca BAKKER <[email protected]>

Signed-off-by: Carmen Bianca BAKKER <[email protected]>

…ssion Signed-off-by: Carmen Bianca BAKKER <[email protected]>

…wallet products Signed-off-by: Carmen Bianca BAKKER <[email protected]>

Signed-off-by: Carmen Bianca BAKKER <[email protected]>

legalsylvain · 2024-05-27T12:29:54Z

I have to review this one ! thanks for the ping !

polchampion · 2024-09-18T13:48:01Z

@carmenbianca à combien d'heures estimes-tu la finalisation du portage ?

carmenbianca changed the title ~~[16.0][MIGpos customer wallet~~ [16.0][MIG] pos_customer_wallet Jul 11, 2023

carmenbianca marked this pull request as draft July 11, 2023 14:14

carmenbianca force-pushed the 16.0-mig-pos_customer_wallet branch from 06ace62 to 1109db1 Compare September 7, 2023 09:38

github-grap-bot added the needs review label Sep 7, 2023

carmenbianca force-pushed the 16.0-mig-pos_customer_wallet branch from 1109db1 to afb08db Compare September 7, 2023 09:46

carmenbianca and others added 24 commits September 7, 2023 16:27

[ADD] pos_customer_wallet

d3ddd79

Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

[ADD] pos_customer_wallet: Display wallet balance on POS payment screen

35077ff

Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

[ADD] pos_customer_wallet: Make global module toggle available in pos…

4139829

….config Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

[ADD] pos_customer_wallet: Journal demo data

5321b00

Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

[ADD] pos_customer_wallet: Mandate selected customer when using walle…

8af348e

…t payment method Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

[ADD] pos_customer_wallet: Check sufficient balance before POS purchase

c4cc880

Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

[FIX] account_customer_wallet: Write account_wallet to res.company in…

4c65655

…stead of settings Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

[ADD] pos_account_wallet: Update wallet balance in POS

843035c

Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

[FIX] pos_customer_wallet: Update write_date on computation of wallet…

9441262

… balance Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

[REM] account_customer_wallet: Remove some fields and api.depends lines

5acebf8

Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

[ADD] pos_customer_wallet: Tests

45868c3

Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

[REF] pos_customer_wallet: Re-implement wallet computation using part…

789a5fa

…ner._invoice_total algorithm Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

[REM] account_customer_wallet: Custom wallet toggle; depend on whethe…

2a5aabb

…r account is set instead Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

[FIX] account_customer_wallet: Search balance on all partners in fami…

32556c1

…ly tree Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

[FIX] pos_customer_wallet: Avoid null error when no customer is selected

d9123bf

Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

[ADD] account_customer_wallet: README.rst

ce833b0

Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

[UPD] Update pos_customer_wallet.pot

63f8257

Signed-off-by: Carmen Bianca BAKKER <[email protected]>

pos_customer_wallet 12.0.1.0.1

748bc6a

Signed-off-by: Carmen Bianca BAKKER <[email protected]>

[FIX] SCRLfs → SC

158bbe9

Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

[ADD] pos_customer_wallet: Show balance on ticket

c87c8db

Signed-off-by: Carmen Bianca BAKKER <[email protected]>

[FIX] pos_customer_wallet: typo

a0e7e0e

Signed-off-by: Carmen Bianca BAKKER <[email protected]>

[REF] *_customer_wallet: Create partner.get_all_partners_in_family

c381b9c

Signed-off-by: Carmen Bianca Bakker <[email protected]> Signed-off-by: Carmen Bianca BAKKER <[email protected]>

carmenbianca mentioned this pull request Sep 18, 2023

[16.0][MIG] pos_customer_wallet_partner_is_user #307

Open

4 tasks

legalsylvain reviewed Sep 19, 2023

View reviewed changes

carmenbianca force-pushed the 16.0-mig-pos_customer_wallet branch 2 times, most recently from 228bd28 to 7f85b83 Compare May 27, 2024 12:14

carmenbianca added 11 commits May 27, 2024 14:23

[MIG] pos_customer_wallet: Migration to 13.0

1818433

base_suspend_security's functionality is wrapped into Odoo's sudo by default

[MIG] pos_customer_wallet: Migration to 14.0

cdcac46

[MIG] pos_customer_wallet: Migration to 15.0

64249a0

[MIG] pos_customer_wallet: Migration to 16.0

e0e8249

[REM] pos_customer_wallet: Remove bank statement line method of walle…

7d1cf54

…t computation The pos session does not create these anymore. It creates pos.payment records instead. Signed-off-by: Carmen Bianca BAKKER <[email protected]>

[FIX] pos_customer_wallet: Add some copyright headers

b4c8c4c

Signed-off-by: Carmen Bianca BAKKER <[email protected]>

[IMP] pos_customer_wallet: Some user-facing documentation

3196438

Signed-off-by: Carmen Bianca BAKKER <[email protected]>

[IMP] pos_customer_wallet: Slightly refactor tests to use the same se…

c670d0f

…ssion Signed-off-by: Carmen Bianca BAKKER <[email protected]>

[IMP] pos_customer_wallet: After closing, correctly handle purchased …

0f8eeef

…wallet products Signed-off-by: Carmen Bianca BAKKER <[email protected]>

[REF] pos_customer_wallet: Refactor to .esm.js

dcf77e8

Signed-off-by: Carmen Bianca BAKKER <[email protected]>

carmenbianca force-pushed the 16.0-mig-pos_customer_wallet branch from 7f85b83 to dcf77e8 Compare May 27, 2024 12:24

carmenbianca requested a review from legalsylvain May 27, 2024 12:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[16.0][MIG] pos_customer_wallet #300

[16.0][MIG] pos_customer_wallet #300

carmenbianca commented Jul 11, 2023 •

edited

Loading

codecov-commenter commented Sep 7, 2023 •

edited

Loading

legalsylvain commented Sep 15, 2023

carmenbianca commented Sep 18, 2023

carmenbianca commented Sep 18, 2023

carmenbianca commented Sep 18, 2023

carmenbianca commented Sep 18, 2023

legalsylvain commented Sep 19, 2023

legalsylvain Sep 19, 2023

carmenbianca Sep 20, 2023

legalsylvain Sep 19, 2023

carmenbianca Sep 20, 2023

legalsylvain Sep 19, 2023

legalsylvain Sep 19, 2023

carmenbianca commented Sep 20, 2023

carmenbianca commented Sep 21, 2023 •

edited

Loading

legalsylvain commented Oct 3, 2023

carmenbianca commented Oct 4, 2023

carmenbianca commented May 17, 2024

legalsylvain commented May 27, 2024

polchampion commented Sep 18, 2024

	async validateOrder(isForceValidate) {
	async validateOrder() {

[16.0][MIG] pos_customer_wallet #300

Are you sure you want to change the base?

[16.0][MIG] pos_customer_wallet #300

Conversation

carmenbianca commented Jul 11, 2023 • edited Loading

Description

Odoo task (if applicable)

Checklist before approval

codecov-commenter commented Sep 7, 2023 • edited Loading

Codecov Report

legalsylvain commented Sep 15, 2023

carmenbianca commented Sep 18, 2023

carmenbianca commented Sep 18, 2023

carmenbianca commented Sep 18, 2023

carmenbianca commented Sep 18, 2023

legalsylvain commented Sep 19, 2023

legalsylvain Sep 19, 2023

Choose a reason for hiding this comment

carmenbianca Sep 20, 2023

Choose a reason for hiding this comment

legalsylvain Sep 19, 2023

Choose a reason for hiding this comment

carmenbianca Sep 20, 2023

Choose a reason for hiding this comment

legalsylvain Sep 19, 2023

Choose a reason for hiding this comment

legalsylvain Sep 19, 2023

Choose a reason for hiding this comment

carmenbianca commented Sep 20, 2023

carmenbianca commented Sep 21, 2023 • edited Loading

Option 1: Split on the pos session's account move

Option 2: Do creative accounting after the pos session closes

Two moves against a temporary account

One move against the wallet account itself

Option 3: Compute the balance from pos orders; don't touch the account moves any further

legalsylvain commented Oct 3, 2023

carmenbianca commented Oct 4, 2023

carmenbianca commented May 17, 2024

legalsylvain commented May 27, 2024

polchampion commented Sep 18, 2024

carmenbianca commented Jul 11, 2023 •

edited

Loading

codecov-commenter commented Sep 7, 2023 •

edited

Loading

carmenbianca commented Sep 21, 2023 •

edited

Loading