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

[16.0][FIX] account_payment_sale: Force add payment mode in invoice vals #1268

Open
wants to merge 1 commit into
base: 16.0
Choose a base branch
from

Conversation

ramiadavid
Copy link
Contributor

Since this PR #1246 was added, an error occurs when there are orders without payment_mode_id along with others that do have them when comparing None > int
`TypeError: '<' not supported between instances of 'NoneType' and 'int'

The error occurs here:
https://github.com/odoo/odoo/blob/fe2fb05892e273587996a2f68cd53fc4f8604c6e/addons/sale/models/sale_order.py#L1149-L1156

Fixed by forcing payment_mode_id to be added even if it is empty because False can be compared with integer

@pedrobaeza pedrobaeza changed the title [FIX] [16.0] account_payment_sale: Force add payment mode in invoice vals [16.0][FIX] account_payment_sale: Force add payment mode in invoice vals May 17, 2024
@pedrobaeza pedrobaeza added this to the 16.0 milestone May 17, 2024
@francesco-ooops
Copy link
Contributor

@rousseldenis can you confirm the issue?

@ramiadavid ramiadavid force-pushed the 16.0-fix-account-payment-sale branch from 2669200 to 21380e0 Compare June 26, 2024 18:34
@ramiadavid
Copy link
Contributor Author

@francesco-ooops I have added a test to see the error

@ramiadavid
Copy link
Contributor Author

Pre-commit fails due to another module: account_banking_pain_base

@PicchiSeba
Copy link

@ramiadavid Can you rebase? This might fix the pre-commit error

@ramiadavid ramiadavid force-pushed the 16.0-fix-account-payment-sale branch from 21380e0 to f2a04c8 Compare July 5, 2024 05:30
Copy link

@PicchiSeba PicchiSeba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review: LGTM

@pedrobaeza
Copy link
Member

Although I agree there should be a solution, the intention of no setting such vals is to keep the partner's payment mode in the moment of the invoice creation, so what about assigning such payment mode following the same rules as if you select the partner in the invoice?

@rousseldenis
Copy link
Contributor

@rousseldenis can you confirm the issue?

I'll check but indeed this seems correct.

Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check my comment

@rousseldenis
Copy link
Contributor

Although I agree there should be a solution, the intention of no setting such vals is to keep the partner's payment mode in the moment of the invoice creation, so what about assigning such payment mode following the same rules as if you select the partner in the invoice?

Because payment mode could be set manually on the order side and should be kept. If I have understood your question

@pedrobaeza
Copy link
Member

It's not a direct question, but how to implement the fix.

@pedrobaeza
Copy link
Member

I clarify more: I'm asking for not putting vals["payment_mode_id"] = False, but in case of not having any payment mode in the sales order, to assign the payment mode following this logic:

def _compute_payment_mode_id(self):

which is what happens currently.

@pedrobaeza
Copy link
Member

The other option is to strip out the vals entry before creation if False for letting the computed to act.

This can be more optimal for avoiding to duplicate code.

@rousseldenis
Copy link
Contributor

rousseldenis commented Jul 12, 2024

I clarify more: I'm asking for not putting vals["payment_mode_id"] = False, but in case of not having any payment mode in the sales order, to assign the payment mode following this logic:

def _compute_payment_mode_id(self):

which is what happens currently.

Yes, I've just seen it.

@ramiadavid Could you maybe try to remove the get_payment_mode_vals() function and override the compute methods defined in account_payment_partner instead ?

Copy link

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Nov 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale PR/Issue without recent activity, it'll be soon closed automatically.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants