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.

Command icon 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:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

m.stenta created an issue. See original summary.

m.stenta’s picture

This 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.

m.stenta’s picture

Related issues: +#3196836: Price quantities
m.stenta’s picture

Title: v1 migration logic does not match usage » Refactor price quantity data architecture
Issue summary: View changes

I 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...

m.stenta’s picture

Status: Active » Needs review

MR 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.

m.stenta’s picture

  • m.stenta committed 03a8459 on 2.x
    Issue #3324839: Refactor price quantity data architecture
    
m.stenta’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.