[account] slow invoice cancel because validate for every line; patch included

The patch looks good for me, a similar patch could be applied on 5.0 too.

On Tuesday 22 March 2011, you wrote:
> ** Patch added: "account.patch"
>
> https://bugs.launchpad.net/bugs/740353/+attachment/1930626/+files/account.
> patch

commit 3ed36cad28dfe5b861, 16/06/2010

It's a cat, not a dog.

--
Say NO to spam and viruses. Stop using Microsoft Windows!

Thanks for Reporting!...

Hello Raphaël Valyi,

I have apply your patch and its working fine.
Its fix in lp:~openerp-dev/openobject-addons/trunk-bug-740353-ara branch. It will be merge soon with trunk addons.
Thanks for contribution.

Revision ID: <email address hidden>
Revision No: 4573

Regard,
ara

On Thursday 24 March 2011, you wrote:

> I have apply your patch and its working fine.
> Its fix in lp:~openerp-dev/openobject-addons/trunk-bug-740353-ara branch.
> It will be merge soon with trunk addons. Thanks for contribution.
>

Thank you for applying MY patch.

Apparently, Raphael has discovered them and is now applying under his name.

Hello,

sorry, but just like for many other fixes, I don't see the fix in the 6.0
branch. Is that really merged in the 6.0 branch or what?
If not, why don't use use the "fix committed" status for this bug and all
the others to explicit that it's not merged yet?
Are this bug-fixes meant for merge at all or are you building some secret
time-bomb for v6.0.2 or 6.1 where all will be merged to days before with
nobody testing the integration of the bug fix?
trunk if for new features and 6.0 is for bugfix maintenance, is that right?
Else the 6.0 branch would just not make any sense...
Could you please clarify or is that just a mistake with the status of a few
bugs?

Thanks.

On Fri, Mar 25, 2011 at 12:41 PM, qdp (OpenERP)
<email address hidden>wrote:

> ** Changed in: openobject-addons
> Status: Fix Committed => Fix Released
>
> --
> You received this bug notification because you are a direct subscriber
> of the bug.
> https://bugs.launchpad.net/bugs/740353
>
> Title:
> [account] slow invoice cancel because validate for every line; patch
> included
>
> Status in OpenERP Modules (addons):
> Fix Released
>
> Bug description:
> Try to create a large invoice with like 100 lines and validate it
> (it's slow but that's for an other one).
>
> Now let's cancel it.
> It's kind of slow like hell right?
>
> In production with customer, it takes like 15 minutes to cancel... Not
> very user friendly...
>
> Now let's analyse why.
> Let's put some print statement in the account_move#validate method.
>
> You'll see that instead of passing here once per invoice, we are
> passing here once per invoice line, after we remove every single move
> line. This is stupid, we can do it only once after removing the lines.
>
> This issue is that the generic unlink method from account_move_lines is
> not smart enough to take in consideration the case where all move lines
> belong to the same account move. But this is very common, especially in our
> common cancel invoice use case!
> Given how slow the account_move#validate slow it, we should always try to
> factor the move ids together before calling this validate method, otherwise
> we just do the same slow thing over and over like 100 times in our test
> case.
>
> The included patch just do that. Thank you to apply it, OpenERP users will
> thank you a lot.
> Before our patch: 50 lines invoices would take 15 minutes to cancel; it
> now takes only 10 seconds. Call it an improvement ;-)
>
> To unsubscribe from this bug, go to:
> https://bugs.launchpad.net/openobject-addons/+bug/740353/+subscribe
>

Hello Raphael,

it has been merged into the _trunk_ branch, which is the development branch. Following our bugfix policy, we only fix the launchpad bugs that are related to trunk and if you want some bugfixes to be backported to stable you have to use your OPW contract.

Thanks for your understanding,
Quentin

Hello Quentin,

yes actually I do understand you cannot afford risking regressions on your
customer base if nobody pays you for that. So I'm not shocked.

Now, I think there is a dramatic loss of information in this process because
it's actually impossible to get any immediate clue if a bug is still present
in v6.0 or not.
This leads to a waste of time for all of us: I'm following the bug flow by
email: I quickly read all the around 50 bugs messages every day. Every time
I find some bugs that I think could have some impact, I read the full
tracker thread. I'm happy when I see bug fixed, but actually I have no way
at all to know if I can peace of mind about the bug or not as it is actually
likely to be still present in the 6.0 we deploy (despite we do bzr merge for
our production customers).

So I think it's not ideal: yes I don't expect OpenERP SA to backport bugs
for free. But I think there should be some flag telling if the bug is still
present on the 60 "stable" version or not. We would all save time with this,
OpenERP SA included, it costs nothing do to and it just about not trashing
away some important information that is known at the time you commit our or
your patch. This could even have a positive $ feedback for you: many people
could then easily get a clue the fix has not been backported and use their
OPW for it and even consider buying one for that.

Thoughts?

On Thu, Mar 31, 2011 at 10:17 AM, qdp (OpenERP)
<email address hidden>wrote:

> Hello Raphael,
>
> it has been merged into the _trunk_ branch, which is the development
> branch. Following our bugfix policy, we only fix the launchpad bugs that
> are related to trunk and if you want some bugfixes to be backported to
> stable you have to use your OPW contract.
>
> Thanks for your understanding,
> Quentin
>
> --
> You received this bug notification because you are a direct subscriber
> of the bug.
> https://bugs.launchpad.net/bugs/740353
>
> Title:
> [account] slow invoice cancel because validate for every line; patch
> included
>
> Status in OpenERP Modules (addons):
> Fix Released
>
> Bug description:
> Try to create a large invoice with like 100 lines and validate it
> (it's slow but that's for an other one).
>
> Now let's cancel it.
> It's kind of slow like hell right?
>
> In production with customer, it takes like 15 minutes to cancel... Not
> very user friendly...
>
> Now let's analyse why.
> Let's put some print statement in the account_move#validate method.
>
> You'll see that instead of passing here once per invoice, we are
> passing here once per invoice line, after we remove every single move
> line. This is stupid, we can do it only once after removing the lines.
>
> This issue is that the generic unlink method from account_move_lines is
> not smart enough to take in consideration the case where all move lines
> belong to the same account move. But this is very common, especially in our
> common cancel invoice use case!
> Given how slow the account_move#validate slow it, we should always try to
> factor the move ids together before calling this validate method, otherwise
> we just do the same slow thing over and over like 100 times in our test
>...