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.

Sign up for GitHub

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

Jump to bottom

Part units #4854

Merged
merged 30 commits into from
May 26, 2023
Merged

Part units #4854

merged 30 commits into from
May 26, 2023

Conversation

SchrodingersGat
Copy link
Member

@SchrodingersGat SchrodingersGat commented May 19, 2023 •
edited

Closes #4829

This PR changes the "units" field for a part to be a physical unit (e.g. "m")

Additionally, supplier parts can be specified in compatible units (e.g "feet"), and converted to base units as appropriate

TODO

  • Create custom units for "pieces" / "each" / etc
  • Unit testing for new custom units
  • Replace all references to old pack_size variable
  • Fix existing unit tests
  • Add unit tests for converting between unit types (i.e. "10 feet" -> "m")
  • Add unit tests to ensure supplier part pricing is correctly converted to base units
  • Add background task to ensure that supplier part units are updated when the parent part changes
  • Documentation
  • Add "native_pack_units" field to the SupplierPart model (which is calculated in base units, for queryset annotations)
  • Data migration for demo dataset - Supplier part units demo-dataset#46
  • Data migration for converting exiting "part.units" to physical units
  • Unit test for above data migration
  • Data migration for converting any existing "pack_size" fields to new units
  • Unit test for above data migration
  • Ensure that existing forms which use supplier part quantity still work as expected
  • Ensure that pricing calculations are correctly considered when using pack_quantity

@SchrodingersGat SchrodingersGat added enhancement This is an suggested enhancement or new feature part Related to Part models labels May 19, 2023
@SchrodingersGat SchrodingersGat added this to the 0.12.0 milestone May 19, 2023
matmair
matmair approved these changes May 19, 2023
View reviewed changes
Copy link
Contributor

@matmair matmair left a comment

Choose a reason for hiding this comment

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

Would it make sense to introduce a unit 'each' or 'piece'/'pc' for a quantity of 1? I have seen that in other PLMs/ERPs

@SchrodingersGat
Copy link
Member Author

Would it make sense to introduce a unit 'each' or 'piece'/'pc' for a quantity of 1? I have seen that in other PLMs/ERPs

Yep, that makes sense. There's an easy entry point as I've already defined a hook for creating a custom UnitRegistry

@SchrodingersGat
Improve "hook" for loading custom unit database
5268288
@SchrodingersGat
Display part units column in part table
c118227 
- Also allow ordering by part units
- Allow filtering to show parts which have defined units
@SchrodingersGat
Adds data migration for converting units to valid values
6382e06
@SchrodingersGat
Add "pack_units_native" field to company.SupplierPart model
5226f1e
@SchrodingersGat
Clean pack units when saving a SupplierPart
c078831 
- Convert to native part units
- Handle empty units value
- Add unit tests
@SchrodingersGat
Add background function to rebuild supplier parts when a part is saved
6c363a0 
- Required to ensure that the "pack_size_native" is up to date
@SchrodingersGat
Template updates
92095d7
@SchrodingersGat
Sort by native units first
4c5800b
@SchrodingersGat
Bump API version
532f592
@SchrodingersGat
Rename "pack_units" to "pack_quantity"
4fcdbca
@SchrodingersGat
Update migration file
acf195f 
- Allow reverse migration
@SchrodingersGat
Fix for currency migration
b1bf086 
- Handle case where no currencies are provided
- Handle case where base currency is not in provided options
@SchrodingersGat
Adds unit test for data migration
1482f70
@SchrodingersGat
Add unit test for part.units data migration
b43e560 
- Check that units fields are updated correctly
@SchrodingersGat
Add some extra "default units"
ec728a3 
- each / piece
- dozen / hundred / thousand
- Add unit testing also
@SchrodingersGat
Update references to "pack_size"
d79d854 
- Replace with "pack_quantity" or "pack_quantity_native" as appropriate
matmair
matmair approved these changes May 23, 2023
View reviewed changes
Copy link
Contributor

@matmair matmair left a comment

Choose a reason for hiding this comment

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

On a general note: Should we consider squashing migrations with the next major release? These migrations that require 3 steps are culminating.

InvenTree/company/models.py
"""Calculate the base unit quantiy for a given quantity."""

q = Decimal(quantity) * Decimal(self.pack_quantity_native)
q = round(q, 10).normalize()
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: Should this rounding be noted in the docs?

@matmair
Copy link
Contributor

matmair commented May 25, 2023

@SchrodingersGat have you seen that there are tests that are not run in the coverage?

@SchrodingersGat
Copy link
Member Author

No, which tests are not runnign?

@SchrodingersGat SchrodingersGat added the api Relates to the API label May 26, 2023
@SchrodingersGat SchrodingersGat mentioned this pull request May 26, 2023
Merged
@matmair
Copy link
Contributor

matmair commented May 26, 2023

The migration tests do not seem to run fully anymore, coverage changed here

@SchrodingersGat
Copy link
Member Author

The migration tests do not seem to run fully anymore, coverage changed here

I think it should be fine

@SchrodingersGat SchrodingersGat merged commit 5dd6f18 into inventree:master May 26, 2023
12 checks passed
@SchrodingersGat SchrodingersGat deleted the part-units branch May 26, 2023 06:57
@SchrodingersGat SchrodingersGat mentioned this pull request Jul 16, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matmair matmair matmair approved these changes

Assignees
No one assigned
Labels
api Relates to the API enhancement This is an suggested enhancement or new feature part Related to Part models
Projects
None yet
Milestone
0.12.0
Development

Successfully merging this pull request may close these issues.

Use physical units for "part units"
2 participants
@SchrodingersGat @matmair