pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[FIX] point_of_sale: random runbot errors due to NumberBuffer abstraction

Browse Source 
The new randomly occurring runbot errors in pos/* can be reproduced by
performing quick successive clicks in numpad widget with the following form:

    1. Press 'Price' button.
    2. A number input.
    3. Press 'Qty' button.
    4. A number input.

The goal of the above steps is to set the price of the selected orderline
followed by setting the quantity. The result however is that the price
isn't changed but the quantity is changed to combined digits of the price
and quantity inputs. This is due to the implementation of the `NumberBuffer`
singleton which is used to abstract and take into account different sources of
user inputs -- via numpad widget, keyboard and barcode scanner.

Barcode scanner performs fast successive keyboard presses. To differentiate it
from normal keyboard input, we used `eventsBuffer` field of `NumberBuffer`
and store keyboard events there. Fast keyboard presses that stores more than
2 in the events are neglected by the buffer and is considered a barcode input.

In this commit, we save the `handler` that handles the keyboard events saved
in the `eventsBuffer`, then instead of only letting it to be invoked by the
setTimeout object, we allow the manual invoking of `handler` using the introduced
`capture` method. This method is called when the numpad mode is changed
(pressing Price, Discount or Qty button).

Additionally, we remove the use of 'useState' to prevent unnecessary rerender
of the component that uses the `NumberBuffer`.

closes odoo/odoo#56493

X-original-commit: a48d5a3599bff8f9f1446a5f7ad0ac9be0723478
Signed-off-by: pimodoo <pimodoo@users.noreply.github.com>
Signed-off-by: Joseph Caburnay (jcb) <caburj@users.noreply.github.com>
This commit is contained in:
Joseph Caburnay
2020-08-25 10:33:02 +00:00
parent db0e95e368
commit 8ada65ee22
6 changed files with 93 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/point_of_sale/static/src/js/Misc/NumberBuffer.js
+23 -3
View File
@@ -1,7 +1,7 @@
odoo.define('point_of_sale.NumberBuffer', function(require) {
'use strict';
const { Component, useState } = owl;
const { Component } = owl;
const { EventBus } = owl.core;
const { onMounted, onWillUnmount, useExternalListener } = owl.hooks;
const { useListener } = require('web.custom_hooks');
@@ -48,15 +48,17 @@ odoo.define('point_of_sale.NumberBuffer', function(require) {
* - set(val)
* - reset()
* - getFloat()
* - capture()
*
* Note
* ====
* - No need to instantiate as it is a single created before exporting in this module.
* - No need to instantiate as it is a singleton created before exporting in this module.
*
* Possible Improvements
* =====================
* - Relieve the buffer from responsibility of handling `Enter` and other control keys.
* - Make the constants (ALLOWED_KEYS, etc.) more configurable.
* - Write more integration tests. NumberPopup can be used as test component.
*/
class NumberBuffer extends EventBus {
constructor() {
@@ -88,6 +90,19 @@ odoo.define('point_of_sale.NumberBuffer', function(require) {
this.state.buffer = '';
this.trigger('buffer-update', this.state.buffer);
}
/**
* Calling this function, we immediately invoke the `handler` method
* that handles the contents of the input events buffer (`eventsBuffer`).
* This is helpful when we don't want to wait for the timeout that
* is supposed to invoke the handler.
*/
capture() {
if (this.handler) {
clearTimeout(this._timeout);
this.handler();
delete this.handler;
}
}
/**
* @returns {number} float equivalent of the value of buffer
*/
@@ -107,11 +122,15 @@ odoo.define('point_of_sale.NumberBuffer', function(require) {
* @param {Object} config Use to setup the buffer
* @param {String|null} config.decimalPoint The decimal character.
* @param {String|null} config.triggerAtEnter Event triggered when 'Enter' key is pressed.
* @param {String|null} config.triggerAtEsc Event triggered when 'Esc' key is pressed.
* @param {String|null} config.triggerAtInput Event triggered for every accepted input.
* @param {String|null} config.nonKeyboardInputEvent Also listen to a non-keyboard input event
* that carries a payload of { key }. The key is checked if it is a valid input. If valid,
* the number buffer is modified just as it is modified when a keyboard key is pressed.
* @param {Boolean} config.useWithBarcode Whether this buffer is used with barcode.
* @emits config.triggerAtEnter when 'Enter' key is pressed.
* @emits config.triggerAtEsc when 'Esc' key is pressed.
* @emits config.triggerAtInput when an input is accepted.
*/
use(config) {
this.eventsBuffer = [];
@@ -120,7 +139,7 @@ odoo.define('point_of_sale.NumberBuffer', function(require) {
onMounted(() => {
this.bufferHolderStack.push({
component: currentComponent,
state: config.state ? config.state : useState({ buffer: '' }),
state: config.state ? config.state : { buffer: '' },
config,
});
this._setUp();
@@ -160,6 +179,7 @@ odoo.define('point_of_sale.NumberBuffer', function(require) {
clearTimeout(this._timeout);
this.eventsBuffer.push(event);
this._timeout = setTimeout(handler, this.maxTimeBetweenKeys);
this.handler = handler
};
}
_onInput(keyAccessor) {
addons/point_of_sale/static/src/js/Screens/ProductScreen/ProductScreen.js
+3 -2
View File
@@ -150,10 +150,11 @@ odoo.define('point_of_sale.ProductScreen', function(require) {
NumberBuffer.reset();
}
async _setNumpadMode(event) {
_setNumpadMode(event) {
const { mode } = event.detail;
this.state.numpadMode = mode;
NumberBuffer.capture();
NumberBuffer.reset();
this.state.numpadMode = mode;
}
async _updateSelectedOrderline(event) {
if(this.state.numpadMode === 'quantity' && this.env.pos.disallowLineQuantityChange()) {
addons/point_of_sale/static/tests/tours/ProductScreen.tour.js
+1 -3
View File
@@ -40,9 +40,7 @@ odoo.define('point_of_sale.tour.ProductScreen', function (require) {
ProductScreen.check.selectedOrderlineHas('Desk Organizer', '12.0', '61.2');
ProductScreen.do.pressNumpad('3');
ProductScreen.check.selectedOrderlineHas('Desk Organizer', '123.0', '627.3');
ProductScreen.do.pressNumpad('.');
ProductScreen.check.selectedOrderlineHas('Desk Organizer', '123.0', '627.3');
ProductScreen.do.pressNumpad('5 0');
ProductScreen.do.pressNumpad('. 5');
ProductScreen.check.selectedOrderlineHas('Desk Organizer', '123.5', '629.85');
ProductScreen.do.pressNumpad('Price');
ProductScreen.do.pressNumpad('1');
addons/point_of_sale/static/tests/tours/helpers/ProductScreenTourMethods.js
-1
View File
@@ -224,7 +224,6 @@ odoo.define('point_of_sale.tour.ProductScreenTourMethods', function (require) {
res.push(...this._do.pressNumpad('Price'));
res.push(...this._check.modeIsActive('Price'));
res.push(...this._do.pressNumpad(unitPrice.toString().split('').join(' ')));
res.push(...this._check.selectedOrderlineHas(productName, '1.0', unitPrice));
res.push(...this._do.pressNumpad('Qty'));
res.push(...this._check.modeIsActive('Qty'));
}
addons/point_of_sale/static/tests/unit/test_NumberBuffer.js
+65
View File
@@ -0,0 +1,65 @@
odoo.define('point_of_sale.tests.NumberBuffer', function(require) {
'use strict';
const { Component, useState } = owl;
const { xml } = owl.tags;
const NumberBuffer = require('point_of_sale.NumberBuffer');
const makeTestEnvironment = require('web.test_env');
const testUtils = require('web.test_utils');
QUnit.module('unit tests for NumberBuffer', {
before() {},
});
QUnit.test('simple fast inputs with capture in between', async function(assert) {
assert.expect(3);
class Root extends Component {
constructor() {
super();
this.state = useState({ buffer: '' });
NumberBuffer.activate();
NumberBuffer.use({
nonKeyboardInputEvent: 'numpad-click-input',
state: this.state,
});
}
resetBuffer() {
NumberBuffer.capture();
NumberBuffer.reset();
}
}
Root.env = makeTestEnvironment();
Root.template = xml/* html */ `
<div>
<p><t t-esc="state.buffer" /></p>
<button class="one" t-on-click="trigger('numpad-click-input', { key: '1' })">1</button>
<button class="two" t-on-click="trigger('numpad-click-input', { key: '2' })">2</button>
<button class="reset" t-on-click="resetBuffer">reset</button>
</div>
`;
const root = new Root();
await root.mount(testUtils.prepareTarget());
const oneButton = root.el.querySelector('button.one');
const twoButton = root.el.querySelector('button.two');
const resetButton = root.el.querySelector('button.reset');
const bufferEl = root.el.querySelector('p');
testUtils.dom.click(oneButton);
testUtils.dom.click(twoButton);
await testUtils.nextTick();
assert.strictEqual(bufferEl.textContent, '12');
testUtils.dom.click(resetButton);
await testUtils.nextTick();
assert.strictEqual(bufferEl.textContent, '');
testUtils.dom.click(twoButton);
testUtils.dom.click(oneButton);
await testUtils.nextTick();
assert.strictEqual(bufferEl.textContent, '21');
root.unmount();
root.destroy();
});
});
addons/point_of_sale/views/pos_assets_qunit.xml
+1
View File
@@ -50,6 +50,7 @@
<template id="point_of_sale.qunit_suite_tests">
<script type="text/javascript" src="/point_of_sale/static/tests/unit/test_ComponentRegistry.js"></script>
<script type="text/javascript" src="/point_of_sale/static/tests/unit/test_NumberBuffer.js"></script>
<script type="text/javascript" src="/point_of_sale/static/tests/unit/test_ChromeWidgets.js"></script>
<script type="text/javascript" src="/point_of_sale/static/tests/unit/test_ProductScreen.js"></script>
<script type="text/javascript" src="/point_of_sale/static/tests/unit/test_PaymentScreen.js"></script>