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

[17.0][MIG] sale_automatic_workflow: Migration to 17.0 #3105

Open
wants to merge 220 commits into
base: 17.0
Choose a base branch
from

Conversation

trisdoan
Copy link
Contributor

@trisdoan trisdoan commented Apr 29, 2024

This change

  • This PR separates the module stock from sale_automatic_workflow

@sebalix
Copy link
Contributor

sebalix commented Apr 30, 2024

@trisdoan thank you. Can you rename your PR [17.0][MIG] sale_automatic_workflow and add the stock separation comment in the PR body instead?

# pylint: disable=W8110
@api.onchange("workflow_process_id")
def _onchange_workflow_process_id(self):
"""Override to add stock related workflow."""
Copy link
Contributor

Choose a reason for hiding this comment

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

Better to use a simple comment to not replace the docstring of the original method

# Override to add stock related workflow

"""Extend to add stock related workflow."""

def create_sale_order(self, workflow, override=None, product_type="product"):
"""Override to create stock operations for each product."""
Copy link
Contributor

Choose a reason for hiding this comment

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

Same, use comment instead of docstring

return order

def create_full_automatic(self, override=None):
"""Override to include default stock related values."""
Copy link
Contributor

Choose a reason for hiding this comment

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

Here too

@trisdoan trisdoan changed the title [17.0][REF] sale_automatic_workflow|_stock: separate [17.0][MIG] sale_automatic_workflow: Migration to 17.0 May 2, 2024
@trisdoan trisdoan force-pushed the imp-sale_automatic_workflow branch from 7e5b798 to f34c352 Compare May 2, 2024 02:33
@trisdoan trisdoan requested a review from sebalix May 2, 2024 02:33
@trisdoan
Copy link
Contributor Author

trisdoan commented May 2, 2024

Hello @sebalix, I updated as you suggest 🙏

@rousseldenis
Copy link
Sponsor Contributor

/ocabot migration sale_automatic_workflow

@OCA-git-bot
Copy link
Contributor

