-
Notifications
You must be signed in to change notification settings - Fork 0
OD-1593/queue/sequence groups #2
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
base: 12.0
Are you sure you want to change the base?
Conversation
…37f5cbf38ec8849b0465)
…9bb104a41594a91e026fc9360ef99bbe926607)
| # In practice only _validate_invoice_job is called through automatic.workflow.job | ||
| # This allows the user to select the field on account.invoice, instead of having | ||
| # to pick automatic.workflow.job, which doesn't have the fields of the invoice model. | ||
| if self.model_name == 'automatic.workflow.job': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that I did not add a dependency on this module because this won't break this module. Inheriting the Job class in a custom module for this did not seem reasonable for this kind of change. Albeit, this is a shim.
|
Here's an additional test I did which you can run in a shell. It covers testing automated invoice validation in combination with vanmoof_magento. from odoo.addons.sale_automatic_workflow_job.tests.test_auto_workflow_job import TestAutomaticWorkflowBase, TestAutoWorkflowJob
self = TestAutomaticWorkflowBase
self.env = env
override = {'validate_invoice': False}
workflow = self.create_full_automatic(self=self, override=override)
self.many_sales = [self.create_sale_order(self=self, workflow=workflow) for x in range(5)]
many_sales = self.many_sales[0].browse([r.id for r in self.many_sales])
values = {'street': 'street', 'city': 'city', 'zip': '1234zp', 'country_id': 1, 'phone': '21345613', 'email': 'email@vanmoof.com'}
[so.partner_shipping_id.write(values) for so in many_sales]
# [True, True, True, True, True]
self.progress(self=self)
invoices = self.env['account.invoice'].browse(
many_sales.mapped('invoice_ids').ids
)
# Check related invoices are all draft
invoices.mapped('state')
# ['draft', 'draft', 'draft', 'draft', 'draft']
# Disable tier validation for now
self.env['tier.definition'].search([]).write({'active': False})
self.env.cr.commit()
# Run automatic workflow to validate invoices
[env["automatic.workflow.job"]._do_validate_invoice(invoice) for invoice in many_sales.mapped('invoice_ids')]
self.progress(self=self)
# We can mock the queue job instead if we don't want to commit to our database
self.env.cr.commit()
invoices.mapped('state')
# Wait for all invoices to get validated
invoices.refresh()
invoices.mapped('state')
# ['open', 'open', 'open', 'open', 'open']
# In the logs you should see entries such as:
# odoo.addons.queue_job.jobrunner.channels: job d28819d4-7492-4e4e-84cf-0598cdf8031e re-queued because a job with the same sequence rule 'invoice' is already running in channel root(C:2,Q:1,R:1,F:0)
# which indicates that the jobs with a matching sequence rule were deferred |
461afc6 to
3151ffa
Compare
| else: | ||
| for rule in sequence_rules: | ||
| value = str(record_ids[rule.field_id.name]) | ||
| self.rule_value = value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just asking to get a better grasp on it, still in the process of understanding this flow - in this case the value of a different rule can be set then the rule name set above (first one taken), is this expected or it doesn't matter when we have multiple ones?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only one rule is allowed per model:
queue/queue_job/models/queue_sequence_rule.py
Lines 37 to 41 in 3151ffa
| _sql_constraints = [ | |
| ( | |
| "uniq_model_id", | |
| "UNIQUE(model_id)", | |
| ("Only one rule per model allowed"), |
I left some stubs to allow for multiple rules per model, as I had started with that approach. I mid-way decided not to implement multiple rules per model because it would make the solution more complex than what we need, especially considering not making the job queue any slower.
antonioburic
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tried to get as detailed look as I could, this will be a great way to manage it - really clean and efficient, nice work Kevin!
Do not merge, should be added to buildout once done.
Note: our buildout base for this repo is currently: fc7ea6a
I'll pull our 12.0 branch up to that version so reviewing will be easier.
For reference, the 12.0 branch was on dd05552 before.