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

[17.0][MIG] sale_order_line_position: Migration to 17.0 #264

Merged
merged 18 commits into from
May 21, 2024

Conversation

xaviedoanhduy
Copy link

@xaviedoanhduy xaviedoanhduy commented Apr 23, 2024

No description provided.

TDu and others added 18 commits April 17, 2024 19:15
This module adds an auto computed position on sale order line.
This position number is added on the report.

The position can be used to keep track of each line during
the delivery, invoicing of the order with the customer.
This is why there is related modules on `account-invoice-reporting`
and `stock-logisics-reporting`.

The position set on a line is not changed when the order is not in
draft anymore.
The previous implementation with the onchange was broken and hardly
feasible (onchange are called on each line and the sequence set on them
changes on each call) and not possible to cover with unit
tests (reproduce the drag and drop).

The other solution could have been to replace the onchange with a
recompute of the position on every `write` of a sale.order but that
also leads to a complex solution.

As the positions are needed to discuss with the customer, the recompute of the
position on print, send and confirm is a good enough, simple solution.
Currently translated at 81.8% (9 of 11 strings)

Translation: sale-reporting-14.0/sale-reporting-14.0-sale_order_line_position
Translate-URL: https://translation.odoo-community.org/projects/sale-reporting-14-0/sale-reporting-14-0-sale_order_line_position/de/
Currently translated at 100.0% (8 of 8 strings)

Translation: sale-reporting-15.0/sale-reporting-15.0-sale_order_line_position
Translate-URL: https://translation.odoo-community.org/projects/sale-reporting-15-0/sale-reporting-15-0-sale_order_line_position/ca/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: sale-reporting-15.0/sale-reporting-15.0-sale_order_line_position
Translate-URL: https://translation.odoo-community.org/projects/sale-reporting-15-0/sale-reporting-15-0-sale_order_line_position/
@gurneyalex
Copy link
Member

/ocabot migration sale_order_line_position

@OCA-git-bot OCA-git-bot added this to the 17.0 milestone Apr 25, 2024
@OCA-git-bot OCA-git-bot mentioned this pull request Apr 25, 2024
10 tasks
Copy link
Member

@angelmoya angelmoya 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 LGTM


def recompute_positions(self):
for sale in self:
if sale.locked_positions or sale.company_id.disable_sale_position_recompute:
Copy link
Member

Choose a reason for hiding this comment

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

maybe could be better with filtered in for

}
sale_pos = {**default_pos, **existing_pos}
for line in vals_list:
if not line.get("display_type"):
Copy link
Member

Choose a reason for hiding this comment

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

same comment, better with filtered

@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). 🤖

@gurneyalex
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 17.0-ocabot-merge-pr-264-by-gurneyalex-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 1298357 into OCA:17.0 May 21, 2024
5 checks passed
@OCA-git-bot
Copy link
Contributor

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

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.