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

Add flag to API which allows using pack size #4741

Merged
merged 20 commits into from
May 13, 2023

Conversation

miggland
Copy link
Contributor

@miggland miggland commented May 1, 2023

Intended for use when adding stock items manually, see #4310

This adds a flag use_pack_size to the stock create API, which when set to True, multiplies the given quantity by the pack size of the supplier part before adding stock.

Additionally, a check that the defined SupplierPart exists is added.

@SchrodingersGat
Copy link
Member

@miggland I like this approach! The PR will need to address the following:

Forms - Ensure that the user is provided a 'use_pack_size' checkbox in forms if a supplier part is specified
Unit Tests - Will need some unit tests for the various edge cases / etc
Documentation - Some docs around this feature will be needed as it is quite subtle and might lead to some confusion

@miggland
Copy link
Contributor Author

miggland commented May 1, 2023

@SchrodingersGat: Tests added, and a field is in the form. It's static though, and I need to check that it actually works properly still.

More coming :)

@miggland
Copy link
Contributor Author

miggland commented May 5, 2023

@SchrodingersGat @matmair : Docs added, tests as well.

Regarding this: "checkbox in forms if a supplier part is specified"
I have added a checkbox after supplier. It is always there, but doesn't have any effect unless there is a pack size != 1 defined. Do you really want it to be dynamic? (I'm not sure how this would be best done - however, it requires more calls to the API, which IMO is unnecessary)

@matmair
Copy link
Contributor

matmair commented May 5, 2023

I would consider the check a nice-to-have but @SchrodingersGat is in the lead here

@matmair
Copy link
Contributor

matmair commented May 5, 2023

@miggland would this PR be ready for review or is there something else missing?

@miggland miggland marked this pull request as ready for review May 7, 2023 11:34
@miggland
Copy link
Contributor Author

miggland commented May 9, 2023

@miggland would this PR be ready for review or is there something else missing?

From my POV this is ready for review.

@SchrodingersGat
Copy link
Member

@miggland sorry for the delayed review on this one.

Looks like the "purchase unit price" does not take pack size into account

For example if you buy a reel of wire, with a pack size of 500m and a purchase price of $1000 the final unit purchase price for the wire should be $2/m and not $1000/m

Might need a slight tweak to divide purchase price through by pack quantity

@matmair
Copy link
Contributor

matmair commented May 9, 2023

And maybe a unit test for this case

@miggland
Copy link
Contributor Author

You're right, the issue is fixed. No test (yet)

@SchrodingersGat
Copy link
Member

@miggland thanks, I'll merge once a simple unit test is added :)

@miggland
Copy link
Contributor Author

I've added tests now.

@SchrodingersGat SchrodingersGat added this to the 0.12.0 milestone May 13, 2023
@SchrodingersGat SchrodingersGat added the api Relates to the API label May 13, 2023
@SchrodingersGat
Copy link
Member

@miggland thanks :) Only remaining thing is to increment the API version

@miggland
Copy link
Contributor Author

@miggland thanks :) Only remaining thing is to increment the API version

Of course, forgot about that :) Done

@SchrodingersGat
Copy link
Member

Thanks for the contribution @miggland

@SchrodingersGat SchrodingersGat merged commit 634daa2 into inventree:master May 13, 2023
12 checks passed
@miggland miggland deleted the use-pack-size branch June 1, 2023 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Relates to the API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants