[16.0][ADD] pos_container_deposit

[hbrunn]

migration of #616 with @legalsylvain's comments applied

[legalsylvain]

Hi @hbrunn. Thanks a lot for porting and finishing this module ! (I need it in V16 in a few monthes ;-))

Some remarks inline + functional review.

Functional review :

problem 1 :
set consign as not available in PoS
set product as available in pos + consign

when selecting the product, there is an not handled error.

Proposal : (not sure)

• at least add a more sexy message in the Javascript. "The product X has a consign, but it is not available in the point of sale, please make the consign available in the PoS".
• (alternatively, load silently the consign, I think it's possible dynamicaly in recent version).
• maybe (not sure), add a constrains, when setting consign on product, if consign are not available in PoS.

Problem 2 :

• select a product in PoS : OK.

• click a second time on the product : KO.

[hbrunn]

@legalsylvain thanks for your comprehensive review, took all your points.

I took the liberty to fix you quoting yourself which messed up runboat ;-)

[legalsylvain]

I took the liberty to fix you quoting yourself which messed up runboat ;-)

oh thanks !

[legalsylvain]

UX nitpicking : I guess product.deposit_product_id[1] contains the name of the deposit. we could add it in the message, so the user know directly which product he has to enable for the PoS.

[legalsylvain]

I didn't faced that issue. I mean

• open PoS
• Add a product (it adds a consign)
• set quantity = 33 (it set qty = 33 on consign)
• refresh
• set quantity = 50 on product (it set qty = 50 on consign).

What is the "bug" / "limitation" described here ?

[hbrunn]

ah, that's a leftover from v13

[legalsylvain]

code review and test. LGTM. Thanks !

[hbrunn]

@legalsylvain curious about your take on discussion with the customer:

currently you can edit the deposit product line independently of the line it's a deposit for, I was thinking about doing this because a customer buys 10 bottles and returns 5, then you don't need to scan 5 negative deposit bottles, but just edit the quantity of the deposit. Do you consider this a valid use case, or should we rather not allow editing the deposit line at all?

[legalsylvain]

currently you can edit the deposit product line independently of the line it's a deposit for, I was thinking about doing this because a customer buys 10 bottles and returns 5, then you don't need to scan 5 negative deposit bottles, but just edit the quantity of the deposit. Do you consider this a valid use case, or should we rather not allow editing the deposit line at all?

Yes. return and purchase consign in a single sale is a common use case, at least in my business : For exemple, the groceries sell beer in returnable bottles. So regularly, the customer comes with an empty bottle, and leaves with a full one.

But I have another questions. (not blocking point).

1. In this exemple (beer in returnable bottles), I'm not sure how to initialize consign stock. I mean : some suppliers doesnt mention the sell of the consign. So if I purchase beer and sell beer + consign, consign stock will be negative. Do you face similar question ? do you think develop some pos_deposit_purchase could make sense to automatically increment consign stock ?

2. In theory, we could imagine use this feature also on sale.order ? (something like a button "compute Consigns" on draft sale.order, present, if the sale.order contains lines with products with deposit. In that case, we could imagine some split of the module. product_deposit / sale_deposit / pos_deposit. What do you think ? (some split or refactoring can be done in a second time, if needed).

3. Finally, about the naming, maybe there is some conflict regarding "deposit" word.

There are first :

• https://github.com/OCA/product-attribute/tree/16.0/product_packaging_container_deposit
• https://github.com/OCA/purchase-workflow/tree/16.0/purchase_product_packaging_container_deposit
• https://github.com/OCA/sale-workflow/tree/16.0/sale_product_packaging_container_deposit
• https://github.com/OCA/multi-company/tree/16.0/purchase_sale_container_deposit_inter_company

If I understand correctly, the 4 modules mentioned above implement something similar to your implementation in the product level, but in a more complex (or complete) manner, with packaging.

Also there are :

• https://github.com/OCA/purchase-workflow/tree/16.0/purchase_deposit
  and also
• https://github.com/OCA/stock-logistics-workflow/tree/16.0/stock_customer_deposit, https://github.com/OCA/stock-logistics-workflow/tree/16.0/stock_customer_deposit_elaboration, https://github.com/OCA/stock-logistics-workflow/tree/16.0/stock_customer_deposit_sale_margin,

But I'm not sure to understand very well the usage of those modules

(Finally, there is https://github.com/OCA/account-financial-tools/tree/16.0/account_cash_deposit but this is a very disctinct topic)

All this to say that the OCA is full of “deposit” modules that talk about a variety of things. I don't have any clear proposals on this subject though.

thanks for reading.

[thomaspaulb]

@legalsylvain Currently in the two installations where this is used, they have manually created "negative deposit products" that mirror the regular ones. In such a case, it makes sense to make the "automatic" POS order line readonly and locked to the number of products, eg 10 beer always comes with 10 bottles, and if bottles come back, it's going to be a separate order line with 5 "return bottles". This is of course not good for stockkeeping, but in their case they don't care because they've set the containers as consumables and just don't count stock. Though for installations that do keep stock it can work by selling negative amounts of containers on a separate line, I've tested this in 16.0 and it also seems to be possible.

In your opinion should we:

1. Force it in this way, so locking the deposit order line and forcing to use a second one if containers come back
2. Allow a different amount product vs container (though then our customer will complain that their process has changed)
3. Make it a configuration option, so both of us can be happy?

PS. About the *_container_deposit modules, that's interesting. Perhaps they should be integrated. We don't have the budget for it in this project, but later in the year the second customer is migrating so perhaps the discussion can be reopened.

[legalsylvain]

Though for installations that do keep stock it can work by selling negative amounts of containers on a separate line, I've tested this in 16.0 and it also seems to be possible.

I don't understand how you can "add a new negative line". If I click on the consign, it readd a quantity 1 to the existing line. It doesn't create a new line. Or do you use this module https://github.com/OCA/pos/tree/16.0/pos_order_new_line ?
Or I missed something ?

Allow a different amount product vs container (though then our customer will complain that their process has changed)

hum.... you have to in any case. if product Test and Test 2 use the same consign product, quantity of line "Test" / "Test 2" and "consign" are different. Don't you think ?

screenshot of the current runboat :

Sorry If I missed something.

[thomaspaulb]

I don't understand how you can "add a new negative line".

True but i was talking the case where we decide to lock the existing consign product line to readonly state, and make sure that Odoo adds a new line instead? In that case, the newly added line can be made a negative one.

you have to in any case. if product Test and Test 2 use the same consign product, quantity of line "Test" / "Test 2" and "consign" are different.

.... yes, I didn't think of that but you're right, they add up in the same line. Though even then, we can have the both options of A) locking that particular line and having it governed only by the amounts on the product lines that use that line, or B) allowing it to be manually edited. I think the main problem with B is that it requires quite the amount of math in the head of the salesperson in the case where people bring back crates/bottles and it's easy to make mistakes? Anyway, it still remains my question to you whether you would be in favour of A, B or a configuration option deciding between the both.

