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

[13.0][FIX] purchase_sale_stock_inter_company: Allow reception with serial tracking and partial quantity (Dropshipping) #733

Conversation

carlos-lopez-tecnativa
Copy link

Issue: When handling intercompany dropshipping with serialized products, an error is raised during reception if the quantity received is less than the original purchase order's quantity. The error message states: "Mismatch between move lines with the corresponding."

Steps to Reproduce:

Settings:

In General Settings (both companies):

  • Enable Sales from Purchase Orders.
  • Disable Sales Order Auto Validation.
  • In Purchase Settings: Enable Dropshipping.
  • In Inventory Settings: Enable Multi-Step Routes.

image

Setup:

In Company 1:
Configure a product with vendor information linked to Company 2.
In Company 2:
Configure the same product with vendor information linked to any other vendor (not a company contact).
Scenario:

In Company 1:

  • Create a sales order with a line using the Dropshipping route and the configured product.
  • Confirm the sales order.
  • Navigate to Purchase Orders, find the related record, and confirm it.

Switch to Company 2:

  • Locate the generated sales order and update the sales line to use the Dropshipping route.
  • Set the quantity to 2 and confirm the sales order.
  • Go to Purchase Orders, find the related record, and confirm it.
  • Navigate to Receipts for the purchase order, enter the lot/serial numbers, and attempt to validate the receipt.

Result: An error is raised, preventing the validation of the receipt due to a mismatch between the move lines and their corresponding records.

SO in C1
http://oca-multi-company-13-0-c48386e2793f.runboat.odoo-community.org/web#id=25&action=376&model=sale.order&view_type=form&cids=1&menu_id=224

PO in C1
http://oca-multi-company-13-0-c48386e2793f.runboat.odoo-community.org/web#id=12&action=355&model=purchase.order&view_type=form&cids=&menu_id=206

SO in C2
http://oca-multi-company-13-0-c48386e2793f.runboat.odoo-community.org/web#id=26&action=376&model=sale.order&view_type=form&cids=2&menu_id=224

PO in C2
http://oca-multi-company-13-0-c48386e2793f.runboat.odoo-community.org/web#id=13&action=355&model=purchase.order&view_type=form&cids=&menu_id=206

image

TT51858
@Tecnativa @pedrobaeza could you please review this

@pedrobaeza pedrobaeza added this to the 13.0 milestone Dec 9, 2024
po_picking_pending = purchase.picking_ids.filtered(
lambda x: x.state not in ["done", "cancel"]
)
po_picking_pending.write({"intercompany_picking_id": pick.id})
Copy link
Member

Choose a reason for hiding this comment

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

Although it gets the same result, I consider dot notation cleaner:

Suggested change
po_picking_pending.write({"intercompany_picking_id": pick.id})
po_picking_pending.intercompany_picking_id = pick.id

and why writing this on each loop? It seems a bit weird this structure.

Choose a reason for hiding this comment

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

Sorry, but I don’t understand. I only adapted the current code, but I don’t know the reasons. I think the action_done method can expect many records, and it’s better to iterate in a for loop.

"corresponding PO %s for assigning "
"quantities and lots from %s for product %s"
po_move_lines = po_moves_pending.mapped("move_line_ids")
if move.product_id.tracking == "serial":
Copy link
Member

Choose a reason for hiding this comment

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

Why doing a special case depending on the product tracking instead of just mimicking the stock.move.line?

Choose a reason for hiding this comment

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

Why doing a special case depending on the product tracking instead of just mimicking the stock.move.line?

My error occurs only with serial products, so I isolated the error. In other cases, the check is fine, but for serial products, it’s not.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think so, as lots may have the same problem if you have more than one lot. As said, you should mimic the stock.move.line in one place and the other.

Choose a reason for hiding this comment

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

Done, please update your review

@carlos-lopez-tecnativa carlos-lopez-tecnativa force-pushed the 13.0-purchase_sale_stock_inter_company_with_serial branch from 90be4a2 to 875853a Compare December 9, 2024 11:46
…tracking and partial quantity (Dropshipping)

Issue: When handling intercompany dropshipping with serialized products, an error is raised during reception if the quantity received is less than the original purchase order's quantity. The error message states: "Mismatch between move lines with the corresponding."

Steps to Reproduce:

Settings:

In General Settings (both companies):
Enable Sales from Purchase Orders.
Disable Sales Order Auto Validation.
In Purchase Settings: Enable Dropshipping.
In Inventory Settings: Enable Multi-Step Routes.
Setup:

In Company 1:
Configure a product with vendor information linked to Company 2.
In Company 2:
Configure the same product with vendor information linked to any other vendor (not a company contact).
Scenario:

In Company 1:
Create a sales order with a line using the Dropshipping route and the configured product.
Confirm the sales order.
Navigate to Purchase Orders, find the related record, and confirm it.
Switch to Company 2:
Locate the generated sales order and update the sales line to use the Dropshipping route.
Set the quantity to 2 and confirm the sales order.
Go to Purchase Orders, find the related record, and confirm it.
Navigate to Receipts for the purchase order, enter the lot/serial numbers, and attempt to validate the receipt.
Result: An error is raised, preventing the validation of the receipt due to a mismatch between the move lines and their corresponding records.
@carlos-lopez-tecnativa carlos-lopez-tecnativa force-pushed the 13.0-purchase_sale_stock_inter_company_with_serial branch from 875853a to 5118408 Compare December 10, 2024 16:57
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.

AFAIK, this change is not changing existing stock.move.line to adapt things like the quantity, so it's not a real synchronization like it should be.

@carlos-lopez-tecnativa
Copy link
Author

AFAIK, this change is not changing existing stock.move.line to adapt things like the quantity, so it's not a real synchronization like it should be.

@pedrobaeza I updated the code to synchronize the stock.move.line. Could you review it again? Thanks in advance.

@carlos-lopez-tecnativa carlos-lopez-tecnativa force-pushed the 13.0-purchase_sale_stock_inter_company_with_serial branch from 6b5b806 to d072eb8 Compare December 16, 2024 17:06
…the origin company to the destination company

The synchronization is done only when the picking is in the "Done" state because, before that, the lots are not yet synchronized.
@carlos-lopez-tecnativa carlos-lopez-tecnativa force-pushed the 13.0-purchase_sale_stock_inter_company_with_serial branch from d072eb8 to bc478bf Compare December 16, 2024 17:19
Copy link
Member

@victoralmau victoralmau 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 OK

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@pedrobaeza
Copy link
Member

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 13.0-ocabot-merge-pr-733-by-pedrobaeza-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit d712123 into OCA:13.0 Dec 17, 2024
5 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 1e155d1. Thanks a lot for contributing to OCA. ❤️

@pedrobaeza pedrobaeza deleted the 13.0-purchase_sale_stock_inter_company_with_serial branch December 17, 2024 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants