OpenERP Community Backports (Addons)

Merge lp://staging/~hugosantosred/ocb-addons/6.1-lp1015717 into lp://staging/ocb-addons/6.1

  1. 6.1-lp1015717
  2. Merge into 6.1
Proposed by Hugo Santos (Factorlibre)
Status: Rejected
Rejected by: Stefan Rijnhart (Opener)
Proposed branch: lp://staging/~hugosantosred/ocb-addons/6.1-lp1015717
Merge into: lp://staging/ocb-addons/6.1
Diff against target: 28 lines (+18/-0)
1 file modified
stock/stock.py (+18/-0)
To merge this branch: bzr merge lp://staging/~hugosantosred/ocb-addons/6.1-lp1015717
Reviewer Review Type Date Requested Status
Numérigraphe (community) Disapprove
Stefan Rijnhart (Opener) Needs Fixing
Nhomar - Vauxoo Disapprove
Review via email: mp+159660@code.staging.launchpad.net

Description of the change

Improvement in code for bug lp:1015717.

Now check availability button in stock picking checks if is there any qty of product in the location and splits move in two lines. One with assigned state and one waiting availability.

To post a comment you must log in.
Revision history for this message
Nhomar - Vauxoo (nhomar) wrote :

Hello.

My comments in the bug.

https://bugs.launchpad.net/openobject-server/+bug/996816

IMHO this is not an aceptable patch, because it change totally the flow of work in the picking in the stable version.

This should be done in a simple extra module here:

https://code.launchpad.net/~stock-logistic-core-editors/stock-logistic-flows/7.0

and

https://code.launchpad.net/~stock-logistic-core-editors/stock-logistic-flows/6.1

Functionally speaking what you did in this approach is a "PoV" of how this problem must be solved, not a generic a clear approach, but again it is just my opinion sorry dudes.
Regards.

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

Hi,

thank you Hugo for your work and Nhomar for your comments on the MP and on the bug. I can imagine that the current behaviour of not allowing automatic split is actually what some people will want to have. So I agree with Nhomar that a proper implementation adds a policy on the sale order and maybe consults other settings to govern the behaviour of the Check availability button (and possibly enforces such policies in other places in the stock flow).

Then again, I am not an expert on this topic, so maybe anyone else can comment as well?

review: Abstain (functional)
lp://staging/~hugosantosred/ocb-addons/6.1-lp1015717 updated
6731. By Hugo Santos (Factorlibre)

lp:1015717 Now checks picking move type before splitting moves

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

Funcionally, it seems alright to me now. Nhomar, what do you think?

Technically, I would like to see the context passed in the calls to copy() and write()

review: Needs Fixing
lp://staging/~hugosantosred/ocb-addons/6.1-lp1015717 updated
6732. By Hugo Santos (Factorlibre)

[IMP] Add context to write and copy

Revision history for this message
Numérigraphe (numerigraphe) wrote :

From reading the code I see that you change only the Move's copy, which keeps the procurements running so I think it's fine.
The only thing that bothers me is that this is not consistent with the Procurement Scheduler, which lacks the same ability to split procurements that can only be partially fulfilled. Until then I'd say this would be better treated as a module.

Just nitpicking, but I'd prefer "ctx = context.copy()" above "ctx = dict(context)".
Also, I think " 'location_id': move.location_id.id" is not needed when copying the stock move.

Lionel Sausin.

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

Setting this proposal to rejected due to the conceptual objections that have been raised in this proposal.

Unmerged revisions

6732. By Hugo Santos (Factorlibre)

[IMP] Add context to write and copy

6731. By Hugo Santos (Factorlibre)

lp:1015717 Now checks picking move type before splitting moves

6730. By Hugo Santos (Factorlibre)

[IMP] Split waiting for availability lines in picking one with units available and one with the units left

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