[16.0] pos_self_service_* #1157

carmenbianca · 2024-02-19T14:15:04Z

This is a heavily WIP module to use the Odoo POS as a self-service terminal for customers. Customers will be able to:

Weigh their own products, like fruit at a supermarket.
Tare their containers, like bringing a pot in which to put any amount of rice.
- And subsequently, weigh that rice with the weight of the container removed.

It'll be a while before this is finished. Opening the PR early for early reviews from colleagues. Other OCA contributors needn't look at this PR for the moment.

carmenbianca · 2024-02-19T14:15:38Z

Pinging @huguesdk for an early eye-over of the code to see if it passes the sniff test.

huguesdk

thanks for this! it seems like a strong base. some things i noticed while testing it:

when it is not able to connect to the iotbox, a red dialog box appears saying:

Unknown Error

Unable to show information about this error.

a new one appears on top of the previous one every few seconds. it would be nice to have a more informative message (or an icon somewhere showing the status).

moreover, it doesn’t seem to take the status of the scale into account: even if it is reported as disconnected, it will read the weight repeatedly.

with this module loaded, the normal (not self-service weight) point of sale interface is different (while it shouldn’t): to purple odoo top bar is not visible.

huguesdk · 2024-02-26T09:51:28Z

pos_self_service_weight_base/__manifest__.py

good idea to mention “weight” in the module name. it seems that i forgot about that. 👍

huguesdk · 2024-02-26T09:54:47Z

pos_self_service_weight_base/models/pos_config.py

