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][ADD] product_packaging_multi_barcode_stock_menu: view packaging on barcode list #541

Open
wants to merge 4 commits into
base: 16.0
Choose a base branch
from

Conversation

santostelmo
Copy link
Contributor

Show packaging field in product barcode tree view

Copy link

@sebalix sebalix left a comment

Choose a reason for hiding this comment

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

Cannot do that, product.barcode.packaging_id field is part of product_packaging_multi_barcode module, so this doesn't respect the inheritance.

@sebalix sebalix changed the title [IMP] product_multi_barcode_stock_menu tree view packaging [16.0][IMP] product_multi_barcode_stock_menu tree view packaging Nov 20, 2023
@santostelmo
Copy link
Contributor Author

product_packaging_multi_barcode

@sebalix Should I make product_multi_barcode_stock_menu dependent of product_packaging_multi_barcode ?

@sebalix
Copy link

sebalix commented Nov 20, 2023

@santostelmo no we cannot do that neither sadly. I think the way it has been designed, we have to create a product_packaging_multi_barcode_stock_menu module to handle this need.
I know its overkill but I do not see another solution.

In the long term, as both product and packaging are part of Odoo std (product module), I'm wondering why product_multi_barcode and product_packaging_multi_barcode couldn't be merge in one module (you install it and you get all your models coming from product extended with this multi barcodes feature), so we could have only one extra module that make the glue between product_multi_barcode with stock (without restricting this to a menu, so a better name would have been product_multi_barcode_stock for instance).
But I don't know these modules nor why it has been designed this way, so for now we have to stick with what we have.

cc @simahawk

@santostelmo santostelmo force-pushed the 16.0-imp-product_multi_barcode_stock_menu-view-packaging branch from 0a3565e to 06f1912 Compare November 21, 2023 13:01
@santostelmo
Copy link
Contributor Author

@santostelmo no we cannot do that neither sadly. I think the way it has been designed, we have to create a product_packaging_multi_barcode_stock_menu module to handle this need. I know its overkill but I do not see another solution.

In the long term, as both product and packaging are part of Odoo std (product module), I'm wondering why product_multi_barcode and product_packaging_multi_barcode couldn't be merge in one module (you install it and you get all your models coming from product extended with this multi barcodes feature), so we could have only one extra module that make the glue between product_multi_barcode with stock (without restricting this to a menu, so a better name would have been product_multi_barcode_stock for instance). But I don't know these modules nor why it has been designed this way, so for now we have to stick with what we have.

cc @simahawk

@sebalix I have created the module product_packaging_multi_barcode_stock_menu

@santostelmo santostelmo changed the title [16.0][IMP] product_multi_barcode_stock_menu tree view packaging [16.0][ADD] product_multi_barcode_stock_menu-view-packaging tree view packaging Nov 21, 2023
@santostelmo santostelmo changed the title [16.0][ADD] product_multi_barcode_stock_menu-view-packaging tree view packaging [16.0][ADD] product_packaging_multi_barcode_stock_menu-view-packaging tree view packaging Nov 21, 2023
@santostelmo santostelmo changed the title [16.0][ADD] product_packaging_multi_barcode_stock_menu-view-packaging tree view packaging [16.0][ADD] product_packaging_multi_barcode_stock_menu-view packaging on barcode list Nov 21, 2023
@santostelmo santostelmo force-pushed the 16.0-imp-product_multi_barcode_stock_menu-view-packaging branch 4 times, most recently from 0fa8585 to 30cd8b2 Compare November 21, 2023 13:36
Copy link

@sebalix sebalix left a comment

Choose a reason for hiding this comment

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

Thank you, would like to hear another opinion about this though, as I do not know these modules a lot

product_packaging_multi_barcode_stock_menu/__manifest__.py Outdated Show resolved Hide resolved
product_packaging_multi_barcode/README.rst Outdated Show resolved Hide resolved
@QuocDuong1306
Copy link

Ping @santostelmo , Could you help to fix this?

@santostelmo santostelmo force-pushed the 16.0-imp-product_multi_barcode_stock_menu-view-packaging branch from 30cd8b2 to cc0f210 Compare January 5, 2024 06:51
@santostelmo
Copy link
Contributor Author

@QuocDuong1306 I fixed the points mentioned above. @sebalix could you please check your change requests ?

Copy link

@sebalix sebalix left a comment

Choose a reason for hiding this comment

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

LG, can you add (in a separate commit) a ROADMAP.rst entry in product_packaging_multi_barcode module to merge it in product_multi_barcode for the next migration, and same for product_packaging_multi_barcode_stock_menu + product_multi_barcode_stock_menu to merge them in a new module product_multi_barcode_stock.

At the end we'll get only two modules, one depending exclusively on product, the other one being a glue module extending the first by adding a dependency on stock.

@sebalix sebalix changed the title [16.0][ADD] product_packaging_multi_barcode_stock_menu-view packaging on barcode list [16.0][ADD] product_packaging_multi_barcode_stock_menu: view packaging on barcode list Jan 5, 2024
@sebalix sebalix requested a review from simahawk January 5, 2024 12:44
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 but LGTM

@cyrilmanuel cyrilmanuel force-pushed the 16.0-imp-product_multi_barcode_stock_menu-view-packaging branch from 2c4ec04 to 5b37b16 Compare April 3, 2024 06:30
@cyrilmanuel cyrilmanuel force-pushed the 16.0-imp-product_multi_barcode_stock_menu-view-packaging branch from 5b37b16 to 13fefcf Compare April 3, 2024 06:34
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.

None yet

7 participants