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

account_invoice_margin: does not populate purchase_price on install #182

Open
dreispt opened this issue Sep 8, 2023 · 5 comments · Fixed by #202
Open

account_invoice_margin: does not populate purchase_price on install #182

dreispt opened this issue Sep 8, 2023 · 5 comments · Fixed by #202
Labels

Comments

@dreispt
Copy link
Member

dreispt commented Sep 8, 2023

Module

account_invoice_margin

Describe the bug

When installing account_invoice_margin, the purchase_price field is expected to be populated with the product's standard_price, but it stays as zero.
Manually running _compute_purchase_price fixes it, but I can't see why this is not already being done on install - I could be missing something.

To Reproduce

Affected versions: 14.0

Steps to reproduce the behavior:

  1. Install account_invoice_margin
  2. Find an Invoice using a product that has a non zero cost
  3. Check the cost invoice line column: is is zero while it should be the product cost

Expected behavior
the cost invoice line column: is is zero while it should be the product cost

@dreispt dreispt added the bug label Sep 8, 2023
@legalsylvain
Copy link
Contributor

Hi @dreispt. Thanks for your comment.

I've already thought about this, and I haven't found a better solution yet. Here's my train of thought:

  • Idea 1 : Let's populate purchase_price with standard_price when installing module, for existing invoices.

  • BUT : that logic is right for current invoices, but not for past invoices. So you'll have bad margin on past invoices.

  • Idea 2 : Let's populate purchase_price, with values present in product.price.history tables, for past invoices.

  • BUT : product.price.history was not present before version 10.0. That means that for databases that have been migrated from older version. (my current database comes from 5.0 version), the purchase_price will be wrong.

-> my current conclusion on that topic : let's put 0 for existing invoices. it's better to have no margin than to have a false margin and base bad decisions on it.

What do you think ?

@dreispt
Copy link
Member Author

dreispt commented Sep 8, 2023

Help me understand why purchase_price it is not populated in the first place:
The field is added by this module, and it is computed.
Why isn't it populated with the computed value? Because of readonly=False ?

@legalsylvain
Copy link
Contributor

legalsylvain commented Sep 8, 2023

Why isn't it populated with the computed value? Because of readonly=False ?

no. because of this hook :

https://github.com/OCA/margin-analysis/blob/12.0/account_invoice_margin/hooks.py

  • see comment in this commit :
    9fe0f39

Copy link

There hasn't been any activity on this issue in the past 6 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 issue 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 Mar 10, 2024
@dreispt dreispt reopened this Aug 20, 2024
@dreispt
Copy link
Member Author

dreispt commented Aug 20, 2024

@legalsylvain Hello, can you please have a look at #202?

@dreispt dreispt removed the stale PR/Issue without recent activity, it'll be soon closed automatically. label Aug 20, 2024
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 a pull request may close this issue.

2 participants