-
-
Notifications
You must be signed in to change notification settings - Fork 239
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][FIX] stock_picking_report_custom_description: avoid dupe product name #353
[16.0][FIX] stock_picking_report_custom_description: avoid dupe product name #353
Conversation
Hi @carlosdauden, |
@@ -31,6 +31,14 @@ def _get_stock_move_values( | |||
) | |||
if values.get("sale_line_id"): | |||
line = self.env["sale.order.line"].browse(values["sale_line_id"]) | |||
res["description_picking"] = line.name | |||
# Avoid printing double printing the name in the picking |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# Avoid printing double printing the name in the picking | |
# Avoid double printing the name in the picking |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one is pending.
@@ -31,6 +31,14 @@ def _get_stock_move_values( | |||
) | |||
if values.get("sale_line_id"): | |||
line = self.env["sale.order.line"].browse(values["sale_line_id"]) | |||
res["description_picking"] = line.name | |||
# Avoid printing double printing the name in the picking | |||
description_picking = line.name.replace( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replacing unconditionally whole display_name
seems too much, isn't it? Shouldn't it be only removed in case of having something more?
if line != line.product_id.display_name:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's what I do in the line below. And I only replace it in case of matching the pattern f"{product.display_name}\n", not just the display name. That pattern comes from the method
get_product_multiline_description_sale`. It basically goes like this:
name = self.display_name
if self.description_sale:
name += '\n' + self.description_sale
So I just want to look after that issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, not exactly, as you are doing an unconditional replace in any part of the text. Imagine a product called Test
and this SOL line text:
Test
This is a Test
Another line
you will end up with
This is a Another Line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I can improve it with a regex match 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or simply an startswith
plus a replace with just 1 repetition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if my suggestion is faster, but usually everything is faster than regex.
3112e1d
to
2802e89
Compare
By default the sale description is set as f"{product_name}\n{description". If we pass that string as the description_picking, when we print the report, the name will be duplicated. To avoid that effect, we want to trim the product name from the description passed to the picking by default. TT52044
@pedrobaeza Changes done |
2802e89
to
62a9f6b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/ocabot merge patch
On my way to merge this fine PR! |
Congratulations, your PR was merged at ca4c165. Thanks a lot for contributing to OCA. ❤️ |
By default the sale description is set as
f"{product_name}\n{description". If we pass that string as the description_picking, when we print the report, the name will be duplicated. To avoid that effect, we want to trim the product name from the description passed to the picking by default.
cc @Tecnativa TT52044
please review @pedrobaeza @pilarvargas-tecnativa