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][FIX] sale_product_pack: keep order lines in order #141

Closed
wants to merge 1 commit into from

Conversation

augusto-weiss
Copy link

@augusto-weiss augusto-weiss commented Aug 4, 2023

The issue:
When add diferent sale orders, packs and their components are added first, and then the other order lines

To reproduce the error:
New sale order
(1) Add a sale order line
(2) Add a sale order line (pack)
Save the sale order

Behavior before this PR:
The sale order will show:
(2) The pack sale order line and components first
(1) The other sale order line

Behavior after this PR:
Hold the order of sale order lines 1 and 2

Untitled_.Aug.4.2023.3_14.PM.webm

@OCA-git-bot
Copy link
Contributor

Hi @ernestotejeda,
some modules you are maintaining are being modified, check this out!

@pedrobaeza pedrobaeza added this to the 16.0 milestone Aug 4, 2023
@pedrobaeza pedrobaeza changed the title [FIX] sale_product_pack: keep order lines in order [16.0][FIX] sale_product_pack: keep order lines in order Aug 4, 2023
@@ -84,8 +84,7 @@ def create(self, vals_list):
line.expand_pack_line()
res |= line
else:
new_vals.append(elem)
res |= super().create(new_vals)
res |= super().create([elem])
Copy link
Member

Choose a reason for hiding this comment

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

This is incorrect, as it's serializing the creation. If you want to keep order, play with other things like sequence, but I don't understand why they aren't properly ordered already. Please include steps to reproduce the problem.

Copy link
Author

Choose a reason for hiding this comment

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

Hi! I added the steps to reproduce error and a video on description

Copy link
Member

Choose a reason for hiding this comment

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

The video is not shown, but as said, the solution for the problem shouldn't be to serialize the creation.

Copy link
Author

Choose a reason for hiding this comment

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

Hi @pedrobaeza I made some changes!


@api.model_create_multi
def create(self, vals_list):
new_vals = []
res = self.browse()
for elem in vals_list:
for idx, elem in enumerate(vals_list):
elem["sequence"] = idx
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 it here? Prepare the sequence on expand_pack_line, summing the main line sequence + an incremental number

Copy link
Author

@augusto-weiss augusto-weiss Aug 7, 2023

Choose a reason for hiding this comment

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

All sale order lines have the same sequence (equal 10).
Editing the component lines also broke the sequence when you have some order lines added.

Example:
sale order line 1 (sequence = 10)
sale order line 2 (sequence = 10) -> pack detailled
sale order line 3 (sequence = 10)

Computed:
sale order line 1 (sequence = 10)
sale order line 2 (sequence = 10) -> pack detailed
Component 1 (sequence = 11)
Component 2 (sequence = 12)
sale order line 3 (sequence = 10)

Output result:
sale order line 1 (sequence = 10)
sale order line 2 (sequence = 10) -> pack detailled
sale order line 3 (sequence = 10)
Component 1 (sequence = 11)
Component 2 (sequence = 12)

Copy link
Author

Choose a reason for hiding this comment

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

Otherside, i must make some change, adding a context because every case have a different behaivour

Copy link
Member

Choose a reason for hiding this comment

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

This is still not correct, as you can make the create of the sales order lines on several batches, so you must check the resequence mechanism implemented in this version, and hook it to respect the relative pack lines orders.

Copy link

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Dec 10, 2023
@github-actions github-actions bot closed this Jan 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale PR/Issue without recent activity, it'll be soon closed automatically.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants