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.

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

[FIX] account: `account.register.payments` model doesn't has attribute `has_invoices` #32847

Open
wants to merge 1 commit into
base: 12.0
from

Conversation

Projects
None yet
7 participants
@hbto
Copy link
Contributor

commented Apr 19, 2019

As _compute_journal_domain_and_types method is defined in an
abstract model and has_invoices field is defined in account.payment model

There is a case where strangely _compute_journal_domain_and_types is called in the abstract.register.payment itself and the model does not has attributeerror is raised.

[IMP] As `_compute_journal_domain_and_types` method is defined in an
abstract model it could be called by other models which could not bear
`has_invoices` field.
@moylop260
Copy link
Contributor

left a comment

@nim-odoo @mart-e
What do you think about?
Should we add this field to abstract class or use this validation or fix it from custom module?

@sswapnesh

This comment has been minimized.

Copy link
Contributor

commented Apr 21, 2019

Fixes #31411

(it seems it was handled in related OPW and closed)

@moylop260

This comment has been minimized.

Copy link
Contributor

commented Apr 22, 2019

@hbto
Could you check the opw answer, please?

cc @luisg123v

@sswapnesh
Thanks!

@luisg123v

This comment has been minimized.

Copy link
Contributor

commented Apr 22, 2019

@moylop260
Summarizing, The opw answer was "yes, it's an issue, we will solve it, but it's not a priority, it will be replaced by an error message".

@hbto hbto changed the title account: [FIX] `account.register.payments` model doesn't has attribute `has_invoices` [FIX] account: `account.register.payments` model doesn't has attribute `has_invoices` Apr 22, 2019

@qdp-odoo

This comment has been minimized.

Copy link
Contributor

commented Apr 24, 2019

although I hardly see the use case where registering payments of 0 is needed, I could agree to merge this PR if you'd improve the commit message to respect our guidelines.

Also, I'd like to emphasize that, whatever the use case is, it won't be possible anymore in saas-12.3 since you won't be able to select the amount when registering payments from the invoice tree view (as it will create a payment per invoice with the residual amount for each).... so I'm not sure it's a good idea to help you with this "feature" (?) while we're going to remove it from you anyway in the future.

@nhomar

This comment has been minimized.

Copy link
Collaborator

commented Apr 24, 2019

@qdp-odoo

I think we are ok managing this in the future versions differently, but this patch is necessary I think, basically it is an anoying error/traceback received.

@hbto take care of the comment for the future/present (**maybe this will need an inmediate review once forward ported).

thanks in advance.

@hbto

This comment has been minimized.

Copy link
Contributor Author

commented Apr 24, 2019

@qdp-odoo

you won't be able to select the amount when registering payments from the invoice tree view (as it will create a payment per invoice with the residual amount for each)

This statement of your is quite scary and seems to me as a setback in the way of making things flexible for end users and developers.

Just a comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.