@hbrunn While testing with two products (say X, Y) using the same consign/deposit product (P) I found bugs though:

1. When product X is added and then product X is added again, it creates an extra line for product X. Not sure why it does that, I would expect Odoo to add to the existing line. In any case, then the deposit line is not increased, probably because it tries to keep the second line in sync with the deposit product line and forgets about the first.
2. When product X is added and product Y is added we then have 2xP. But if I then remove product Y, the full line with 2P is removed. Instead it should reduce by 1

[legalsylvain]

True but i was talking the case where we decide to lock the existing consign product line to readonly state, and make sure that Odoo adds a new line instead? In that case, the newly added line can be made a negative one.

ah. yes. indeed.

Well, for me :

• this module as it, can be merged now or after introduction or extra feature.
• if you introduce "lock" mechanism, it's better if you make it as an option. If no, no problem, I can do it in a second time when I'll use this module in a few monthes.
• Regarding the naming, maybe "pos_container_deposit" could be a better option, so we could imagine to split / make glue module like product_container_deposit, sale_container_deposit and purchase_container_deposit. If we keep "pos_deposit", create "purchase_deposit" will not be possible, because the name is already used. In the same time container_deposit looks different of the other implementation that contains "package_container_deposit".

[denniiiiis]

I agree with tom's suggestion.

• Force it in this way, so locking the deposit order line and forcing to use a second one if containers come back
• Allow a different amount product vs container (though then our customer will complain that their process has changed)
• Make it a configuration option, so both of us can be happy?

Regarding the issue with calculation of number of deposits products X.

At the moment we have in our v8 version;

• per product odoo adds a line of deposit product, which means if product X and Y uses the same deposit product odoo Pos will show (1x product X, 1x deposit 0,25, 2x product Y, 2x deposit 0,25) --> I would prefer this and is if I'm not mistaken common practice with large retailers (Albert Heijn / Jumbo / etc). It is easier to understand the receipt for customers/employees.
• first product, second line deposit
• if you change the amount of products, deposit automatically follows
• if a customer returns a deposit product (sometimes customer only returns deposit and doesn't buy a new product with deposits)

[thomaspaulb]

@denniiiiis So to clarify, your preference would be this:

(...) locking the deposit order line and forcing to use a second one in case containers come back

And not this, unless it's an option that can be switched off:

Allow a different amount product vs container

Then also, your preference would be that each new product that has a matching deposit has their own, paired and separate deposit line, which is locked.

So in the end if people buy 5 milk, 5 cola and also return 3 bottles, it would look like this:

MILK 5
bottle 5
COLA 5
bottle 5
bottle -3

Correct?

[thomaspaulb]

@legalsylvain If we then make it in this way and rename the module, would you add your own process as an option that can be selected, later this year? Or would you perhaps explain your users to prefer this option also (I agree with Dennis that it's both easier for the cashier and clearer for everyone)

[legalsylvain]

@legalsylvain If we then make it in this way and rename the module, would you add your own process as an option that can be selected, later this year? Or would you perhaps explain your users to prefer this option also (I agree with Dennis that it's both easier for the cashier and clearer for everyone)

I'm totally agree with the following solution.

MILK 2
bottle 2
COLA 5
bottle 5
bottle -3

In fact, it will be better :

bottle 2
MILK 2
bottle 5
COLA 5
bottle -3

I mean, when we scan a product, the consign is added before the product added. Otherwise, if the user want to change price, quantity, etc., he will change the price / quantity of the consign, that is not what he want. Don't you think ?

Otherwise, LGTM.

For the renaming, sorry for the extra work. I think that it's better to rename now than once merged in OCA repo.

[denniiiiis]

@legalsylvain, I got your point regarding the order and see the benefits.
But in our case we barely adjust prices of these kind of items and we have the order the other way around.

@thomaspaulb / @hbrunn , maybe we could make it (order) a configuration option as well?

[thomaspaulb]

@legalsylvain

If after scanning MILK, bottle comes automatically but the focus jumps back to MILK and the bottle line is locked / can't be edited, does your order argument still apply?

[legalsylvain]

I think it's OK in that case.
Anyway, hard to know if the cashiers will find it ergonomic.
Maybe ill do some adjustement in a second time.

[hbrunn]

followed up everything here and added js tests

[thomaspaulb]

Should the title of the MR rather be [ADD][16.0] pos_container_deposit, since the 13.0 one was never merged and likely wont be

[thomaspaulb]

works well!

[OCA-git-bot]

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