The migration issue (#2766) has not been updated to reference the current pull request because a previous pull request (#2826) is not closed.
Perhaps you should check that there is no duplicate work.
CC @sanderlienaerts

@@ -44,8 +55,6 @@ def _onchange_workflow_process_id(self):
if not self.workflow_process_id:
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

@trisdoan @sebalix Maybe it's time to change the field to computed/stored/readonly=False one to remove onchanges.

UPDATE: In fact, you should override the _compute_team_id() method and to add 'workflow_process_id' in depends.

Copy link
Contributor Author

@trisdoan trisdoan May 14, 2024

Choose a reason for hiding this comment

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

Hello @rousseldenis @sebalix, I updated as you suggest

I added this commit.
I adapt field picking_policy in sale.order to writable computed field. Unfortunately, there is a module: sale_order_type, which does the same thing but depends on field 'type_id' instead.
-> So one solution is to mark sale_order_type as rebel module. What do you guys think?

Copy link
Contributor

@sebalix sebalix May 22, 2024

Choose a reason for hiding this comment

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

Not sure it's a good idea 🤔
What you could do is to extract the @api.depends values in a method and make use of hasattr like it's done in the compute method:

# in sale_order_type
def _depends_picking_policy(self):
    depends = []
    if hasattr(super(), "_depends_picking_policy"):
        depends = super()._depends_picking_policy()
    depends.append("type_id")
    return depends

@api.depends(lambda self: self._depends_picking_policy())
def _compute_picking_policy():
    [...]
# in sale_automatic_workflow_stock
def _depends_picking_policy(self):
    depends = []
    if hasattr(super(), "_depends_picking_policy"):
        depends = super()._depends_picking_policy()
    depends.append("workflow_process_id")
    return depends

@api.depends(lambda self: self._depends_picking_policy())
def _compute_picking_policy():
    [...]

This way we are dynamically compatible with both modules installed (and probably still with an undefined behavior, but that's better than something else I guess).

Copy link
Contributor

Choose a reason for hiding this comment

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

@trisdoan any news about this?

Copy link
Contributor Author

@trisdoan trisdoan May 27, 2024

Choose a reason for hiding this comment

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

commit is coming today. Will ping you when it's done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello @sebalix, updated as you suggest + tests are green 🙏

@trisdoan trisdoan marked this pull request as draft May 12, 2024 05:55
@trisdoan
Copy link
Contributor Author

DRAFT: working on failed tests

@trisdoan trisdoan force-pushed the imp-sale_automatic_workflow branch 9 times, most recently from d61452d to 028edd7 Compare May 14, 2024 04:52
@trisdoan trisdoan marked this pull request as ready for review May 14, 2024 05:03
jcoux and others added 27 commits May 27, 2024 14:36
Parts of `sale_automatic_workflow_payment_mode` have been merged into `sale_automatic_workflow`.
But these parts have been not removed from `sale_automatic_workflow_payment_mode`.

This PR clean both modules after this merge.

Originals PR of the merge: OCA#1789

(forwrd port of c8d59ed)
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: sale-workflow-15.0/sale-workflow-15.0-sale_automatic_workflow
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-15-0/sale-workflow-15-0-sale_automatic_workflow/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: sale-workflow-16.0/sale-workflow-16.0-sale_automatic_workflow
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_automatic_workflow/
Currently translated at 96.4% (54 of 56 strings)

Translation: sale-workflow-16.0/sale-workflow-16.0-sale_automatic_workflow
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_automatic_workflow/es/
Currently translated at 19.6% (11 of 56 strings)

Translation: sale-workflow-16.0/sale-workflow-16.0-sale_automatic_workflow
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_automatic_workflow/it/
Currently translated at 21.4% (12 of 56 strings)

Translation: sale-workflow-16.0/sale-workflow-16.0-sale_automatic_workflow
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_automatic_workflow/it/
Currently translated at 50.0% (28 of 56 strings)

Translation: sale-workflow-16.0/sale-workflow-16.0-sale_automatic_workflow
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_automatic_workflow/sl/
Currently translated at 21.4% (12 of 56 strings)

Translation: sale-workflow-16.0/sale-workflow-16.0-sale_automatic_workflow
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_automatic_workflow/it/
Currently translated at 28.5% (16 of 56 strings)

Translation: sale-workflow-16.0/sale-workflow-16.0-sale_automatic_workflow
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_automatic_workflow/it/
Currently translated at 100.0% (56 of 56 strings)

Translation: sale-workflow-16.0/sale-workflow-16.0-sale_automatic_workflow
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_automatic_workflow/es/
Currently translated at 62.5% (35 of 56 strings)

Translation: sale-workflow-16.0/sale-workflow-16.0-sale_automatic_workflow
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_automatic_workflow/de/
Currently translated at 98.2% (55 of 56 strings)

Translation: sale-workflow-16.0/sale-workflow-16.0-sale_automatic_workflow
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_automatic_workflow/pt_BR/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: sale-workflow-16.0/sale-workflow-16.0-sale_automatic_workflow
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_automatic_workflow/
Currently translated at 75.0% (42 of 56 strings)

Translation: sale-workflow-16.0/sale-workflow-16.0-sale_automatic_workflow
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_automatic_workflow/de/
Currently translated at 100.0% (56 of 56 strings)

Translation: sale-workflow-16.0/sale-workflow-16.0-sale_automatic_workflow
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_automatic_workflow/es/
Currently translated at 100.0% (56 of 56 strings)

Translation: sale-workflow-16.0/sale-workflow-16.0-sale_automatic_workflow
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_automatic_workflow/es/
@trisdoan trisdoan force-pushed the imp-sale_automatic_workflow branch from 028edd7 to 528d46f Compare May 27, 2024 09:02
There is an issue for service companies that do not need the stock
module to be installed when using `sale_automatic_workflow`. Module
depended on `stock`, therefore it was auto installed. Now, stock related
part is moved to a separate module - `sale_automatic_workflow_stock` and
can be installed separately.
@trisdoan trisdoan force-pushed the imp-sale_automatic_workflow branch from 528d46f to d255560 Compare May 27, 2024 09:12
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