[11.0][ADD] account_invoice_validation_queued: Enqueue account invoice validation #589

pedrobaeza · 2019-08-29T23:23:15Z

This module allows to enqueue in several jobs the account validation process to be executed in paralell on background, that's it's normally done serially and on foreground.

Each invoice creates a job for performing the validation, but you will only be able to enqueue invoices for the same date.

Installation

This module depends on queue_job module that is hosted on https://github.com/OCA/queue.

Configuration

If you want to see queued jobs, you need "Job Queue / Job Queue Manager" permission in your user.

Usage

Go to Invoicing > Sales > Documents > Customer Invoices or Invoicing > Purchases > Documents > Vendor Bills.
Mark at least one check on the left part of one draft invoice line in the list view.
Click on Action > Confirm Draft Invoice.
On the dialog popup that appears, click on "Enqueue Validation".
If any of the selected invoices are not in draft state, you will receive an error.
If any of the invoices is already enqueued, there will be a message saying so and avoiding to perform the process.
Once enqueued, and having the "Job Queue Manager" permission, you can go to the invoice, and see the tab "Validation Jobs". A list with all the jobs related to that invoice can be found there.

Known issues / Roadmap

Allow to enqueue for validating invoices of different date.
There's a chance that if you perform several enqueues of invoice validations from different dates, the order of the validated invoices, and thus, the number given for it, will result disordered.

cc @Tecnativa

account_invoice_validation_queued/models/account_invoice.py

account_invoice_validation_queued/models/queue_job.py

account_invoice_validation_queued/readme/CONFIGURE.rst

account_invoice_validation_queued/readme/DESCRIPTION.rst

account_invoice_validation_queued/views/queue_job_views.xml

account_invoice_validation_queued/wizards/account_invoice_confirm.py

account_invoice_validation_queued/wizards/account_invoice_confirm_views.xml

pedrobaeza · 2019-09-02T19:46:14Z

Changes done

account_invoice_validation_queued/README.rst

…dation This module allows to enqueue in several jobs the account validation process to be executed in paralell on background, which is normally done serially and on foreground. Each invoice creates a job for performing the validation, but you will only be able to enqueue invoices for the same date. Installation ============ This module depends on *queue_job* module that is hosted on https://github.com/OCA/queue. Configuration ============= Jobs are enqueued in the channel ``root.account_invoice_validation_queued``, so you must adjust your `Odoo configuration <https://github.com/OCA/queue/tree/11.0/queue_job#configuration>`_ according this. If you want to see queued jobs, you need "Job Queue / Job Queue Manager" permission in your user. Usage ===== * Go to *Invoicing > Sales > Documents > Customer Invoices* or *Invoicing > Purchases > Documents > Vendor Bills*. * Mark at least one check on the left part of one draft invoice line in the list view. * Click on *Action > Confirm Draft Invoice*. * On the dialog popup that appears, click on "Enqueue Validation". * If any of the selected invoices are not in draft state, you will receive an error. * If any of the invoices is already enqueued, there will be a message saying so and avoiding to perform the process. * Once enqueued, and having the "Job Queue Manager" permission, you can go to the invoice, and see the tab "Validation Jobs". A list with all the jobs related to that invoice can be found there. Known issues / Roadmap ====================== * Allow to enqueue for validating invoices of different date. * There's a chance that if you perform several enqueues of invoice validations from different dates, the order of the validated invoices, and thus, the number given for it, will result disordered.

pedrobaeza · 2019-09-03T11:44:05Z

/ocabot merge

OCA-git-bot · 2019-09-03T11:44:15Z

This PR looks fantastic, let's merge it!
Rebased to 11.0-ocabot-merge-pr-589-by-pedrobaeza-bump-no, awaiting test results.

Signed-off-by pedrobaeza

OCA-git-bot · 2019-09-03T12:02:56Z

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

PS: Don't worry if GitHub says there are unmerged commits: it is due to a rebase before merge. All commits of this PR have been merged into 11.0.

lmignon

I'm too late but I think that you should take care of my comments...

lmignon · 2019-09-09T12:33:44Z

