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

[11.0][FIX] avoid KeyError #200

Merged
merged 1 commit into from
Feb 5, 2019

Conversation

dcorio
Copy link
Contributor

@dcorio dcorio commented Feb 4, 2019

These lines were breaking other modules inheriting this method with sale_commission installed

@dcorio dcorio changed the title [FIX] avoid KeyError [11.0][FIX] avoid KeyError Feb 4, 2019
@pedrobaeza
Copy link
Member

This is a nonsense... invoice_id is a required field, so no possible to not indicate it.

@dcorio dcorio closed this Feb 4, 2019
@dcorio dcorio deleted the 11.0-FIX-sale_commission-invoice_id branch February 4, 2019 17:05
@dcorio dcorio restored the 11.0-FIX-sale_commission-invoice_id branch February 5, 2019 03:23
@dcorio dcorio reopened this Feb 5, 2019
@dcorio
Copy link
Contributor Author

dcorio commented Feb 5, 2019

@pedrobaeza after having ran a few checks I can say that:

  1. invoice_id on account.invoice.line isn't mandatory
  2. there are modules that first create the account.invoice.line and the account.invoice after
  3. this is the only module so far that breaks the chain

If you still believe this is "nonsense" I'll try to create PR against the other repos with modules that creates account.invoice.lines records without passing invoice_id even if it's not mandatory.

@pedrobaeza
Copy link
Member

Hi @dcorio, I have checked in odoo itself and you're right:

https://github.com/odoo/odoo/blob/64f06858362acf584e4371e3848a6305ecea6b25/addons/account/models/account_invoice.py#L1485

They have removed the required flag from the line, so your PR is correct and apologies from my initial mistake.

@pedrobaeza pedrobaeza added this to the 11.0 milestone Feb 5, 2019
Copy link

@warp10 warp10 left a comment

Choose a reason for hiding this comment

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

LGTM

@pedrobaeza pedrobaeza merged commit 2451f19 into OCA:11.0 Feb 5, 2019
@pedrobaeza
Copy link
Member

Cherry-picked for v12 in 410b28c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants