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

[18.0][MIG] stock_packaging_calculator_packaging_level: Migration to 18.0 #2234

Open
wants to merge 18 commits into
base: 18.0
Choose a base branch
from

Conversation

Kimkhoi3010
Copy link

@Kimkhoi3010 Kimkhoi3010 commented Jan 14, 2025

No description provided.

@Kimkhoi3010 Kimkhoi3010 marked this pull request as draft January 14, 2025 02:49
@Kimkhoi3010 Kimkhoi3010 force-pushed the 18.0-mig-stock_packaging_calculator_packaging_level branch from 74902ab to 689b450 Compare January 14, 2025 02:53
@Kimkhoi3010 Kimkhoi3010 marked this pull request as ready for review January 14, 2025 02:56
@rousseldenis
Copy link
Contributor

/ocabot migration stock_packaging_calculator_packaging_level

@rousseldenis
Copy link
Contributor

@Kimkhoi3010 Thanks for this.

To help reviewers, could you add the link to the depending PR in this one's description ? Thanks

@Kimkhoi3010 Kimkhoi3010 force-pushed the 18.0-mig-stock_packaging_calculator_packaging_level branch from 689b450 to 1b06d8f Compare January 16, 2025 05:15
Copy link

@ajaniszewska-dev ajaniszewska-dev left a comment

Choose a reason for hiding this comment

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

LG

Copy link
Contributor

@jbaudoux jbaudoux left a comment

Choose a reason for hiding this comment

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

The commits history is lacking before 16.0

@Kimkhoi3010 Kimkhoi3010 force-pushed the 18.0-mig-stock_packaging_calculator_packaging_level branch 3 times, most recently from c745ca5 to c4fff18 Compare March 12, 2025 04:28
@Kimkhoi3010 Kimkhoi3010 force-pushed the 18.0-mig-stock_packaging_calculator_packaging_level branch from c4fff18 to c98b378 Compare March 12, 2025 04:47
@Kimkhoi3010 Kimkhoi3010 force-pushed the 18.0-mig-stock_packaging_calculator_packaging_level branch from c98b378 to 3168b87 Compare March 12, 2025 04:48
@Kimkhoi3010 Kimkhoi3010 requested a review from jbaudoux March 12, 2025 04:48
Copy link
Contributor

@jbaudoux jbaudoux left a comment

Choose a reason for hiding this comment

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

Not directly related to migration. Can you put those fixes in a separate commit?

Comment on lines +10 to +11
def _packaging_name_getter(self, packaging):
return packaging.packaging_level_id.name
Copy link
Contributor

Choose a reason for hiding this comment

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

I would not change that as this is called when there is no packaging level

sep = "" if compact_mode else " "
# Override to use packaging level code
if packaging and packaging.packaging_level_id:
name = packaging.packaging_level_id[qty_by_packaging_level_fname]
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
name = packaging.packaging_level_id[qty_by_packaging_level_fname]
name = packaging.packaging_level_id.get(qty_by_packaging_level_fname, "code")

"license": "LGPL-3",
"application": False,
"installable": True,
"auto_install": True,
Copy link
Contributor

@jbaudoux jbaudoux Mar 12, 2025

Choose a reason for hiding this comment

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

I would make it optional. It should be up to the user to choose to display by packaging name or level code

Suggested change
"auto_install": True,
"auto_install": False,

Comment on lines +1 to +3
Glue module for stock_packaging_calculator and product_packaging_type.

Mainly to use packaging type's code instead of packaging's name.
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
Glue module for stock_packaging_calculator and product_packaging_type.
Mainly to use packaging type's code instead of packaging's name.
Modify the string representation of the quantities by packaging. Use the packaging level code instead of the packaging name.
For example, instead of "1 Pallet, 2 Big Box, 7 Box, 10 Units", you get "1PL, 2TU, 7CU, 10 Units"

Copy link
Contributor

@jbaudoux jbaudoux left a comment

Choose a reason for hiding this comment

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

To move to product-attribute and rename into product_packaging....

Following #2173 (comment)

# Copyright 2021 Camptocamp SA
# License LGPL-3.0 or later (https://www.gnu.org/licenses/lgpl)
{
"name": "Stock packaging calculator packaging level",
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
"name": "Stock packaging calculator packaging level",
"name": "Product packaging calculator packaging level",

"application": False,
"installable": True,
"auto_install": True,
"depends": ["stock_packaging_calculator", "product_packaging_level"],
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
"depends": ["stock_packaging_calculator", "product_packaging_level"],
"depends": ["product_packaging_calculator", "product_packaging_level"],

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.

10 participants