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] purchase_request: Migration to 18.0 #2433

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

Conversation

quoc-pn
Copy link
Member

@quoc-pn quoc-pn commented Oct 12, 2024

No description provided.

JordiBForgeFlow and others added 30 commits October 12, 2024 17:03
* Move yml into python test
* Pep8/Flake8
mymage and others added 20 commits October 12, 2024 17:34
Currently translated at 100.0% (229 of 229 strings)

Translation: purchase-workflow-17.0/purchase-workflow-17.0-purchase_request
Translate-URL: https://translation.odoo-community.org/projects/purchase-workflow-17-0/purchase-workflow-17-0-purchase_request/it/
Currently translated at 100.0% (229 of 229 strings)

Translation: purchase-workflow-17.0/purchase-workflow-17.0-purchase_request
Translate-URL: https://translation.odoo-community.org/projects/purchase-workflow-17-0/purchase-workflow-17-0-purchase_request/it/
Currently translated at 100.0% (229 of 229 strings)

Translation: purchase-workflow-17.0/purchase-workflow-17.0-purchase_request
Translate-URL: https://translation.odoo-community.org/projects/purchase-workflow-17-0/purchase-workflow-17-0-purchase_request/it/
Currently translated at 100.0% (229 of 229 strings)

Translation: purchase-workflow-17.0/purchase-workflow-17.0-purchase_request
Translate-URL: https://translation.odoo-community.org/projects/purchase-workflow-17-0/purchase-workflow-17-0-purchase_request/sv/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: purchase-workflow-17.0/purchase-workflow-17.0-purchase_request
Translate-URL: https://translation.odoo-community.org/projects/purchase-workflow-17-0/purchase-workflow-17-0-purchase_request/
Currently translated at 100.0% (228 of 228 strings)

Translation: purchase-workflow-17.0/purchase-workflow-17.0-purchase_request
Translate-URL: https://translation.odoo-community.org/projects/purchase-workflow-17-0/purchase-workflow-17-0-purchase_request/it/
Currently translated at 100.0% (228 of 228 strings)

Translation: purchase-workflow-17.0/purchase-workflow-17.0-purchase_request
Translate-URL: https://translation.odoo-community.org/projects/purchase-workflow-17-0/purchase-workflow-17-0-purchase_request/it/
Currently translated at 100.0% (228 of 228 strings)

Translation: purchase-workflow-17.0/purchase-workflow-17.0-purchase_request
Translate-URL: https://translation.odoo-community.org/projects/purchase-workflow-17-0/purchase-workflow-17-0-purchase_request/it/
@quoc-pn quoc-pn force-pushed the 18.0-mig-purchase_request branch 2 times, most recently from 080e345 to e952f7e Compare October 12, 2024 10:48
@quoc-pn quoc-pn mentioned this pull request Oct 12, 2024
30 tasks
Copy link
Member

@StefanRijnhart StefanRijnhart left a comment

Choose a reason for hiding this comment

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

Please check the tests. There is no product type product anymore. It's now product type consu + is_storable = True.

@StefanRijnhart
Copy link
Member

@quoc-pn Tests still fail. You could try running them locally to see if all is well before you push. There is also a markup warning. I solved a similar warning in https://github.com/OCA/purchase-workflow/pull/2382/files that you could apply here as well.

@@ -104,7 +104,6 @@ def test_procure_purchase_request(self):
self.env["mail.activity"].search(
[("activity_type_id", "=", activity.id)]
).unlink()
activity.unlink()
Copy link
Member

Choose a reason for hiding this comment

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

The comments above these lines say 'An activity is created on the request even if the activity type was deleted'. If we don't delete the activity type anymore, we don't test the original requirement. Did this functionality break?

Copy link
Member Author

@quoc-pn quoc-pn Oct 18, 2024

Choose a reason for hiding this comment

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

This error will be raised when unlinking the activity type.

@api.ondelete(at_uninstall=False)
    def _unlink_except_todo(self):
        if self.env.ref('mail.mail_activity_data_todo') in self:
            raise UserError(_("The 'To-Do' activity type is used to create reminders from the top bar menu and the command palette. Consequently, it cannot be archived or deleted."))

odoo/addons/mail/models/mail_activity_type.py

Copy link
Member

@StefanRijnhart StefanRijnhart Oct 18, 2024

Choose a reason for hiding this comment

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

That is great! That means that the activity type is guaranteed to exist. We can then drop the safeguarding from https://github.com/OCA/purchase-workflow/pull/2433/files#diff-556ada671b2bd79c2398ed16f2f6ad66bc3720e47b87d1ca4c232a367e72a122R44-R47. Can you also remove or adjust the lines of this test that are about the situation in which this particular activity type is not found?

if item.line_id.analytic_distribution:
analytic_account_ids = list(item.line_id.analytic_distribution.keys())
order_line_data.append(
("analytic_distribution", "in", analytic_account_ids)
Copy link
Member

Choose a reason for hiding this comment

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

I think matching the exact distribution, not just overlapping accounts is required.

("analytic_distribution", "in", analytic_account_ids)
)
else:
order_line_data.append(("analytic_distribution", "=", False))
Copy link
Member

Choose a reason for hiding this comment

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

The =? operator evaluates as True if the right hand side is falsy. That means that a request line without analytics can be linked to any order line with or without analytics. Did you mean to change that behaviour?

Copy link
Member Author

Choose a reason for hiding this comment

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

The =? operator doesn't support when searching the field analytic_distribution anymore.

    def _condition_to_sql(self, alias: str, fname: str, operator: str, value, query: Query) -> SQL:
        if fname != 'analytic_distribution' or self.env.context.get('account_report_analytic_groupby'):
            return super()._condition_to_sql(alias, fname, operator, value, query)

        if operator not in ('=', '!=', 'ilike', 'not ilike', 'in', 'not in'):
            raise UserError(_('Operation not supported'))

odoo/addons/analytic/models/analytic_mixin.py

Copy link
Member

Choose a reason for hiding this comment

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

Yes indeed! Do you agree that we should still keep the original behaviour, which means ignoring the analytics of existing purchase lines in case of a request line that has no analytics set?

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

Successfully merging this pull request may close these issues.