From f1dbd84e9dc9ca2ac475e1c1a47aab7adf6e0911 Mon Sep 17 00:00:00 2001 From: vlst Date: Fri, 8 Sep 2023 20:03:01 +0200 Subject: [PATCH] [FIX] point_of_sale: fix MoneyDetailsPopup The MoneyDetailsPopup accepts the prop `total`. This popup is used to keep track of the bills selected by the user and it's total should thus simply be computed from the given bills information. Passing the `total` prop and using it as `state` serves no purpose and leads to bugs, such as the one addressed in this task. Steps to reproduce: 1. Click on "Close Session". 2. Input a value in the "counted" field 3. Open the "MoneyDetailsPopup" 4. Observe the fact that the "Total" value is the one from the "counted" field, instead of "0". This is caused because the `ClosePosPopup` has to also independently keep track of this `total`, in order to be able to pass it to the `MoneyDetailsPopup` on future calls. In order to fix this problem from the root cause, we completely remove the prop `total` and instead allow the popup to compute it from it's information about the selected bills. This simplifies the code, preventing future similar bugs. Task: 3499242 Part-of: odoo/odoo#131171 --- .../app/navbar/closing_popup/closing_popup.js | 6 ----- .../cash_opening_popup/cash_opening_popup.js | 4 --- .../money_details_popup.js | 25 +++++-------------- .../money_details_popup.xml | 23 ++++++++--------- 4 files changed, 16 insertions(+), 42 deletions(-) diff --git a/addons/point_of_sale/static/src/app/navbar/closing_popup/closing_popup.js b/addons/point_of_sale/static/src/app/navbar/closing_popup/closing_popup.js index 94c207e6ec4..f317e9c3fef 100644 --- a/addons/point_of_sale/static/src/app/navbar/closing_popup/closing_popup.js +++ b/addons/point_of_sale/static/src/app/navbar/closing_popup/closing_popup.js @@ -27,7 +27,6 @@ export class ClosePosPopup extends AbstractAwaitablePopup { this.report = useService("report"); this.hardwareProxy = useService("hardware_proxy"); this.customerDisplay = useService("customer_display"); - this.manualInputCashCount = false; this.cashControl = this.pos.config.cash_control; this.closeSessionClicked = false; this.moneyDetails = null; @@ -74,9 +73,6 @@ export class ClosePosPopup extends AbstractAwaitablePopup { this.hardwareProxy.openCashbox(action); const { confirmed, payload } = await this.popup.add(MoneyDetailsPopup, { moneyDetails: this.moneyDetails, - total: this.manualInputCashCountpayments - ? 0 - : this.state.payments[this.defaultCashDetails.id].counted, action: action, }); if (confirmed) { @@ -90,7 +86,6 @@ export class ClosePosPopup extends AbstractAwaitablePopup { if (moneyDetailsNotes) { this.state.notes = moneyDetailsNotes; } - this.manualInputCashCount = false; this.moneyDetails = moneyDetails; } } @@ -103,7 +98,6 @@ export class ClosePosPopup extends AbstractAwaitablePopup { } let expectedAmount; if (paymentId === this.defaultCashDetails?.id) { - this.manualInputCashCount = true; this.moneyDetails = null; this.state.notes = ""; expectedAmount = this.defaultCashDetails.amount; diff --git a/addons/point_of_sale/static/src/app/store/cash_opening_popup/cash_opening_popup.js b/addons/point_of_sale/static/src/app/store/cash_opening_popup/cash_opening_popup.js index 86c01c6e94b..dfbf416c22b 100644 --- a/addons/point_of_sale/static/src/app/store/cash_opening_popup/cash_opening_popup.js +++ b/addons/point_of_sale/static/src/app/store/cash_opening_popup/cash_opening_popup.js @@ -15,7 +15,6 @@ export class CashOpeningPopup extends AbstractAwaitablePopup { setup() { super.setup(); - this.manualInputCashCount = null; this.moneyDetails = null; this.pos = usePos(); this.state = useState({ @@ -44,7 +43,6 @@ export class CashOpeningPopup extends AbstractAwaitablePopup { this.hardwareProxy.openCashbox(action); const { confirmed, payload } = await this.popup.add(MoneyDetailsPopup, { moneyDetails: this.moneyDetails, - total: this.manualInputCashCount ? 0 : this.state.openingCash, action: action, }); if (confirmed) { @@ -53,7 +51,6 @@ export class CashOpeningPopup extends AbstractAwaitablePopup { if (moneyDetailsNotes) { this.state.notes = moneyDetailsNotes; } - this.manualInputCashCount = false; this.moneyDetails = moneyDetails; } } @@ -61,7 +58,6 @@ export class CashOpeningPopup extends AbstractAwaitablePopup { if (!this.env.utils.isValidFloat(this.state.openingCash)) { return; } - this.manualInputCashCount = true; this.state.notes = ""; } } diff --git a/addons/point_of_sale/static/src/app/utils/money_details_popup/money_details_popup.js b/addons/point_of_sale/static/src/app/utils/money_details_popup/money_details_popup.js index 6867ce9e90b..1d46b617488 100644 --- a/addons/point_of_sale/static/src/app/utils/money_details_popup/money_details_popup.js +++ b/addons/point_of_sale/static/src/app/utils/money_details_popup/money_details_popup.js @@ -16,30 +16,17 @@ export class MoneyDetailsPopup extends AbstractAwaitablePopup { moneyDetails: this.props.moneyDetails ? { ...this.props.moneyDetails } : Object.fromEntries(this.pos.bills.map((bill) => [bill.value, 0])), - total: this.props.total ? this.props.total : 0, - action: this.props.action ? this.props.action : null, }); } - get firstHalfMoneyDetails() { - const moneyDetailsKeys = Object.keys(this.state.moneyDetails).sort((a, b) => a - b); - return moneyDetailsKeys.slice(0, Math.ceil(moneyDetailsKeys.length / 2)); - } - get lastHalfMoneyDetails() { - const moneyDetailsKeys = Object.keys(this.state.moneyDetails).sort((a, b) => a - b); - return moneyDetailsKeys.slice( - Math.ceil(moneyDetailsKeys.length / 2), - moneyDetailsKeys.length - ); - } - updateMoneyDetailsAmount() { - this.state.total = Object.entries(this.state.moneyDetails).reduce( + computeTotal(moneyDetails = this.state.moneyDetails) { + return Object.entries(moneyDetails).reduce( (total, money) => total + money[0] * money[1], 0 ); } //@override async getPayload() { - let moneyDetailsNotes = !floatIsZero(this.state.total, this.currency.decimal_places) + let moneyDetailsNotes = !floatIsZero(this.computeTotal(), this.currency.decimal_places) ? "Money details: \n" : null; this.pos.bills.forEach((bill) => { @@ -50,10 +37,10 @@ export class MoneyDetailsPopup extends AbstractAwaitablePopup { } }); return { - total: this.state.total, + total: this.computeTotal(), moneyDetailsNotes, moneyDetails: { ...this.state.moneyDetails }, - action: this.state.action, + action: this.props.action, }; } async cancel() { @@ -62,7 +49,7 @@ export class MoneyDetailsPopup extends AbstractAwaitablePopup { this.pos.config.iface_cashdrawer && this.pos.hardwareProxy.connectionInfo.status === "connected" ) { - this.pos.logEmployeeMessage(this.state.action, "ACTION_CANCELLED"); + this.pos.logEmployeeMessage(this.props.action, "ACTION_CANCELLED"); } } } diff --git a/addons/point_of_sale/static/src/app/utils/money_details_popup/money_details_popup.xml b/addons/point_of_sale/static/src/app/utils/money_details_popup/money_details_popup.xml index cb7834f3c23..928a9ae9a63 100644 --- a/addons/point_of_sale/static/src/app/utils/money_details_popup/money_details_popup.xml +++ b/addons/point_of_sale/static/src/app/utils/money_details_popup/money_details_popup.xml @@ -6,23 +6,20 @@

Coins/Bills

-
-
- -
- - -
- + +
+
+ +

Total - +