Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[16.0][MOV] sale_by_packaging becomes sell_only_by_packaging #2623

Merged
merged 48 commits into from
Nov 16, 2023

Conversation

QuocDuong1306
Copy link

@QuocDuong1306 QuocDuong1306 commented Jul 28, 2023

This module feature was extracted from the original sale-workflow/sale_by_packaging module.

This module supports 2 features based on sale_by_packaging v14:

  • Sell only by packaging: On product template model, this checkbox restricts
    sales of these products if no packaging is selected on the sale order line.
    If no packaging is selected, it will either be auto-assigned if the quantity
    on the sale order line matches a packaging quantity or an error will be raised.

  • Force sale quantity (on the packaging): force rounds up the quantity during
    creation/modification of the sale order line with the factor set on the packaging.

Change:

  • sell_only_by_packaging is now company dependent

Depend on:

Base on:

grindtildeath and others added 30 commits June 16, 2023 15:58
The functions of this module were first added into sale_order_line_packaging_qty.

After sorting out and extracting the different functions in multiple modules in
the previous commits, this one gets it all together in a single module.

Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
Following warning is shown at runtime:

WARNING odoodb_test odoo.models: method product.template._check_sell_only_by_packaging_can_be_sold_packaging_ids: @constrains parameter 'packaging_ids.can_be_sold' is not a field name

Only fields of the current model can be used in a constrains.

Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
…g SO line create/update

Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
The automatic assignation of a packaging and packaging quantity was done
only for product to `only_sell_by_packaging`
But if possible it should also be done for other products.

Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
When a product must be sold by packaging do not allow for a packaging
qantity at zero or negative.

Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
When the uom quantity of a sale order line is changed for a value that
is not a multiple of possible packaging. The packaging and the packaging
quantity must be reset to an empty value

Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
This is taken from
OCA@290aed9

Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
When a quantity is set on the sale order line before or at the same time
than the product itself, the onchange triggered will replace the line
set quantity with the default size of the package.
This is especially problematic for sales order created programmatically.
So to improve the solution, if the quantity fits with the package
quantity it is kept.

Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
The test was calling the onchanges manually to test their result, but as
some onchanges must be called in cascade to have a correct result, the
test tells more about itself, if it calls the onchanges in the correct
order or not.
Using Form will test them correctly in cascade automatically.

Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
This feature was problematic because for order created programatically
By changing the packaging on product change the quantity ordered would
also change.
The correct packaging will be set on save.
And if not possible an error will be raised by a constraint.

Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
Before this change the packaging was always overridden
hence the user had not control on the packaging to be used on SO line.

Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
When the product_uom_qty is computed from inverse method of packaging qty
the packaging is already there and there's no need to auto-assign.

This also prevents skipping direct writes on prod uom qty or other keys
other than product_packaging.

Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
This check is already done by core sale module.
Tests where not checkingt for that specific exception.

Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
This change allows to be more specific on which product.packaging can be
sold or not.
By default the value from the product.packaging.type is used. But it is
editable afterwards, with this change.

Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
The product_product_packaging_qty on sale.order.line should always be an
integer, no ?

Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
@QuocDuong1306
Copy link
Author

Hi @jbaudoux , I updated

@simahawk
Copy link
Contributor

/ocabot migration sale_by_packaging

@OCA-git-bot OCA-git-bot added this to the 16.0 milestone Oct 25, 2023
@OCA-git-bot OCA-git-bot mentioned this pull request Oct 25, 2023
97 tasks
@simahawk
Copy link
Contributor

@QuocDuong1306 @jbaudoux don't we need a post install hook to migrate data from the old module?

)
def _check_product_packaging_sell_only_by_packaging(self):
for line in self:
if not line.product_id.sell_only_by_packaging:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if not line.product_id.sell_only_by_packaging:
if not line.product_id.sell_only_by_packaging or not line.product_uom_qty:

@QuocDuong1306 IMO there's no point to check lines without any quantity assigned. If we are not able to assign 0 to the product_uom_qty we get stuck in many circumstances (specially when the order is already confirmed).

Copy link
Contributor

@santostelmo santostelmo Oct 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@QuocDuong1306 Thank you for the change

Copy link

@cyrilmanuel cyrilmanuel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not an expert LGTM

@simahawk
Copy link
Contributor

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-2623-by-simahawk-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit dbe620d into OCA:16.0 Nov 16, 2023
5 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 354fa06. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.