account_invoice_validation_queued/models/queue_job.py

+class QueueJob(models.Model):
+    _inherit = 'queue.job'
+
+    def cancel(self):


This code should never be there... Moreover it's a duplicate of 6ac79c2#diff-01026d97e6f71aadeb837a145ddac1caR11

Yes, because you can install each module and need the same feature, but there's no problem in having it "duplicated", and this is serving an specific UI purpose (to be able to cancel the job), so no better way to do it that I know of.

lmignon · 2019-09-09T12:36:19Z

account_invoice_validation_queued/wizards/account_invoice_confirm.py

+                    "invoice #%s. Please wait until it's finished or remove "
+                    "it from the selection."
+                ) % invoice.id)
+            new_delay = invoice.sudo().with_delay().action_invoice_open_job()


we lost the traceability of the user at the origin of the invoice confirmation.

Can I put in the user to execute the work somehow?

lmignon · 2019-09-09T12:37:00Z

account_invoice_validation_queued/views/queue_job_views.xml

+<!-- Copyright 2019 Tecnativa - Pedro M. Baeza
+     License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). -->
+<odoo>
+    <record id="view_queue_job_account_invoice_validation" model="ir.ui.view">


This view is a duplicate of 6ac79c2#diff-9478f8f14832f66efde975e04566b2d4R4

It's not the same. That view is primary and it's intended to show the specific fields for queue.job that are interesting to be shown.

Does this mean that if a module has properly extended the tree view for queue_job, its changes will never appear again once this module is installed?

Not really. This view is only intended for showing the jobs attached to an invoice, with the relevant fields in it for an "accountant". List in queue section will be the same as standard, with the same added fields.

lmignon · 2019-09-09T12:41:34Z

account_invoice_validation_queued/wizards/account_invoice_confirm.py

+                    "You can't enqueue invoices with different dates."
+                ))
+        for invoice in invoices:
+            if invoice.validation_job_ids.filtered(


@pedrobaeza Queue job allows you to define a job_identity key to avoid the creation of duplicates. No need of specific code here... OCA/queue@2e86357

But this is not unique... You can validate the invoice several times (you cancel it and validate it again).

OK, good to know. I will change this in a new PR when we clarify all the comments.

Also, jobs must be idempotent: the first thing they should do is verify the state and return if it doesn't match its expectation (e.g. return if the state is already 'open').

By removing this custom code, you will be able to remove the override of action_cancel() that you added on queue.job. I agree with @lmignon that it shouldn't be there. You shouldn't change the behavior of the queue.job model here.

About the idempotent you mentioned, @guewen, I have double checked again, and the method that is call from the job is already handling the cases where the state is different from open, and that's why I prefer to leave them that way, so that the job fails in that case with the exception raised from upstream.

what about changing the user that will execute the job. Is it possible from the with_delay call?

The job is executed as the user delaying the job. However, the user does not need access rights on queue.job because with_delay() creates the record using sudo().

I have double checked again, and the method that is call from the job is already handling the cases where the state is different from open, and that's why I prefer to leave them that way, so that the job fails in that case with the exception raised from upstream.

If the state is already good, the job has nothing to do: it should be done.

The job is executed as the user delaying the job. However, the user does not need access rights on queue.job because with_delay() creates the record using sudo().

Great then, changing the code for removing sudo.

If the state is already good, the job has nothing to do: it should be done.

Well, Odoo thinks the contrary on this, as it raises the exception if you are trying to validate again one validated invoice. What do you think about this, @lmignon ?

Well, Odoo thinks the contrary on this, as it raises the exception if you are trying to validate again one validated invoice

My point is not about odoo's code but about jobs. Jobs are not user-oriented, they are background, low-level representation of "a method running a background" (think celery). Users should not have to deal with them and have to care about them (beside a few advanced users tracking issues). A good functioning system using jobs should have zero jobs failing (in practice it's inevitable to have some but that should be the goal). The thing is you modified the UI to show the job records to users, that's why you have different expectations, but IMO that's a bad idea. The job should only validate the invoice, if it's already validated then it should return and be done; if there is an error during validation, the job should log a message in the invoice's thread and be done. But "normal" users should not have to deal with failed jobs.

