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

[FIX] stock_dropshipping: do not merge PO lines coming from a SO #32683

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
5 participants
@len-odoo
Copy link
Contributor

commented Apr 15, 2019

Fine tuning of commit b9e3c15, for the same use-case.
The route may not be passed in values, in which case the bug is still present.
There are two ways to go around that.
We could add the stock.rule in values, which contains the route.
The solution we adopt here is to use the sale_line_id field on the purchase line
If it is present, it was added through _prepare_purchase_order_line,
and thus we want to keep track of the origin.

opw 1957701

Description of the issue/feature this PR addresses:

Current behavior before PR:

Desired behavior after PR is merged:

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@len-odoo len-odoo requested a review from amoyaux Apr 15, 2019

@robodoo robodoo added the seen 🙂 label Apr 15, 2019

@C3POdoo C3POdoo added the OE label Apr 15, 2019

@len-odoo len-odoo force-pushed the odoo-dev:11.0-opw1957701-drop-len branch from dc62d5c to fd80625 Apr 15, 2019

@robodoo robodoo added the CI 🤖 label Apr 15, 2019

@len-odoo len-odoo requested a review from nim-odoo Apr 17, 2019

@nim-odoo
Copy link
Contributor

left a comment

LGTM, but @amoyaux input would be nice as well

@@ -17,7 +17,9 @@ def _prepare_stock_moves(self, picking):
return res

def _merge_in_existing_line(self, product_id, product_qty, product_uom, location_id, name, origin, values):
if values.get('route_ids') and values['route_ids'] == self.env.ref('stock_dropshipping.route_drop_shipping'):
if self.sale_line_id and (self.sale_line_id.id != values.get('sale_line_id', 0)):

This comment has been minimized.

Copy link
@nim-odoo

nim-odoo Apr 17, 2019

Contributor

, 0 is probably not necessary since:

>>> 1 != None
True

I guess (self.sale_line_id.id != values.get('sale_line_id', 0)) is to take into account the modification of a validated SO, right?

This comment has been minimized.

Copy link
@amoyaux

amoyaux Apr 17, 2019

Contributor

you probably won't merge a dropship in an existing line without dropship. I would use

values.get('sale_line_id') and values['sale_line_id'] != self.sale_line_id.id

This comment has been minimized.

Copy link
@len-odoo

len-odoo Apr 18, 2019

Author Contributor

The zero was just to check an integer for an integer, but it is useless indeed.
Your guess is right; the test you added checked that the line value was increased, which was not the case since without that check it would have created another line.

I went for Arnold's version then.

[FIX] stock_dropshipping: do not merge PO lines coming from a SO
Fine tuning of commit b9e3c15, for the same use-case.
The route may not be passed in values, in which case the bug is still present.
There are two ways to go around that.
We could add the stock.rule in values, which contains the route.
The solution we adopt here is to use the sale_line_id field on the purchase line
If it is present, it was added through _prepare_purchase_order_line,
and thus we want to keep track of the origin.

opw 1957701

@len-odoo len-odoo force-pushed the odoo-dev:11.0-opw1957701-drop-len branch from fd80625 to 717b85a Apr 18, 2019

@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Apr 18, 2019

@len-odoo

This comment has been minimized.

Copy link
Contributor Author

commented Apr 18, 2019

robodoo r+

pniederlag pushed a commit to pniederlag/odoo that referenced this pull request Apr 18, 2019

[FIX] stock_dropshipping: do not merge PO lines coming from a SO
Fine tuning of commit b9e3c15, for the same use-case.
The route may not be passed in values, in which case the bug is still present.
There are two ways to go around that.
We could add the stock.rule in values, which contains the route.
The solution we adopt here is to use the sale_line_id field on the purchase line
If it is present, it was added through _prepare_purchase_order_line,
and thus we want to keep track of the origin.

opw 1957701

closes odoo#32683

Signed-off-by: Nans Lefebvre (len) <len@odoo.com>

@robodoo robodoo added merged 🎉 and removed merging 👷 labels Apr 18, 2019

@robodoo

This comment has been minimized.

Copy link
Contributor

commented Apr 18, 2019

Merged, thanks!

@robodoo robodoo closed this Apr 18, 2019

@len-odoo len-odoo deleted the odoo-dev:11.0-opw1957701-drop-len branch Apr 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.