+class PosConfig(models.Model):
+    _inherit = "pos.config"
+
+    iface_self_service = fields.Boolean(


i’m wondering about iface in the name. i see other settings like that in odoo, but they all seem directly related to hardware devices. what do you think about is_self_service_weight or enable_self_service_weight?

done in 4d2c034

huguesdk · 2024-02-26T09:57:33Z

pos_self_service_weight_base/models/pos_config.py

+        string="Is Self-Service",
+        help="Use this POS as self-service point",


i think mentioning “weight” would be useful.

done in 4d2c034

huguesdk · 2024-02-26T09:58:39Z

pos_self_service_weight_base/models/res_config_settings.py

+class ResConfigSettings(models.TransientModel):
+    _inherit = "res.config.settings"
+
+    pos_iface_self_service = fields.Boolean(


same here: what about pos_is_self_service_weight?

done in 4d2c034

huguesdk · 2024-02-26T10:00:53Z

pos_self_service_weight_base/static/src/css/pos.css

where are all these css rules coming from? is that self-made or copied from somewhere else?

huguesdk · 2024-02-26T10:05:40Z

pos_self_service_weight_base/static/src/js/Chrome.js

+    const Chrome = require("point_of_sale.Chrome");
+    const Registries = require("point_of_sale.Registries");
+
+    const SelfServiceChrome = (Chrome_) =>


any reason for the underscore in Chrome_? i know it is a local variable independent of the one defined above, but i see that most oca modules use the same name.

I like Carmen's way better but I changed it anyway for consistency.

cf 4d2c034

huguesdk · 2024-02-26T14:46:23Z

pos_self_service_weight_base/static/src/js/Screens/SelfServiceScreen/WeightWidget.js

+
+    const {onMounted, onWillUnmount, useState} = owl;
+
+    class WeightWidget extends PosComponent {


could this inherit from point_of_sale.ScaleScreen to avoid re-implementing similar methods?

I'm not sure it's a good idea :

the template displays the order button : should be hidden in both tare and product mode

the template displays the price, it should be hidden in tare mode

it needs to be loaded with a product

it is usually called as a popup which returns a payload

If there is a clean way to do it, please point in the right direction :-)

robinkeunen · 2024-03-20T16:20:06Z

pos_self_service_weight_base/static/src/js/Screens/SelfServiceScreen/SelfServiceHomeButton.js

+    const Registries = require("point_of_sale.Registries");
+    const {useListener} = require("@web/core/utils/hooks");


@huguesdk i don't understand the difference between these two syntaxes, can you enlighten me ?

the second syntax is called destructuring assignment and in this case it is equivalent to:

const useListener = require("@web/core/utils/hooks").useListener;

it avoids repeating the variable name, but it is even more useful when assigning multiple variables together, like this:

const {useListener, useService} = require("@web/core/utils/hooks");

robinkeunen · 2024-03-20T16:24:29Z

pos_self_service_weight_base/static/src/js/Screens/SelfServiceScreen/WeightWidget.js

+        _readScale() {
+            this.env.proxy_queue.schedule(this._setWeight.bind(this), {
+                duration: 500,
+                repeat: true,
+            });
+        }


Do I understand correctly ? Once the weight widget is mounted, _read_scale schedules a call of _set_weight every 500ms ?

I do not understand the bind instruction.

unlike in python, in javascript a reference to a method is not bound to a this variable. bind() returns a new function that, when called, provides the this variable to the method. more information on mdn.

i’m not familiar with the schedule() function, but as i understand it, it will call this._setWeight() every 500 ms, forever.

robinkeunen · 2024-03-20T17:01:17Z

@huguesdk should i consider translating the code as a Native JS Module ?

robinkeunen · 2024-03-21T13:40:11Z

squashed previous work

huguesdk

@robinkeunen thanks for all the improvements!

some remarks:

the red “unknown error” dialog box still appears.
the top bar is now visible again in the normal pos, but in self-service weighing mode as well, while it should be hidden, as the customer shouldn’t be able to easily exit the interface.
previously, the module was called pos_self_service_base, without a reference to weighing, while here it is specifically about weighing, so i suggest to rename all classes (and files) from *SelfService* to *SelfServiceWeighing*.
(not that important) it is possible to create a self-service weight station without checking the “electronic scale” checkbox is the iot box section. having something checking this would be a plus (only if it is quick and easy).

huguesdk · 2024-04-05T12:43:37Z

pos_self_service_weight_base/static/src/css/pos.css

+}
+
+.foodbox {
+    background-image: url("../../img/empty_foodbox_on_scale.svg");


where is this file?

is all this css needed at this point or is this mostly coming from the old pos_self_service_base?

I did not look into it at all. Writing it down for later.

huguesdk · 2024-04-05T12:52:10Z

pos_self_service_weight_base/static/src/js/Chrome.esm.js

@@ -0,0 +1,35 @@
+/** @odoo-module alias=pos_self_service_weight_base.Chrome **/


is the alias needed? (same remark for all modules)

it is needed if we want to allow other modules to inherit this one using the old Odoo define syntax. so not strictly needed.

i also prefer importing module names than file names but that's maybe me coming from python and lack consistency.

do you think I should change it ?

since this module is extending classes that still use the old define syntax, i think t makes sense to keep it.

huguesdk · 2024-04-06T15:36:58Z

pos_self_service_weight_base/static/src/js/Chrome.esm.js

+        }
+
+        showTicketButton() {
+            return (


a why comment would help to understand what this method is for.

huguesdk · 2024-04-06T15:50:00Z

pos_self_service_weight_base/models/pos_config.py

+    _inherit = "pos.config"
+
+    is_self_service_weight_point = fields.Boolean(
+        string="Is Self-Service",


Suggested change

string="Is Self-Service",

string="Is a Self-Service Weighing Station",

huguesdk · 2024-04-06T15:52:02Z

pos_self_service_weight_base/views/pos_config_views.xml

+                        <div class="o_setting_right_pane">
+                            <label for="is_self_service_weight_point" />
+                            <div class="text-muted mb16" id="is_self_service_weight_point_text">
+                                Use your PoS as self-service


Suggested change

Use your PoS as self-service

Use this PoS as a self-service weighing station

huguesdk · 2024-04-06T16:31:42Z

...lf_service_weight_base/static/src/xml/Screens/SelfServiceScreen/SelfServiceControlButton.xml

+-->
+<templates id="template" xml:space="preserve">
+
+    <t t-name="pos_self_service_weight_base.SelfServiceControlButton" owl="1">


Suggested change

<t t-name="pos_self_service_weight_base.SelfServiceControlButton" owl="1">

<t t-name="pos_self_service_weight_base.SelfServiceWeighingControlButton" owl="1">

huguesdk · 2024-04-06T16:31:51Z

pos_self_service_weight_base/static/src/xml/Screens/SelfServiceScreen/SelfServiceScreen.xml

+-->
+<templates id="template" xml:space="preserve">
+
+    <t t-name="pos_self_service_weight_base.AbstractSelfServiceScreen" owl="1">


Suggested change

<t t-name="pos_self_service_weight_base.AbstractSelfServiceScreen" owl="1">

<t t-name="pos_self_service_weight_base.AbstractSelfServiceWeighingScreen" owl="1">

huguesdk · 2024-04-06T16:32:16Z

...rvice_weight_base/static/src/xml/Screens/SelfServiceWeightScreen/SelfServiceActionButton.xml

+-->
+<templates id="template" xml:space="preserve">
+
+    <t t-name="pos_self_service_weight_base.SelfServiceActionButton" owl="1">


Suggested change

<t t-name="pos_self_service_weight_base.SelfServiceActionButton" owl="1">

<t t-name="pos_self_service_weight_base.SelfServiceWeighingActionButton" owl="1">

huguesdk · 2024-04-06T16:32:29Z

...vice_weight_base/static/src/xml/Screens/SelfServiceWeightScreen/SelfServiceWelcomeScreen.xml

+SPDX-License-Identifier: AGPL-3.0-or-later
+-->
+<templates id="template" xml:space="preserve">
+    <t t-name="pos_self_service_weight_base.SelfServiceWelcomeScreen" owl="1">


Suggested change

<t t-name="pos_self_service_weight_base.SelfServiceWelcomeScreen" owl="1">

<t t-name="pos_self_service_weight_base.SelfServiceWeighingWelcomeScreen" owl="1">

done for all

huguesdk · 2024-04-06T16:38:41Z

...vice_weight_base/static/src/xml/Screens/SelfServiceWeightScreen/SelfServiceWelcomeScreen.xml

+        <div class="screen">
+            <div class="screen-full-width">
+                    <div class="self-service-action-buttons">
+                        <SelfServiceActionButton t-if="env.isDebug()" />


why is this button shown only in debug mode?

It exists to test the behavior of the base module when there are nos other modules installed. It can be removed in the future.

robinkeunen · 2024-04-10T18:00:35Z

@huguesdk I restored the top bar because

i wanted to exit cleanly the interface
i find it useful to have the information of the connections and the logged user
i find the look more consistent with the other point of sale
(i read the specification too fast)

To mitigate the risk of closing the point of sale by accident, I added the confirmation popup (suggestion from Rémy).

I tend to agree that the risk to have customer closing the weighing station is high so, keeping in mind the add benefits of the bar, do you agree to rather hide the "close" button and remove the popup altogether.

@victor-champonnois fyi

Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be> [IMP] pos_self_service_weight: hide orders button [IMP] pos_self_service: add CloseSelfServicePopup No cash is manipulated in self service mode do not therefore ask the user to control the amounts. [IMP] pos_self_service: translate to js native modules [IMP] pos_self_service: rename files to .esm.js [IMP] pos_self_service: rename files to .esm.js [ADD] pos_self_service: add welcome screen [REF] pos_self_service: rename SSScreen

carmenbianca

Quite a big review. Some of the things in the review are my own bad/questionable decisions.

carmenbianca · 2024-04-11T09:43:41Z

pos_self_service_weighing_base/readme/CONTRIBUTORS.rst

@@ -0,0 +1,3 @@
+* `Coop IT Easy SC, Odoo Community Association (OCA) <https://github.com/OCA/pos>`_:
+
+  * Carmen Bianca BAKKER


Add Robin Keunen

carmenbianca · 2024-04-11T11:47:48Z

pos_self_service_weighing_base/static/src/xml/Chrome.xml

+            <attribute name="t-if">showTicketButton()</attribute>
+        </xpath>
+    </t>
+


There used to be this code here:

<xpath expr="//div[hasclass('pos-topheader')]" position="attributes"> <attribute name="t-att-class" >{ oe_hidden: state.uiState !== 'READY' || env.pos.config.iface_self_service }</attribute> </xpath> <xpath expr="//div[hasclass('pos-content')]" position="attributes"> <t t-if="env.pos.config.ifaceself_service"> <attribute name="class">pos-content positionzero</attribute> </t> </xpath>

This removed the bar at the top. Now, a user can click on 'close', and be sent back to the Odoo interface. This is bad for two reasons:

Depending on how Odoo is configured, this allows the user access to things they shouldn't be able to see.

It may be difficult/time-consuming to recover from such an error.

'Close' is also an incredibly normal button to press, so this may be very frequent.

I talked with @robinkeunen after forgetting to read his comment from earlier today → hiding the close button would also be a satisfactory solution. This would also allow getting rid of some code elsewhere related to the close pop-up.

carmenbianca · 2024-04-11T12:00:36Z

pos_self_service_weighing_base/static/src/js/ChromeWidgets/HeaderButton.esm.js

+const SelfServiceWeighingHeaderButton = (HeaderButton) =>
+    class extends HeaderButton {
+        async onClick() {
+            if (!this.env.pos.config.is_self_service_weighing_point) {
+                return super.onClick();
+            }
+            try {
+                const info = await this.env.pos.getClosePosInfo();
+                this.showPopup("CloseSelfServiceWeighingPopup", {
+                    info: info,
+                    keepBehind: true,
+                });
+            } catch (e) {
+                if (isConnectionError(e)) {
+                    this.showPopup("OfflineErrorPopup", {
+                        title: this.env._t("Network Error"),
+                        body: this.env._t(
+                            "Please check your internet connection and try again."
+                        ),
+                    });
+                } else {
+                    this.showPopup("ErrorPopup", {
+                        title: this.env._t("Unknown Error"),
+                        body: this.env._t(
+                            "An unknown error prevents us from getting closing information."
+                        ),
+                    });
+                }
+            }
+        }
+    };
+Registries.Component.extend(HeaderButton, SelfServiceWeighingHeaderButton);
+export default SelfServiceWeighingHeaderButton;


Oof… This is 99% upstream code, except uses a custom implementation of the closing pop-up. That isn't great. Can we not edit the XML/template of ClosePosPopup to display something completely different if the POS is a weighing station?

Although, as mentioned in a previous comment, I'm not sure we should be able to close from this screen.

...base/static/src/js/Screens/SelfServiceWeighingScreen/SelfServiceWeighingControlButton.esm.js

carmenbianca · 2024-04-11T12:18:02Z

...g_base/static/src/js/Screens/SelfServiceWelcomeScreen/SelfServiceWeighingActionButton.esm.js

+class SelfServiceWeighingActionButton extends PosComponent {
+    get name() {
+        return "Base Action";
+    }
+
+    get faSymbol() {
+        return "fa-barcode";
+    }


What is this class? This seems very similar to SelfServiceWeighingControlButton, but with some strange defaults?

I think I've worked this out. It's the button on the welcome screen. The defaults are there because there's a button in the debug mode?

I think a few things need to happen to improve this:

Rename SelfServiceWeighingActionButton to SelfServiceWeighingWelcomeButton. This is much easier to disambiguate from SelfServiceWeighingControlButton.

Remove all the default values from SelfServiceWeighingWelcomeButton.

Create SelfServiceWeighingDebugButton with the removed defaults.

carmenbianca · 2024-04-11T13:03:31Z

...weighing_base/static/src/xml/Screens/SelfServiceWeighingScreen/SelfServiceWeighingScreen.xml

+<templates id="template" xml:space="preserve">
+
+    <t
+        t-name="pos_self_service_weighing_base.AbstractSelfServiceWeighingScreen"


Name does not match file name.

carmenbianca · 2024-04-11T13:04:42Z

..._tare/static/src/xml/Screens/SelfServiceWeighingTareScreen/SelfServiceWeighingTareScreen.xml

+    <t t-name="pos_self_service_weighing_tare.SelfServiceWeighingTareScreen" owl="1">
+        <div class="screen">
+            <div class="screen-full-width">
+                <div class="leftpane pane-border">
+                    <WeightWidget />
+                    <div class="self-service-control-buttons">
+                        <SelfServiceWeighingHomeButton />
+                    </div>
+                </div>
+                <div class="rightpane">
+                    <div class="self-service-action-buttons">
+                        <SelfServiceWeighingPrintTareButton />
+                    </div>
+                </div>
+            </div>
+        </div>
+    </t>


This completely re-implements pos_self_service_weighing_base.AbstractSelfServiceWeighingScreen, which I'm not sure is intentional. Can you inherit the latter template without changing it for other inheritors?

Talked with @robinkeunen about this. We can do primary inheritance here as described in my previous comment, but there's a chance that the primary inheritance would be more lines of code in a much less transparent way. e.g. insert a button here, insert a button there, etc.

For developer ease, it may be nice to just see the simple hierarchy like in the quoted code. The hierarchy is sufficiently small that the repetition is barely a problem. And there would be repetition, because there will be other screens that re-implement pos_self_service_weighing_base.AbstractSelfServiceWeighingScreen. Furthermore, although we currently think that other implementations of AbstractSelfServiceWeighingScreen will look incredibly similar, this may not always be the case.

But… For the time being, it's still repetition. And because this hierarchy kind of depends on pos.css from the main module to display well, it's probably better to do primary inheritance?

Your thoughts @huguesdk ?

@huguesdk we discussed this with Carmen. It is now obvious to us wether primary inheritance is better (less code - maybe) or worse (lacks clarity) then redefining a template. What is your point of view ?

carmenbianca · 2024-04-11T13:05:28Z

...ng_tare/static/src/js/Screens/SelfServiceWeighingScreen/SelfServiceWeighingTareScreen.esm.js

+import PosComponent from "point_of_sale.PosComponent";
+import Registries from "point_of_sale.Registries";
+
+class SelfServiceWeighingTareScreen extends PosComponent {}


Why does this not extend AbstractSelfServiceWeighingScreen?

carmenbianca · 2024-04-11T13:06:14Z

...tic/src/xml/Screens/SelfServiceWeighingWelcomeScreen/SelfServiceWeighingTareScreenButton.xml

+    <t
+        t-name="pos_self_service_weighing_tare.SelfServiceWeighingTareScreenButton"
+        owl="1"
+    >
+        <button class="button">
+            <t t-esc="name" />
+            <i t-attf-class="fa {{ faSymbol }}" />
+        </button>
+    </t>


This file is not needed.

done (and other similar)

carmenbianca · 2024-04-11T13:06:48Z

...re/static/src/js/Screens/SelfServiceWeighingScreen/SelfServiceWeighingPrintTareButton.esm.js

+SelfServiceWeighingPrintTareButton.template =
+    "pos_self_service_weighing_tare.SelfServiceWeighingPrintTareButton";


You can set the template to pos_self_service_weighing_base.SelfServiceWeighingControlButton.

…utton

legalsylvain added this to the 16.0 milestone Feb 21, 2024

huguesdk suggested changes Feb 26, 2024

View reviewed changes

robinkeunen reviewed Mar 20, 2024

View reviewed changes

robinkeunen force-pushed the 16.0-self-service-weight branch from 4ce441e to 90a8935 Compare March 21, 2024 13:39

robinkeunen force-pushed the 16.0-self-service-weight branch 2 times, most recently from 574f882 to 9c19367 Compare March 28, 2024 11:18

robinkeunen force-pushed the 16.0-self-service-weight branch 2 times, most recently from fd7eaec to b070c43 Compare April 4, 2024 14:13

robinkeunen force-pushed the 16.0-self-service-weight branch from b070c43 to cfb0155 Compare April 5, 2024 09:34

huguesdk suggested changes Apr 6, 2024

View reviewed changes

robinkeunen force-pushed the 16.0-self-service-weight branch from cfb0155 to ad88585 Compare April 10, 2024 17:31

robinkeunen force-pushed the 16.0-self-service-weight branch from ad88585 to 636b82f Compare April 10, 2024 18:01

carmenbianca commented Apr 11, 2024

View reviewed changes

robinkeunen added 3 commits April 11, 2024 16:51

[ADD] pos_self_service_weight_tare

31f99fd

fixup! [ADD] pos_self_service_weight_base

ec74c6a

[IMP] pos_self_service: remove close button

759e250

robinkeunen force-pushed the 16.0-self-service-weight branch from 636b82f to 759e250 Compare April 11, 2024 15:16

robinkeunen added 4 commits April 11, 2024 18:02

[REF] pos_self_service_base: refactor SelfServiceActionButton

7d8714f

[REF] pos_self_service_base: reuse control button template for home b…

c150077

…utton

[REF] pos_self_service_tare: reuse control button template for home b…

8fd828a

…utton

fixup! [ADD] pos_self_service_weight_base

d942e95

robinkeunen force-pushed the 16.0-self-service-weight branch from 0cd9587 to 86c297c Compare April 11, 2024 16:12

robinkeunen added 2 commits April 12, 2024 10:20

fixup! [ADD] pos_self_service_weight_tare

a586b0e

fixup! fixup! [ADD] pos_self_service_weight_base

27970e3

wip

b15ce82

robinkeunen force-pushed the 16.0-self-service-weight branch from 86c297c to b15ce82 Compare May 8, 2024 11:21

		string="Is Self-Service",
		help="Use this POS as self-service point",


		const {onMounted, onWillUnmount, useState} = owl;

		class WeightWidget extends PosComponent {

		const Registries = require("point_of_sale.Registries");
		const {useListener} = require("@web/core/utils/hooks");

		@@ -0,0 +1,35 @@
		/ @odoo-module alias=pos_self_service_weight_base.Chrome /

	string="Is Self-Service",
	string="Is a Self-Service Weighing Station",

	Use your PoS as self-service
	Use this PoS as a self-service weighing station

	<t t-name="pos_self_service_weight_base.SelfServiceControlButton" owl="1">
	<t t-name="pos_self_service_weight_base.SelfServiceWeighingControlButton" owl="1">

		@@ -0,0 +1,3 @@
		* `Coop IT Easy SC, Odoo Community Association (OCA) <https://github.com/OCA/pos>`_:

		* Carmen Bianca BAKKER

		SelfServiceWeighingPrintTareButton.template =
		"pos_self_service_weighing_tare.SelfServiceWeighingPrintTareButton";

[16.0] pos_self_service_* #1157

Are you sure you want to change the base?

[16.0] pos_self_service_* #1157

Conversation

carmenbianca commented Feb 19, 2024

carmenbianca commented Feb 19, 2024

huguesdk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robinkeunen Mar 21, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robinkeunen commented Mar 20, 2024

robinkeunen commented Mar 21, 2024

huguesdk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robinkeunen commented Apr 10, 2024

carmenbianca left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robinkeunen Mar 21, 2024 •

edited