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.

Sign up for GitHub

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

Jump to bottom

[14.0][ADD] sale_automatic_workflow_periodicity #2504

Merged
merged 2 commits into from
Oct 18, 2023
Merged

[14.0][ADD] sale_automatic_workflow_periodicity #2504

merged 2 commits into from
Oct 18, 2023

Conversation

TDu
Copy link
Member

@TDu TDu commented May 8, 2023

This module allows to set a frequency at which a workflow is going to be executed.

The frequency of execution can be set in seconds in the workflow configuration. The
next execution time is displayed below it.

@TDu TDu force-pushed the 14-sale-automatic-workflow-frequency branch 3 times, most recently from 775defb to 379aba9 Compare May 9, 2023 01:56
@TDu TDu mentioned this pull request May 9, 2023
Closed
simahawk
simahawk reviewed May 9, 2023
View reviewed changes
Copy link
Contributor

@simahawk simahawk left a comment

Choose a reason for hiding this comment

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

LG

sale_automatic_workflow_frequency/models/sale_workflow_process.py Outdated Show resolved Hide resolved
sale_automatic_workflow_frequency/models/sale_workflow_process.py Outdated Show resolved Hide resolved
sale_automatic_workflow_frequency/models/sale_workflow_process.py Outdated Show resolved Hide resolved
grindtildeath
grindtildeath reviewed May 9, 2023
View reviewed changes
Copy link
Contributor

@grindtildeath grindtildeath left a comment

Choose a reason for hiding this comment

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

Looks good overall, but just nitpicking on the semantics

sale_automatic_workflow_frequency/models/sale_workflow_process.py Outdated Show resolved Hide resolved
@TDu TDu changed the title [14.0][ADD] sale_automatic_workflow_frequency [14.0][ADD] sale_automatic_workflow_periodicity May 10, 2023
sebalix
sebalix reviewed May 10, 2023
View reviewed changes
sale_automatic_workflow_periodicity/models/automatic_workflow_job.py Outdated
sale_workflow.next_execution = datetime.now() + timedelta(
seconds=sale_workflow.periodicity
)
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 Odoo API:

Suggested change
sale_workflow.next_execution = datetime.now() + timedelta(
seconds=sale_workflow.periodicity
)
now = fields.Datetime.now()
sale_workflow.next_execution = fields.Datetime.add(now, seconds=sale_workflow.periodicity)

Copy link
Member Author

Choose a reason for hiding this comment

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

Any reason why ?
As fields is not yet imported.

Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

Because we are in Odoo context

sale_automatic_workflow_periodicity/models/sale_workflow_process.py Outdated
Comment on lines 28 to 30
vals["next_execution"] = fields.Datetime.now() + timedelta(
seconds=periodicity
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same

grindtildeath
grindtildeath approved these changes May 10, 2023
View reviewed changes
Copy link
Contributor

@grindtildeath grindtildeath left a comment

Choose a reason for hiding this comment

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

👍

simahawk
simahawk approved these changes May 11, 2023
View reviewed changes
Copy link
Contributor

@simahawk simahawk left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LG. @TDu up to you if you want to followup w/ Seb's comments

@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). 🤖

@simahawk
Copy link
Contributor

@TDu can you squash? Also you might want to answer @sebalix questions :)

@TDu TDu force-pushed the 14-sale-automatic-workflow-frequency branch from 73dde68 to f81f07b Compare May 15, 2023 11:11
hieulucky111
hieulucky111 approved these changes Jun 2, 2023
View reviewed changes
Copy link

@hieulucky111 hieulucky111 left a comment

Choose a reason for hiding this comment

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

LGTM
Code review

rousseldenis
rousseldenis approved these changes Aug 30, 2023
View reviewed changes
Copy link
Sponsor Contributor

@rousseldenis rousseldenis left a comment

Choose a reason for hiding this comment

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

Code review

@rousseldenis
Copy link
Sponsor Contributor

@TDu Could you just change to fields.Datetime ?

@TDu
Copy link
Member Author

TDu commented Sep 8, 2023

@rousseldenis I did the change asked for.

@TDu TDu force-pushed the 14-sale-automatic-workflow-frequency branch from 7295344 to 6b8c07e Compare September 8, 2023 08:34
@TDu
Copy link
Member Author

TDu commented Sep 8, 2023

Last change reuqested where in a fixup, I just squashed them.

@TDu
Add domain hook on sale_automatic_workflow
be2603e
@TDu
Add sale_automatic_workflow_frequency
c685176 
This module allows to set a frequency at which a workflow is going to be executed.

The frequency of execution can be set in seconds in the workflow configuration. The
next execution time is displayed below it.
@TDu TDu force-pushed the 14-sale-automatic-workflow-frequency branch from 6b8c07e to c685176 Compare September 8, 2023 09:06
@simahawk
Copy link
Contributor

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

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

@OCA-git-bot OCA-git-bot merged commit 39375a9 into OCA:14.0 Oct 18, 2023
11 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at d1badc0. 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
Reviewers

@sebalix sebalix sebalix left review comments

@rousseldenis rousseldenis rousseldenis approved these changes

@simahawk simahawk simahawk approved these changes

@grindtildeath grindtildeath grindtildeath approved these changes

@hieulucky111 hieulucky111 hieulucky111 approved these changes

Assignees
No one assigned
Labels
approved merged 🎉 ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@TDu @OCA-git-bot @simahawk @rousseldenis @grindtildeath @sebalix @hieulucky111