[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
This commit is contained in:
vlst
2023-09-13 09:38:58 +00:00
parent 60fb310696
commit f1dbd84e9d
4 changed files with 16 additions and 42 deletions
@@ -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;
@@ -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 = "";
}
}
@@ -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");
}
}
}
@@ -6,23 +6,20 @@
<h4 class="modal-title">Coins/Bills</h4>
</div>
<main class="modal-body">
<div class="money-details-info d-flex justify-content-end mb-2 oveflow-auto">
<div t-foreach="[firstHalfMoneyDetails, lastHalfMoneyDetails]" t-as="moneyDetailsList" t-key="moneyDetailsList_index">
<t t-foreach="moneyDetailsList" t-as="moneyValue" t-key="moneyValue">
<div class="money-details-value d-flex align-items-center w-100 my-1 mx-auto" t-on-input="updateMoneyDetailsAmount">
<input class="pos-input form-control w-50 text-end" t-att-id="moneyValue" type="number" t-model.number="state.moneyDetails[moneyValue]" t-on-focus="ev=>ev.target.select()"/>
<label class="oe_link_icon w-25 text-end" t-att-for="moneyValue">
<t t-if="currency.position === 'before'" t-esc="currency.symbol"/>
<span class="mx-1" t-esc="moneyValue"/>
<t t-if="currency.position === 'after'" t-esc="currency.symbol"/>
</label>
</div>
</t>
<t t-set="bills" t-value="Object.keys(state.moneyDetails).sort((a, b) => a - b)"/>
<div t-attf-style="display: grid; grid-template-rows: repeat(calc({{bills.length}}/2) ,auto); grid-auto-flow: column;">
<div t-foreach="bills" t-as="moneyValue" t-key="moneyValue" class="d-flex align-items-center justify-content-center my-1 ">
<input class="pos-input form-control w-50 text-end" t-att-id="moneyValue" type="number" t-model.number="state.moneyDetails[moneyValue]" t-on-focus="ev=>ev.target.select()"/>
<label class="oe_link_icon w-25 text-end" t-att-for="moneyValue">
<t t-if="currency.position === 'before'" t-esc="currency.symbol"/>
<span class="mx-1" t-esc="moneyValue"/>
<t t-if="currency.position === 'after'" t-esc="currency.symbol"/>
</label>
</div>
</div>
<h4 class="total-section rounded py-2">
Total
<t t-esc="env.utils.formatCurrency(state.total)"/>
<t t-esc="env.utils.formatCurrency(computeTotal())"/>
</h4>
</main>
<footer class="footer footer-flex modal-footer">