Problem/Motivation
I am in the process of migrating farmOS v1 instances to v2, and some of these are using the Sale/Purchase log types that were provided in v1 (which are now provided by this module). In reviewing the way these Sale/Purchase logs were used in v1, I'm noticing that the v2 module's migration logic is not exactly correct.
When we created the new "Price quantity", we designed it so that the "value" of the quantity would represent the "total price" of the transaction, and two additional fraction fields were added to price quantities alongside that: "Unit price" (the price per unit) and "Unit quantity" (the number of units). Thus, unit price multiplied by unit quantity should always equal total price (and we even hide the value field on this quantity type and auto-populate it based on that math). In this design, the "Units" of the price quantity is expected to be the currency of the transaction (eg: "USD").
However, looking back at the fields on v1 Sale/Purchase logs, our migration logic is flawed.
These are the v1 fields and their descriptions:
- Quantity - How many were sold?
- Units - What is the unit of measurement for the quantity entered?
- Unit price - (no description)
- Total price - (no description)
The current migration logic provided by this v2 module, however, creates a Price quantity mapped from v1 data as follows:
- Value - "Total price" from v1
- Units - "Units" from v1
- Unit quantity - "Quantity" from v1
- Unit price - "Unit price" from v1
As you can see, most make sense... but "Units" does not. At least not if we are working under the assumption that "Value" will be the total price and "Units" will be the currency.
To illustrate, imagine this Sale log in v1:
- Quantity: 5
- Units: plants
- Unit price: 10
- Total price: 50
The v2 migration logic will create a Price quantity that looks like this:
- Value (aka total price): 50
- Units: plants
- Unit price: 10
- Unit quantity: 5
Proposed resolution
- Remove unit_quantity
field.
- Add total_value
field.
- Migrate existing quantity value
data to new total_value
field.
- Migrate existing unit_quantity
data to quantity value
field.
- Update all Views, migration, theming, logic, etc.
Remaining tasks
(none)
User interface changes
- "Unit quantity" field is replaced with "Total price" field.
- Measure, Value, and Inventory fields are no longer hidden.
- Revert to default quantity theming.
API changes
- Removal of unit_quantity
field on price
quantities.
- Addition of total_price
field on price
quantities.
Data model changes
- Removal of unit_quantity
field on price
quantities.
- Addition of total_price
field on price
quantities.
- The value
field of price
quantities no longer represents the "total price" of the quantity. Instead, it represents the "number of units", which is more inline with how other quantity types are used.
Issue fork farm_ledger-3324839
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
m.stentaThis is making me rethink how we've approached the data architecture of Price quantities more generally.
Perhaps we should have structured it like this instead:
- Value == unit quantity
- Units == unit of measurement of the value
- Unit price (new field) == fraction field
- Total price (new field) == fraction field, auto-calculated
- Currency (new field) == a new taxonomy reference?
The difference being: instead of adding "Unit price" and "Unit quantity" fields, we add "Unit price", "Total price", and "Currency" fields.
This way, the quantity "Value" and "Units" would work more consistently with other quantities, and we would be adding fields specifically for price information on top of that.
I feel like that makes more sense, but changing to this approach would require some consideration. If anyone has already started using this module in v2, we would need to think through how changing this would affect existing data, and whether or not a data migration (via update hook) would be necessary.
Comment #3
m.stentaComment #4
m.stentaI put together a branch that makes all the changes I described. It feels, looks, and works better all around, IMO. I was able to remove some of the overrides we had previously (like hiding the Measure field, defaulting measures to "value" all the time, hiding inventory management field, etc).
This also means that price quantities can adjust inventory now, which is very useful! A sale/purchase can not only record how much it cost, but also increment/decrement inventory with a single quantity.
MR coming soon...
Comment #6
m.stentaMR for review!
Tested the update hook and v1 migrations and everything is working.
I also started coding a validation constraint to ensure
value * unit_price == total_price
, but ran into some issues with the availability of old vs new values in the callback. I need to debug it more, but it doesn't need to hold up this MR, so I'll open a separate issue for it.Comment #7
m.stentaFollow-up for validation constraint: #3325332: Add a validation constraint for value * unit_price == total_price
Comment #9
m.stenta