[18.0][MIG] sale_automatic_workflow_job: Migration to 18.0 by chaule97 · Pull Request #3460 · OCA/sale-workflow

chaule97 · 2024-12-05T10:47:48Z

Depend on:
- [18.0][MIG] sale_automatic_workflow: Migration to 18.0 #3441
Because this commit separated the module stock from sale_automatic_workflow. I removed everything related to stock.
because _do_validate_sale_order will be run in job, I add _do_send_order_confirmation_mail in job and it will be run after 2 minutes
because queue_job affects test case in sale_automatic_workflow, I have removed tagged in TestAutomaticWorkflow

sebalix · 2025-01-14T09:24:51Z

/ocabot migration sale_automatic_workflow_job

yankinmax

I don't get finally, why don't you replace existing string formatting with f-string. Instead you use old style.

vvrossem · 2025-02-12T12:00:21Z

-
    def _do_sale_done_job_options(self, sale, domain_filter):
-        description = _("Mark sales order {} as done").format(sale.display_name)
+        description = self.env._("Mark sales order %s as done", sale.display_name)


Suggested change

description = self.env._("Mark sales order %s as done", sale.display_name)

description = self.env._(f"Mark sales order {sale.display_name} as done")

@vvrossem FTR using f strings will break translations because the string will be evaluated at run time making the string always different. Ideally we should have named placeholders like _("Foo %(baz)s", baz=self.baz), but is not a blocker.

Other reviewers suggested to use a positional argument if there is only one (as is the case here), and switch to named arguments in case of multiple. I like that one myself, but maybe that's all that it is, a preference?

vvrossem · 2025-02-12T12:06:31Z

 from odoo import fields
-from odoo.tests import tagged

 from .common import TestAutomaticWorkflowMixin, TestCommon


I would adapt TestCommon so it uses BaseCommon instead of TransactionCase

Use BaseCommon as base test class to reduce overhead from tracking and boost the test suite.

gurneyalex · 2025-02-26T09:29:40Z

I'm wondering why we are keeping the cron jobs in this module?

This seems so inefficient, when we could have the queue jobs created on creation of the various records (sale, picking, invoice) based on the configured automatic workflow.

Maybe I'm missing a use case...

[EDIT] use case I'm missing: external event changing something on the record which makes it match the configured domain to trigger the workflow. But this could be handled by an automated action.

simahawk · 2025-02-26T15:26:53Z

I'm wondering why we are keeping the cron jobs in this module?

This seems so inefficient, when we could have the queue jobs created on creation of the various records (sale, picking, invoice) based on the configured automatic workflow.

Maybe I'm missing a use case...

[EDIT] use case I'm missing: external event changing something on the record which makes it match the configured domain to trigger the workflow. But this could be handled by an automated action.

I can give you another use case: having a delay between the creation of the SO and the automatic approval (real customer use case).

IMO it's better to keep the cron because it also helps not to spawn useless jobs and to not add too much overhead on save or confirm or whatever action you want to hook to.

StefanRijnhart · 2025-02-26T16:07:24Z

Agreed. With the cron queuing the jobs, you can plan the load on your system.

simahawk · 2025-04-02T06:58:18Z

/ocabot merge nobump

OCA-git-bot · 2025-04-02T06:58:25Z

What a great day to merge this nice PR. Let's do it!
Prepared branch 18.0-ocabot-merge-pr-3460-by-simahawk-bump-nobump, awaiting test results.

Signed-off-by simahawk

OCA-git-bot · 2025-04-02T07:02:19Z

@simahawk your merge command was aborted due to failed check(s), which you can inspect on this commit of 18.0-ocabot-merge-pr-3460-by-simahawk-bump-nobump.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

Use Queue Jobs to process the Sales Automatic Workflow actions. The default behavior of the automatic workflow module is to use a scheduled action that searches all the record that need a workflow action and sequentially process all of them. It can hit some limits when the number of records is too high. This module keeps the scheduled action to search the records, but instead of directly executing the actions (confirm a sales order, create invoices for a sales order, validate invoices, ...), it creates one job per operation to do. It uses an identity key on the jobs so it will not create the same job for the same record and same operation twice. ~ I needed to extract methods in `sale_automatic_workflow` in order to be able to execute them as jobs. A new decorator "job_auto_delay" (to eventually move in queue_job) makes these methods automatically delayed when called.

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

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

chaule97 · 2025-04-02T07:29:18Z

Ciao @simahawk, can you review and merge again? I have found that module queue_job changed parameter _job_force_sync to queue_job__no_delay

simahawk · 2025-04-07T13:48:14Z

/ocabot merge nobump

OCA-git-bot · 2025-04-07T13:48:21Z

This PR looks fantastic, let's merge it!
Prepared branch 18.0-ocabot-merge-pr-3460-by-simahawk-bump-nobump, awaiting test results.

OCA-git-bot · 2025-04-07T13:57:15Z

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

chaule97 mentioned this pull request Dec 5, 2024

[18.0][ADD] sale_automatic_workflow_stock_job #3461

