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

[FIX] Make check in quant level so that it works in manufacturing as well #256

Closed
wants to merge 2 commits into from

Conversation

levkar
Copy link

@levkar levkar commented Sep 10, 2016

This is simpler way to have the same functionality and works well with any kind of move (such as manufacturing moves).

def _quant_create(self, qty, move, lot_id=False, owner_id=False,
src_package_id=False, dest_package_id=False,
force_location_from=False, force_location_to=False):
if move.product_id.check_no_negative \
Copy link
Member

Choose a reason for hiding this comment

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

You are not checking here if qty is negative.

Copy link
Author

Choose a reason for hiding this comment

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

_quant_create is called when negative otherwise _quant_split is called.

yes it is not self explanatory but it works like a charm.

Copy link
Member

Choose a reason for hiding this comment

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

OK, I see, please document it then properly, indicating that always that this method is called and the location is internal, a negative quant is created.

@pedrobaeza
Copy link
Member

You have to rebase your branch.

@levkar
Copy link
Author

levkar commented Sep 10, 2016

Yes I admit, I am very bad at using git.

@pedrobaeza
Copy link
Member

I have checked today that _quant_create is also called when receiving materials, so you must check quantity variable

@pedrobaeza
Copy link
Member

Why don't you simply use the approach on #137?

@levkar
Copy link
Author

levkar commented Sep 23, 2016

move.location_id.usage == 'internal' prevents it in material receive, but I agree with you that #137 seems to be a cleaner solution.

stock_no_negative and stock_disallow_negative both doing the same thing. Do you think it is a good idea to copy the same thing from stock_disallow_negative into stock_no_negative? I know it is not a good idea to have two modules doing the same thing but perhaps it's better to remove stock_no_negative and use stock_disallow_negative.

Let me know what you think so that i can proceed.

@pedrobaeza
Copy link
Member

The other PR can be disregarded with this one. They serve the same purpose, so better to include that here that is already in the repository.

@Cedric-Pigeon
Copy link

@levkar Hello, do you still plan to work on this PR ? Is it still useful ?

@levkar
Copy link
Author

levkar commented May 30, 2017

3 PR's are very related,

  1. this one (we are in production with this and very happy with it)
  2. PR [IMP] No negative by default #258 (see pedro's comment) (i get lost in the conversation)
  3. PR 8.0 Add module stock_disallow_negative #137 (very similar solution and PR [IMP] No negative by default #258 is included by design)

If you can decide on how to proceed I would be happy to help. Hopefully we can close all three PR's.

@levkar
Copy link
Author

levkar commented Jun 29, 2017

I am closing this PR due to lack of interest on this module and existing alternatives #137.

A backport of #275 would be a better choice anyway.

@levkar levkar closed this Jun 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants