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][MIG] purchase_invoice_new_picking_line #2078

Open
wants to merge 9 commits into
base: 16.0
Choose a base branch
from

Conversation

carmenbianca
Copy link
Member

@carmenbianca carmenbianca force-pushed the 16.0-mig-purchase_invoice_new_picking_line branch 2 times, most recently from 432c860 to 7fd29f4 Compare June 3, 2024 12:11
Copy link
Member Author

@carmenbianca carmenbianca left a comment

Choose a reason for hiding this comment

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

still need to write some tests

Comment on lines 20 to 32
supplierinfo = self.env["product.supplierinfo"]
price_unit = 0.0
if self.partner_id:
supplierinfo = stock_move_id.product_id._select_seller(
partner_id=self.partner_id,
quantity=qty,
date=date,
uom_id=stock_move_id.product_uom,
params={"order_id": self},
)
if supplierinfo:
price_unit = supplierinfo.price
else:
price_unit = stock_move_id.product_id.lst_price
Copy link
Member Author

Choose a reason for hiding this comment

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

I am not entirely sure about this. Previously, the price_unit came from the identically named field on stock.move. This field still exists in 16, but only contains useful data in some corner-cases.

I wrote the quoted snippet under the impression that it should be sufficient. However, upstream in purchase, the code to get the unit price for a purchase.order.line is much much much more complicated. See this permalink. There is no way to easily re-use this code except copy-pasting and adjusting it, but I do not frankly understand some of what is going on there.

I am hopeful that the above will suffice. If not, I see two options:

  1. Copy-paste the complicated linked code. Not great.
  2. Pass the parcel. Instead of creating invoice lines from stock.moves, create purchase.order.lines from stock.moves upon validating stock.pickings. This seems doable, but I'm not sure if it's semantically correct, because these products were never ordered as such.


return data

def add_missing_picking_products_to_account_move(self, account_move_id):
Copy link
Member Author

Choose a reason for hiding this comment

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

This could theoretically be called from purchase_order_change in account.move, which has changed in semantics since Odoo 12. purchase_order is not stored on account.move. Instead, if you add a purchase order in the interface, it 'loads' data from the purchase order. I don't know if purchase_order_change is idempotent—this method is not.

@carmenbianca carmenbianca force-pushed the 16.0-mig-purchase_invoice_new_picking_line branch 2 times, most recently from 2bd2587 to dc67d3f Compare June 3, 2024 13:14
@carmenbianca
Copy link
Member Author

Wrote the test. Ready to review, but also still needs some functional testing.

@carmenbianca carmenbianca marked this pull request as ready for review June 3, 2024 13:22
@carmenbianca carmenbianca force-pushed the 16.0-mig-purchase_invoice_new_picking_line branch from dc67d3f to d649ffd Compare June 3, 2024 13:24
@carmenbianca carmenbianca force-pushed the 16.0-mig-purchase_invoice_new_picking_line branch 2 times, most recently from 2b2c7ea to 12cf2a3 Compare June 3, 2024 15:04
@carmenbianca carmenbianca force-pushed the 16.0-mig-purchase_invoice_new_picking_line branch from 12cf2a3 to edac9dd Compare June 3, 2024 15:21
Copy link
Member

@victor-champonnois victor-champonnois left a comment

Choose a reason for hiding this comment

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

Functional Review OK, thanks !

Copy link

@remytms remytms left a comment

Choose a reason for hiding this comment

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

LGTM

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.

6 participants