[IMP] PoS usability improvements #32870

AntoineVDV · 2019-04-23T10:53:27Z

Description of the issue/feature this PR addresses

Add a stat button on PoS session forms linking to the details of the orders.
Put the 'Sessions' menu item in first position of the 'Orders' menu.
Set 'PoS Orders' as the default operation type (picking type) for newly created PoS configs.

Task ID : 1970513

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

bouvyd

some comments, nothing major besides the read_group to count pos order for better perf

bouvyd · 2019-05-02T11:59:03Z

addons/point_of_sale/models/pos_session.py

@@ -111,6 +112,11 @@ def _confirm_orders(self):

    _sql_constraints = [('uniq_name', 'unique(name)', "The name of this POS Session must be unique !")]

+    @api.multi
+    def _compute_order_count(self):
+        for pos in self:


Aaaah, classic badly-performing count field :D

use read_group instead

bouvyd · 2019-05-02T12:02:15Z

addons/point_of_sale/models/pos_session.py

+    @api.multi
+    def action_view_order(self):
+        action = {
+            'name': 'Orders',


should be translatable

action = { 'name': _('Orders'), # .....

bouvyd · 2019-05-02T12:03:40Z

addons/point_of_sale/models/pos_session.py

@@ -345,6 +351,20 @@ def open_cashbox(self):

        return action

+    @api.multi


@api.multi is the default mode, no need to specify it (same for the method above as well)

bouvyd · 2019-05-02T12:05:47Z

addons/point_of_sale/models/stock_warehouse.py

+class Warehouse(models.Model):
+    _inherit = "stock.warehouse"
+
+    pos_type_id = fields.Many2one('stock.picking.type', string="PoS type")


shitty label is shitty (I know, you took example on stock - I prefer the example in mrp though)
let's use "Point of Sale Operation Type" instead

bouvyd · 2019-05-02T12:08:06Z

addons/point_of_sale/views/pos_session_view.xml

@@ -62,6 +62,13 @@
                            type="object" context="{'balance': 'end'}">
                            <span class="o_stat_text">Set Closing Balance</span>
                        </button>
+                        <button name="action_view_order"
+                            class="oe_stat_button"
+                            attrs="{'invisible': [('order_count', '=', 0)]}"


I wouldn't make the button invisible in case of 0 orders, we can expect all sessions to always have at least one order anyway (and users might get confused if the field is not always visible)

unless it was a specific request from the PO? didn't see it in the spec

bouvyd · 2019-05-02T12:15:46Z

addons/point_of_sale/models/pos_session.py

+            'type': 'ir.actions.act_window',
+            'domain': [('session_id', 'in', self.ids)],
+        }
+        if len(self.order_ids) == 1:


I wouldn't handle that case; if you have only one order in your session it doesn't shock me to have the list view open anyway...
this makes sense when it's frequent for an o2m field to lead to a single record (e.g. link between SO and invoice: you expect only one invoice per SO most of the time); less so when you expect the normal case to be 'more than one'

addons/point_of_sale/models/stock_warehouse.py

bouvyd · 2019-05-02T12:32:29Z

addons/point_of_sale/__init__.py

+
+    It is necessary if the point_of_sale module is installed after some warehouses were already created.
+    """
+    env = api.Environment(cr, SUPERUSER_ID, {})


since you have multiple functions in the same parent call (both functions get called), it's better to use an environment manager here to avoid instantiating twice the same env by using the env manager:

with api.Environment.manage(): env = api.Environment(cr, SUPERUSER_ID, {}) # do stuff

same for _assign_picking_types

the first environment you will instantiate will be stored in memory and reused in the second function

bouvyd · 2019-05-02T12:41:44Z

addons/point_of_sale/__init__.py

+    env = api.Environment(cr, SUPERUSER_ID, {})
+    pos_configs = env['pos.config'].search([('picking_type_id', '=', False)])
+    for pos_config in pos_configs:
+        default_picking_type_id = env['stock.warehouse'].search([('company_id', '=', pos_config.company_id.id)], limit=1).pos_type_id.id


wouldn't it make more sense to get the picking type from the warehouse linked to the pos_config via the stock_location_id m2o instead? that way we're sure that we take the picking type from the same location...

not entirely trivial, since there is no direct link between the location and the warehouse AFAICT

Resolved as of part 4 of task #1970513

addons/point_of_sale/models/pos_config.py

bouvyd

multi-company changes needed
@pimodoo can review as well if he so wishes

bouvyd · 2019-05-14T08:00:01Z

addons/point_of_sale/models/pos_config.py

@@ -39,6 +39,9 @@ class PosConfig(models.Model):
    _name = 'pos.config'
    _description = 'Point of Sale Configuration'

+    def _default_picking_type_id(self):
+        return self.env['stock.warehouse'].search([('company_id', '=', self.env.user.company_id.id)], limit=1).pos_type_id.id


multi-company revamp has been merged before you, so a few changes:

[('company_id', '=', self.env.company_id.id)]

should do the trick (self.env.company_id is the main company in the context, self.env.company_ids contains all visible companies in the context - the context is stored client side, so different browser tabs can have completely different contexts)

bouvyd · 2019-05-14T08:00:55Z

addons/point_of_sale/models/pos_config.py

+        related='picking_type_id.default_location_src_id',
+        string='Stock Location',
+        readonly=True,
+        default=_get_default_location,


no need for default on a related readonly - it might break stuff in fact (readonly is only enforced in client; if your default is called after the picking_type_id default and does not match the value, you've just updated the default location of the picking!)

bouvyd · 2019-05-14T08:14:49Z

addons/point_of_sale/models/pos_order.py

@@ -572,7 +572,7 @@ def _default_pricelist(self):
    picking_type_id = fields.Many2one('stock.picking.type', related='session_id.config_id.picking_type_id', string="Operation Type", readonly=False)
    location_id = fields.Many2one(
        comodel_name='stock.location',
-        related='session_id.config_id.stock_location_id',
+        related='session_id.config_id.picking_type_id.default_location_src_id',


I should've seen it the first time, but this is not correct; if the picking type's location gets changed it will retroactively change the location of old orders... it's actually not your code that causes this (it was here before) but this is clearly a bug waiting to happen. Same for picking_type_id - changing the picking type of the config should certainly not change the picking type of all existing orders for that config... I'm actually pretty surprised this did not happen before 🤔
@pimodoo probably an override of default_get, a good task for a newbie - do you want it or do we do it here?

edit: never mind my comment for picking_type_id, it's not stored. Just need a default for location_id then

As session, is a more global object, we want to put its menu item before the one for orders. TASK-ID: 1970513

When you are looking at a point of sale session, the detail of what is sales have been done during this session is stored in the POS Orders linked to it. So we've added button on the session form that will show all related sales done during this session. TASK-ID: 1970513

We are handeling if the default sales team for pos has been deleted by raising and catching an Access error instead of a ValueError TASK-ID: 1970513

When you are selling goods through point of sale, pinkings are created to remove what have been sold from the stock. This picking is created by using the picking_type set on pos_config. For now, the there is only one picking type POS Order existing by default, which is located on the first warehouse in data. It means that if you create another warehouse and/or company, you will have to manually create picking type if you want to be able to link pos config to the new warehouse. So now, each warehouse will have its own picking_type for POS, which will ease the link between the pos config and the warehouse. We've also set a default value on pos config to take the first picking type, and added a domain that will show only the outgoing types. TASK-ID: 1970513

There is currently a stock location that is set on the pos config which is by default the the default source of the picking type. This field is not really usefull and we prefer that the configuration of the source location where the POS will take products come from a dedicated picking type that will have the correct default source location. So we have removed the stock_location_id from the pos_config and changed the related set on orders, by the location of the picking set on the pos order, as it was not really consistent that if the pos_config was changed the location of all existing orders was changed too. TASK-ID: 1970513

pimodoo · 2019-06-03T13:47:48Z

robodoo r+ rebase-ff

robodoo · 2019-06-03T13:48:12Z

Merge method set to rebase and fast-forward

There is currently a stock location that is set on the pos config which is by default the the default source of the picking type. This field is not really usefull and we prefer that the configuration of the source location where the POS will take products come from a dedicated picking type that will have the correct default source location. So we have removed the stock_location_id from the pos_config and changed the related set on orders, by the location of the picking set on the pos order, as it was not really consistent that if the pos_config was changed the location of all existing orders was changed too. TASK-ID: 1970513 closes #32870 Signed-off-by: pimodoo <pimodoo@users.noreply.github.com>

robodoo · 2019-06-03T16:24:12Z

Merged, thanks!

robodoo added the CI 🤖 Robodoo has seen passing statuses label Apr 23, 2019

C3POdoo added the RD research & development, internal work label Apr 23, 2019

robodoo removed the CI 🤖 Robodoo has seen passing statuses label Apr 26, 2019

AntoineVDV force-pushed the master-pos-usability-anv branch from f067842 to a0d0651 Compare April 29, 2019 07:48

robodoo added the CI 🤖 Robodoo has seen passing statuses label Apr 29, 2019

AntoineVDV force-pushed the master-pos-usability-anv branch from a0d0651 to 994d9ee Compare April 29, 2019 09:48

robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels Apr 29, 2019

bouvyd requested changes May 2, 2019

View reviewed changes

bouvyd reviewed May 2, 2019

View reviewed changes

addons/point_of_sale/models/pos_config.py Show resolved Hide resolved

AntoineVDV force-pushed the master-pos-usability-anv branch from 994d9ee to 61677a5 Compare May 3, 2019 09:59

robodoo removed the CI 🤖 Robodoo has seen passing statuses label May 3, 2019

AntoineVDV force-pushed the master-pos-usability-anv branch 4 times, most recently from 404b90b to 6c47521 Compare May 3, 2019 13:37

robodoo added the CI 🤖 Robodoo has seen passing statuses label May 3, 2019

AntoineVDV force-pushed the master-pos-usability-anv branch from 6c47521 to d7850bc Compare May 8, 2019 13:28

robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels May 8, 2019

bouvyd requested changes May 14, 2019

View reviewed changes

AntoineVDV force-pushed the master-pos-usability-anv branch from d7850bc to dadb85b Compare May 16, 2019 10:45

robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels May 16, 2019

AntoineVDV force-pushed the master-pos-usability-anv branch from dadb85b to b1d9013 Compare May 22, 2019 08:12

robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels May 22, 2019

pimodoo force-pushed the master-pos-usability-anv branch from b1d9013 to bc9269d Compare May 24, 2019 09:54

robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels May 24, 2019

robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels May 28, 2019

pimodoo force-pushed the master-pos-usability-anv branch from f61a20d to cf87518 Compare May 28, 2019 12:20

robodoo removed the CI 🤖 Robodoo has seen passing statuses label May 28, 2019

pimodoo force-pushed the master-pos-usability-anv branch 5 times, most recently from 74ecd9e to 544e2ef Compare May 28, 2019 13:15

robodoo added the CI 🤖 Robodoo has seen passing statuses label May 28, 2019

AntoineVDV and others added 4 commits June 3, 2019 11:01

[IMP] point_of_sale: Change 'Orders' menu sequence

03005c7

As session, is a more global object, we want to put its menu item before the one for orders. TASK-ID: 1970513

[REF] point_of_sale: default sales team on config

db8e342

We are handeling if the default sales team for pos has been deleted by raising and catching an Access error instead of a ValueError TASK-ID: 1970513

pimodoo force-pushed the master-pos-usability-anv branch from 544e2ef to 5d517f8 Compare June 3, 2019 09:02

robodoo removed the CI 🤖 Robodoo has seen passing statuses label Jun 3, 2019

pimodoo force-pushed the master-pos-usability-anv branch from 5d517f8 to ea26512 Compare June 3, 2019 12:09

robodoo added the CI 🤖 Robodoo has seen passing statuses label Jun 3, 2019

robodoo added the r+ 👌 label Jun 3, 2019

robodoo added merging 👷 and removed merging 👷 labels Jun 3, 2019

robodoo closed this Jun 3, 2019

AntoineVDV deleted the master-pos-usability-anv branch June 19, 2019 12:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[IMP] PoS usability improvements #32870

[IMP] PoS usability improvements #32870

AntoineVDV commented Apr 23, 2019

bouvyd left a comment

bouvyd May 2, 2019

bouvyd May 2, 2019

bouvyd May 2, 2019

bouvyd May 2, 2019

bouvyd May 2, 2019

bouvyd May 2, 2019

AntoineVDV May 3, 2019

bouvyd May 2, 2019

bouvyd May 2, 2019

AntoineVDV May 3, 2019

bouvyd left a comment

bouvyd May 14, 2019

bouvyd May 14, 2019

bouvyd May 14, 2019

pimodoo commented Jun 3, 2019

robodoo commented Jun 3, 2019

robodoo commented Jun 3, 2019

		@@ -345,6 +351,20 @@ def open_cashbox(self):

		return action

		@api.multi

[IMP] PoS usability improvements #32870

[IMP] PoS usability improvements #32870

Conversation

AntoineVDV commented Apr 23, 2019

bouvyd 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

bouvyd 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

pimodoo commented Jun 3, 2019

robodoo commented Jun 3, 2019

robodoo commented Jun 3, 2019