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 mig base product mass addition #1393

Conversation

aiendry-aktivsoftware
Copy link

Standard migration to version 16.0

Pierrick Brun and others added 30 commits July 24, 2023 15:02
…ss to product.product may still use the feature
Currently translated at 7.1% (1 of 14 strings)

Translation: product-attribute-14.0/product-attribute-14.0-base_product_mass_addition
Translate-URL: https://translation.odoo-community.org/projects/product-attribute-14-0/product-attribute-14-0-base_product_mass_addition/fr/
@aiendry-aktivsoftware aiendry-aktivsoftware force-pushed the 16.0-mig-base_product_mass_addition branch 3 times, most recently from a34dcf0 to 186c213 Compare July 27, 2023 09:37
cr = self.env.cr
Model = self._build_model(self.pool, cr)
Model._log_access = set(LOG_ACCESS_COLUMNS)
return super(ProductProduct, self).write(vals)
Copy link
Contributor

Choose a reason for hiding this comment

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

we should revert the changes done in Model after the write, don't you think ?

Copy link
Member

@bealdav bealdav Oct 16, 2023

Choose a reason for hiding this comment

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

@legalsylvain do you consider changes made by @aiendry-aktivsoftware solve your case ? It seems it's on the way !?

Thanks both for your contributions.

Copy link
Contributor

Choose a reason for hiding this comment

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

well, on that topic, I'm not an expert of the core framework of odoo. (never played modifying technical field like _log_access and rebuild model). Not sure if there are side effect, or if it is safe.
@pedrobaeza : a point of view, here ?

Copy link
Member

Choose a reason for hiding this comment

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

This will have side effects sooner than later for sure, so it should be forbidden.

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks a lot for your analysis.

@aiendry-aktivsoftware aiendry-aktivsoftware force-pushed the 16.0-mig-base_product_mass_addition branch from 186c213 to 3f0c81a Compare July 27, 2023 10:28
@aiendry-aktivsoftware aiendry-aktivsoftware force-pushed the 16.0-mig-base_product_mass_addition branch from 3f0c81a to 936f18a Compare July 28, 2023 08:43
@@ -0,0 +1,2 @@
Adding new implementations would be great:
on sale.order or on stock.picking.batch for instance.
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

(and moved into the description part).

Copy link
Contributor

@legalsylvain legalsylvain left a comment

Choose a reason for hiding this comment

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

Thanks for porting this module !

nipick: Trivial proposal inlines.

question : you adapted the code for V16, removing "modified" orverload by "write" overload. I fear that rebuilding two times the model (Model = self._build_model(self.pool, cr)) can be slow. Did you faced some performance issue.

Model = self._build_model(self.pool, cr)
Model._auto = False
Model._log_access = set(list([]))
res = super(ProductProduct, self).write(vals)
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
res = super(ProductProduct, self).write(vals)
res = super().write(vals)

cr = self.env.cr
Model = self._build_model(self.pool, cr)
Model._auto = False
Model._log_access = set(list([]))
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
Model._log_access = set(list([]))
Model._log_access = set()

is the same, no ?

@@ -0,0 +1,2 @@
Adding new implementations would be great:
on sale.order or on stock.picking.batch for instance.
Copy link
Contributor

Choose a reason for hiding this comment

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

(and moved into the description part).

@@ -26,6 +26,7 @@ class ProductProduct(models.Model):
domain="[('category_id', '=', quick_uom_category_id)]",
compute="_compute_quick_uom_id",
inverse="_inverse_set_process_qty",
store=True,
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: you try to store a field that should be computed on the fly, and not be stored. I guess something is wrong with that design. could you explain that change ? Thanks !

Choose a reason for hiding this comment

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

This is the issue without store=True

tests/test_product_mass_addition.py::TestProductMassAddition::test_quick_line_add
  /odoo/src/odoo/fields.py:808: UserWarning: Field product.product.quick_uom_id in dependency of product.product.quick_uom_category_id should be searchable. This is necessary to determine which records to recompute when uom.uom.category_id is modified. You should either make the field searchable, or simplify the field dependency.

It seems there is some kind of dependency race on this.

However I'm not sure there is a need to have the quick_uom_category_id field since it's not use anywhere in the module and it could be accessed via quick_uom_id.category_id

Copy link

@FranzPoize FranzPoize Oct 18, 2023

Choose a reason for hiding this comment

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

By the way the store=True breaks functionnality on downstream module it sometimes lead to a None valued uom_id on the product

@bealdav
Copy link
Member

bealdav commented Oct 16, 2023

Thanks a lot @legalsylvain for this review.
On my side I need to port sale_quick to 16.0 then it could be a great help if we could go ahead in this current process.
Thanks both

@FranzPoize
Copy link

FranzPoize commented Oct 17, 2023

The test need odoo_test_helper==2.1.1.

And also it seems it's not possible to directly write into metadata anymore, i.e cls.product.write_uid = user_demo does not change the write_uid of the product

@FranzPoize
Copy link

The test need odoo_test_helper==2.1.1.

And also it seems it's not possible to directly write into metadata anymore, i.e cls.product.write_uid = user_demo does not change the write_uid of the product

It's confirmed that we can't change write_id like this anymore. This leads to this code:

        if self._log_access:
            # the superuser can set log_access fields while loading registry
            if not(self.env.uid == SUPERUSER_ID and not self.pool.ready):
                bad_names.update(LOG_ACCESS_COLUMNS)

        # set magic fields
        vals = {key: val for key, val in vals.items() if key not in bad_names}

which remove all LOG_ACCESS_COLUMNS from the written values

@FranzPoize
Copy link

To fix the rest of the text instead of relying on write_uid we could store the write_date and checked if it changed or not

@HaraldPanten
Copy link

Hi @aiendry-aktivsoftware Will you continue this PR?

THX!

@legalsylvain
Copy link
Contributor

hi @aiendry-aktivsoftware. Friendly reminder. could you fix the CI so we can move forward ?

thanks !

Copy link

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label May 26, 2024
@legalsylvain
Copy link
Contributor

Done here : #1512

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale PR/Issue without recent activity, it'll be soon closed automatically.
Projects
None yet
Development

Successfully merging this pull request may close these issues.