Merge lp://staging/~hirt/ocb-addons/6.1_stock_use_date_always into lp://staging/ocb-addons/6.1

Proposed by Etienne Hirt
Status: Work in progress
Proposed branch: lp://staging/~hirt/ocb-addons/6.1_stock_use_date_always
Merge into: lp://staging/ocb-addons/6.1
Diff against target: 179 lines (+16/-14)
2 files modified
stock/stock.py (+3/-1)
stock/stock_view.xml (+13/-13)
To merge this branch: bzr merge lp://staging/~hirt/ocb-addons/6.1_stock_use_date_always
Reviewer Review Type Date Requested Status
Stefan Rijnhart (Opener) Needs Information
Ronald Portier (Therp) Needs Fixing
Holger Brunn (Therp) Needs Fixing
Review via email: mp+194646@code.staging.launchpad.net

Description of the change

[6.1] Improve Sorting for pickings (always make date valid use date for sorting)

To post a comment you must log in.
6814. By Etienne Hirt

Minor additional fix:
remove restriction to company addresses in incoming deliveries

Revision history for this message
Holger Brunn (Therp) (hbrunn) wrote :

Could you explain #93?

And date and date_expected are different things in my opinion. So we lose data the way you propose.

Actually, I don't even exactly understand the problem you want to fix here. Please file a bug report and elaborate there.

review: Needs Fixing
Revision history for this message
Etienne Hirt (hirt) wrote :

Dear Holger,

Thanks for your analysis and your patience.

I finally added a bug report (Bug #1265608) for explaining the sorting problem with the date_expected vs. date.

Please have a look at the Bug #1265640 for explanation of #93

Revision history for this message
Ronald Portier (Therp) (rportier1962) wrote :

Dear Etienne,

I do not think it is a good idea to have one merge proposal for two changes that are nearly totally unrelated. Except for the fact that they modify the same file. Please put the change of the address selection in a separate proposal.

As for the ordering problem, I think your proposal is quite risky. If you really want to be sure that date is always filled with a valid value - based on date_expected until a move is done -, I feel it would be more appropiate to override create/write methods. Otherwise you might still end up sorting on an date withouth value.

review: Needs Fixing
6815. By Etienne Hirt

remove address restriction change as done in a separate branch

Revision history for this message
Stefan Rijnhart (Opener) (stefan-opener) wrote :

I can see that the 'date' usage is a bit of a mess: a halfhearted way of keeping it synchronized with date_expected is implemented using an on_change method, but there a places where a stock move is created without a value for 'date'. Note that your fix does not solve this either, because you only override the write() method. Biggest problem with this is that it messes up the sorting for existing moves if they don't have a 'date' set (I think that is what Ronald hints at above). In that case it seems better to me to be able to sort by expected date, even for delivered moves. Note that in 7.0, the 'date' field is hidden in the stock move tree views (group_no_one), so that a forward port of this fix would lead to seemingly slightly random orders in the views.

Maybe it is better to identify the places where 'date_expected' is written and 'date' is not kept synchronous. Fix this code. You can then as a user in 6.1 at least click on the 'date' column to see the moves ordered by this field (show the field in a custom module in 7.0), and maybe change the default order in a custom module (in 6.1 and 7.0).

You could also propose the changed order to trunk, see what they say. The gaps in the data could be filled in by a small query in the migration procedure.

What do you think?

review: Needs Information
6816. By Etienne Hirt

commit removed executable flags

6817. By Etienne Hirt

fix indent in stock_move write

Unmerged revisions

6817. By Etienne Hirt

fix indent in stock_move write

6816. By Etienne Hirt

commit removed executable flags

6815. By Etienne Hirt

remove address restriction change as done in a separate branch

6814. By Etienne Hirt

Minor additional fix:
remove restriction to company addresses in incoming deliveries

6813. By Etienne Hirt

make date field readonly in view and set it from expected as long as not in done state
now sort according to date always such that the sorting is always correct independent of state

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches