Unit displays architecture refactor proposal (August 2022)
- Lite Farm
Original author: Jag Dhillon
Context - Where we are (as of August 2022) with units
At the time of writing, the unit component is used in 54 different instances across 28 different files in the LiteFarm codebase. All the different types of measurements (area, length, mass, volumeFlowRate, volume, time) store default units in the database regardless of what unit preference (eg. grams or kilograms) or unit system (ie. metric or imperial) is used by the user. The intuition behind having the values persisted in default units in the database was to have a consistent value/unit combination in the database instead of having to store raw value/unit combinations. Storing raw value/unit combinations introduces problems when the user ever tries to switch unit systems. The system they prefer could be metric, but if they used imperial before then all of their current plans and tasks would still be in metric values. Having this consistent value/unit combination would make it easy to convert to whatever the user’s preferred units/system is.
Example 1: If I am making a crop plan and I input 3 mt as the estimated seed required. The value/unit combination will be stored as 3000 kg (because the default value for mass is in kg) in the database with a note that the user prefers metric tons as their unit value so when the value is pulled it is converted into metric tons in the frontend before being displayed on a read only view, edit view, etc.
Example 2: If I am making a cleaning task and input 1500 fl-oz as as my preferred value/unit pair, it will actually be stored as 44.36029 Litres in the database with a note that fl-oz is the preferred unit and again the conversion will take place on the frontend.
Furthermore, the unit component in the production build currently changes the current inputted value to match its equivalent value in different units whenever the unit is changed.
Example: I currently inputted 1000 with the unit of kg
and if I change the unit then it will change number to the previous units equivalent 1000kg=1mt
This also is the case for area, length, volumeFlowRate, volume, and time units. Some users like that changing the unit changes the value. Others have provided feedback against it - preferring to input their own unit-value-pairing. There may actually be:
Two different use cases here, with some situations the unit dynamically updating the value and in others not.
A user-to-user configuration option defining whether units should update values or not
Using global search to find instances of Units
The main way for finding all updated instances of the unit component is to type in "<Unit" in the global search of the codebase, this will show every instance of the Unit component and where they are located as well as the file they are named under are descriptive enough to give an idea to the person searching for it.
Current architecture of the units epic
The backend/database is just responsible for receiving the request and storing the converted values in the database
The frontend/client handles all conversions and pre population of the value/units in the read-only and edit views of plans and tasks.
Proposed architecture
The backend should be responsible for all conversions of value/unit pairs and the frontend should be responsible for pre populating units in the unit component when filling out an empty form and displaying the values coming from the backend. The unit component has multiple different useEffects that are causing most of this tight interdependency which is why we are proposing the conversion be moved to the backend.
Example shown below of the pre populated units in an empty form based on the user’s system preference
Most of the logic has already been implemented and to my knowledge is also working correctly as well. But the problem is the code is tightly coupled and minor changes to the code easily break some of the 56 component instances throughout the app. This code in the folder packages/webapp/src/util/convert-units needs to be moved to the backend packages/api/src/util folder. This way the component will only be responsible for displaying the values passed from the backend and having default values based off the flow and user preference.
I will describe some code snippets I understand to help the future engineer get a head start on what is being done along with my recommendation but it is ultimately up to the future engineer’s discretion to keep the code in the frontend or backend as they will be diving into the details of the unit refactor.
File: packages/webapp/src/components/Form/Unit/index.jsx
Lines: 291-299
This useEffect triggers when a value is inputted into the unit component and ensures the value is within 2 decimals places and non negative. I recommend we keep this in the frontend as this is more of a validation rather than conversion.
useEffect(() => {
if (databaseUnit && hookFormUnit) {
setVisibleInputValue(
hookFormValue > 0 || hookFormValue === 0
? roundToTwoDecimal(convert(hookFormValue).from(databaseUnit).to(hookFormUnit))
: '',
);
}
}, [hookFormValue]);
File: packages/webapp/src/components/Form/Unit/index.jsx
Lines: 243-251
This triggers when the unit has been changed and changes the value of the input to the equivalent value in different units like I described above where 1000kg=1mt. I recommend this be taken out completely. Taking this out completely broke the app before so we had to bring it back to preserve functionality
useEffect(() => {
if (hookFormUnit && hookFormValue !== undefined) {
setVisibleInputValue(
roundToTwoDecimal(convert(hookFormValue).from(databaseUnit).to(hookFormUnit)),
);
//Trigger validation
(hookFormValue === 0 || hookFormValue > 0) && hookFormSetHiddenValue(hookFormValue);
}
}, [hookFormUnit]);
File: packages/webapp/src/components/Form/Unit/index.jsx
Lines: 228-234
useEffect(() => {
!hookFormGetValue(displayUnitName) &&
hookFormSetValue(displayUnitName, getUnitOptionMap()[displayUnit]);
if (hookFormGetValue(displayUnitName)) {
hookFormSetValue(displayUnitName, getUnitOptionMap()[displayUnit]);
}
}, []);I recommend we keep this code in the frontend because this is pre populating the unit values to their defaults instead of having blank units when filling out a form
Pitfalls to avoid
Original author: Jag Dhillon
Scrapping the unit component
The unit component does need to have automatic conversions in system generated values (Case 1-a under Unit displays | Ways users will interact with value unit pairs). An example of this is when you create a new location. Smaller locations should display their area in square meters or square feet while larger locations should display in hectares or acres before the user has a chance to set a default. I suggest we keep the production code for the unit component only for this part of the app and create and entirely new unit component for all other instances in the app.
Tight coupling
The problem of unit displays is an example of tight coupling. This means there is a high interconnectivity between the lines of code in the unit displays component. The unit displays component tries to achieve two things at once.
1: Displaying the units
2: Handling unit conversions
To reduce the tight coupling aka solve this problem is to separate these two tasks. The handling of the unit conversions should be in the backend while displaying the units should be a client side job. Separating the tasks should allow for easier debugging, and would be much easier to add new functionality. (Simple but longer code is better than shorter and clever code)
Having multiple developers work on units
Although units is something we all use in our day to day lives, approaches on how to tackle solving this problem varies. I suggest minimizing the amount of developers working on this component in order to have a single source of truth coming from a single (or very few) developer instead of having varying perspectives on how to tackle and the status of this problem