We have different vision about this and what the user should know and interact with, but in this case, as the scope is very limited, I'm adding the condition about the state in https://github.com/OCA/account-invoicing/compare/fc5af5be55fcb243fa7ef1500be236415c264959..85444ce0e5d09adcf335059db1f9a78013f50eb4 for #593 for not having the job failing.

pedrobaeza added this to the 11.0 milestone Aug 29, 2019

pedrobaeza added the needs review label Aug 29, 2019

pedrobaeza requested review from sergio-teruel and carlosdauden August 29, 2019 23:23

pedrobaeza force-pushed the 11.0-account_invoice_validation_queued branch 3 times, most recently from 82f2d2c to 2467b91 Compare August 30, 2019 08:21

pedrobaeza mentioned this pull request Aug 31, 2019

[11.0][ADD] sale_order_invoicing_queued: Enqueue sales order invoicing #588

Merged

pedrobaeza force-pushed the 11.0-account_invoice_validation_queued branch from 2467b91 to c68c4ae Compare August 31, 2019 00:46

yajo suggested changes Sep 2, 2019

View reviewed changes

pedrobaeza force-pushed the 11.0-account_invoice_validation_queued branch from c68c4ae to 20acf61 Compare September 2, 2019 19:44

sergio-teruel reviewed Sep 2, 2019

View reviewed changes

account_invoice_validation_queued/README.rst Show resolved Hide resolved

pedrobaeza force-pushed the 11.0-account_invoice_validation_queued branch from 20acf61 to 4869c22 Compare September 2, 2019 22:05

carlosdauden approved these changes Sep 3, 2019

View reviewed changes

sergio-teruel approved these changes Sep 3, 2019

View reviewed changes

OCA-git-bot added a commit that referenced this pull request Sep 3, 2019

Merge PR #589 into 11.0

bf29f2e

Signed-off-by pedrobaeza

OCA-git-bot added the merged 🎉 label Sep 3, 2019

OCA-git-bot closed this Sep 3, 2019

lmignon reviewed Sep 9, 2019

View reviewed changes

pedrobaeza deleted the 11.0-account_invoice_validation_queued branch September 9, 2019 12:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[11.0][ADD] account_invoice_validation_queued: Enqueue account invoice validation #589

[11.0][ADD] account_invoice_validation_queued: Enqueue account invoice validation #589

pedrobaeza commented Aug 29, 2019

pedrobaeza commented Sep 2, 2019

pedrobaeza commented Sep 3, 2019

OCA-git-bot commented Sep 3, 2019

OCA-git-bot commented Sep 3, 2019

lmignon left a comment

lmignon Sep 9, 2019

pedrobaeza Sep 9, 2019

lmignon Sep 9, 2019

pedrobaeza Sep 9, 2019

lmignon Sep 9, 2019

pedrobaeza Sep 9, 2019

lmignon Sep 9, 2019

pedrobaeza Sep 9, 2019

lmignon Sep 9, 2019

pedrobaeza Sep 9, 2019

lmignon Sep 9, 2019

pedrobaeza Sep 9, 2019

guewen Sep 9, 2019 •

edited

pedrobaeza Sep 9, 2019 •

edited

guewen Sep 9, 2019

pedrobaeza Sep 9, 2019

guewen Sep 10, 2019

pedrobaeza Sep 10, 2019

[11.0][ADD] account_invoice_validation_queued: Enqueue account invoice validation #589

[11.0][ADD] account_invoice_validation_queued: Enqueue account invoice validation #589

Conversation

pedrobaeza commented Aug 29, 2019

Installation

Configuration

Usage

Known issues / Roadmap

pedrobaeza commented Sep 2, 2019

pedrobaeza commented Sep 3, 2019

OCA-git-bot commented Sep 3, 2019

OCA-git-bot commented Sep 3, 2019

lmignon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guewen Sep 9, 2019 • edited

Choose a reason for hiding this comment

pedrobaeza Sep 9, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guewen Sep 9, 2019 •

edited

pedrobaeza Sep 9, 2019 •

edited