New review pushed [1]

[1] https://codereview.tryton.org/295401002/#ps298421002

Code review ready for testing.

But this patch set [1] uploaded with upload.py does not include the removed file as the screenshot attached [2] of mercurial state shows. Why? What have I done wrong?


[1] https://codereview.tryton.org/295401002/#ps309901003
[2] mercurial_state.png

PS -> I'm conscious of the disaster of previous patches and comments. That will not occur again since I'm be able to manage partial not ready developments without the need to push to code review tool. Excuse me for that.

Relying on in other issues I think the right state is testing.

Correct me if I am wrong, please.

The issue is not finished but I need some guidance/responses to the code in the review so is for that I set to chatting this issue.

If it is not the right workflow please help to know how I have to do it.

Here is review295401002 which adds the required filters also improve the cancell invoice scenario to test this case.

> Yes, but as far as customer invoices can also be cancelled (when refunded they are marked as cancelled) we should also take them in account. 

> I will not do any distinction here and fix both cases. This means adding the cancel state filter and also exclude "canceled moves" as we did on issue9097

Understood, so in this case, just for information, are you still ensuring it will be needed to update account_eu too?

>> I'm talking all time about canceled (and not refund) **purchase** (from supplier) invoices **not canceled customer invoices**.

Yes, but as far as customer invoices can also be cancelled (when refunded they are marked as cancelled) we should also take them in account. 

I will not do any distintion here and fix both cases. This means adding the cancel state filter and also exclude "canceled moves" as we did on issue9097

@pokoli @ced remember I'm talking all time about canceled (and not refund) **purchase** (from supplier) invoices **not canceled customer invoices**.

On 2020-04-15 10:10, Sergi Almacellas Abellana wrote:
> > I do not see why account_eu should be modified?
> As far as we allow to cancel custom invoices I think it's safer to also exclude the cancelled invoices.

For me, it is a big problem because (except Spanish) I do not know a
country that allow to "update" such report after sending it.
Canceling a customer invoice should always be done with a credit note in
Europe.

> I do not see why account_eu should be modified?
As far as we allow to cancel custom invoices I think it's safer to also exclude the cancelled invoices.

I do not see why account_eu should be modified?

Ok, so we should add a cancel filter on Spanish and European EC Operation list. 

But if we make this filter we should also include the cancel invoices like we did in issue9097

Not exactly...

In issue9097 we're talking about a customer refund so in this case all the invoices/moves must be taken into account.

In this case I'm talking about a cancel (not refund) of purchase invoice which was accounted as mistake.

In other words I think that:

- A cancel purchase invoice must not be taken into account
- A refund purchase invoice must be taken into account

@josesalvador is this similar to issue9097 but for Spanish EC Operation List?

IIUC when you cancel a purchase invoice (eg to correct a purchase invoice accounted as mistake) Tryton cancel it generating a counterpart move.

So IMHO that operation must not be showed in EC Operation List or at least it should be showed as zero amount (although I don't like the last one alternative...)

As long as they have accounting move, for me it is expected.

EC Operation List shows canceled purchase invoices