Merged

chaule97 force-pushed the 18.0-mig-sale_automatic_workflow_job branch from 3e8a3f0 to 1907b62 Compare January 9, 2025 04:00

henrybackman approved these changes Jan 9, 2025

View reviewed changes

OCA-git-bot added this to the 18.0 milestone Jan 14, 2025

OCA-git-bot mentioned this pull request Jan 14, 2025

Migration to version 18.0 #3344

Open

sebalix reviewed Jan 14, 2025

View reviewed changes

Comment thread sale_automatic_workflow_job/models/automatic_workflow_job.py Outdated

chaule97 force-pushed the 18.0-mig-sale_automatic_workflow_job branch from 4aff9e4 to c4d794a Compare January 14, 2025 09:59

chaule97 requested a review from sebalix January 14, 2025 10:03

yankinmax approved these changes Feb 10, 2025

View reviewed changes

OCA-git-bot added the approved label Feb 10, 2025

ivantodorovich approved these changes Feb 10, 2025

View reviewed changes

vvrossem approved these changes Feb 12, 2025

View reviewed changes

OCA-git-bot added a commit that referenced this pull request Apr 2, 2025

Merge PR #3460 into 18.0

806d721

Signed-off-by simahawk

OCA-git-bot added the bot is merging ⏳ label Apr 2, 2025

OCA-git-bot removed the bot is merging ⏳ label Apr 2, 2025

Guewen Baconnier and others added 8 commits April 2, 2025 14:12

[UPD] Update sale_automatic_workflow_job.pot

ad530b4

[UPD] README.rst

bee5759

[ADD] icon.png

6706a7f

[IMP] related_job to origin document

1ebdfdf

[UPD] Update sale_automatic_workflow_job.pot

452793b

[UPD] README.rst

f470333

sale_automatic_workflow_job 12.0.1.0.1

96f5c82

OCA-git-bot and others added 17 commits April 2, 2025 14:12

[UPD] README.rst

d405f90

[IMP] update dotfiles [ci skip]

733b131

[ADD] Initial commit

3935f05

[MIG] sale_automatic_workflow_job: Migration into v15

fd95fad

[UPD] Update sale_automatic_workflow_job.pot

f0ac455

[UPD] README.rst

4d7c540

[MIG] sale_automatic_workflow_job: Migration to 16.0

cff64c2

[UPD] Update sale_automatic_workflow_job.pot

1d3dc85

[UPD] README.rst

204baf8

[UPD] README.rst

63f9f22

Added translation using Weblate (Spanish)

22eef5f

Added translation using Weblate (Italian)

946a1d8

[IMP] sale_automatic_workflow_job: pre-commit auto fixes

4a06f74

[MIG] sale_automatic_workflow_job: Migration to 18.0

002c2b9

chaule97 force-pushed the 18.0-mig-sale_automatic_workflow_job branch from c4d794a to c2fc43d Compare April 2, 2025 07:21

chaule97 added 2 commits April 2, 2025 14:23

[FIX] sale_automatic_workflow: use queue_job__no_delay

ec6f286

[FIX] sale_automatic_workflow_periodicity: use queue_job__no_delay

9893f3b

chaule97 force-pushed the 18.0-mig-sale_automatic_workflow_job branch from c2fc43d to 9893f3b Compare April 2, 2025 07:25

chaule97 requested a review from simahawk April 7, 2025 04:44

OCA-git-bot added the bot is merging ⏳ label Apr 7, 2025

OCA-git-bot merged commit 7905556 into OCA:18.0 Apr 7, 2025

OCA-git-bot added merged 🎉 and removed bot is merging ⏳ labels Apr 7, 2025

	description = self.env._("Mark sales order %s as done", sale.display_name)
	description = self.env._(f"Mark sales order {sale.display_name} as done")

Uh oh!

Conversation

chaule97 commented Dec 5, 2024 • edited by sebalix Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sebalix commented Jan 14, 2025

Uh oh!

Uh oh!

yankinmax left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vvrossem Feb 12, 2025

Choose a reason for hiding this comment

Uh oh!

simahawk Feb 26, 2025

Choose a reason for hiding this comment

Uh oh!

StefanRijnhart Feb 26, 2025

Choose a reason for hiding this comment

Uh oh!

vvrossem Feb 12, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gurneyalex commented Feb 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

simahawk commented Feb 26, 2025

Uh oh!

StefanRijnhart commented Feb 26, 2025

Uh oh!

simahawk commented Apr 2, 2025

Uh oh!

OCA-git-bot commented Apr 2, 2025

Uh oh!

OCA-git-bot commented Apr 2, 2025

Uh oh!

chaule97 commented Apr 2, 2025

Uh oh!

simahawk commented Apr 7, 2025

Uh oh!

OCA-git-bot commented Apr 7, 2025

Uh oh!

OCA-git-bot commented Apr 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

18 participants

chaule97 commented Dec 5, 2024 •

edited by sebalix

Loading

gurneyalex commented Feb 26, 2025 •

edited

Loading