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

Merged
merged 220 commits into from
Jun 28, 2024

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
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
Contributor

@rousseldenis rousseldenis May 8, 2024

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
francesco-ooops and others added 17 commits June 21, 2024 15:05
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/
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.
@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@gurneyalex
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 17.0-ocabot-merge-pr-3105-by-gurneyalex-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 4e25bd1 into OCA:17.0 Jun 28, 2024
5 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 834b836. Thanks a lot for contributing to OCA. ❤️

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.