Unit displays architecture refactor proposal (February 2023)
- Sayaka Ono
- Lite Farm
Author: @Sayaka Ono
What are Unit displays?
Please see the introduction in https://lite-farm.atlassian.net/l/cp/rCoihsHd.
The frontend has a component named “Unit” which handles unit displays (/packages/webapp/src/components/Form/Unit/index.jsx).
User’s farm unit system preference
Users can set Unit system(metric/imperial) in the /farm page.
How does the Unit component look on the app?
simple usage  | system generated(pre-populated)  | system generated(pre-populated)  Unit is fixed |
disabled  | “hide label”  inputs do not have a unit inside | automatic conversion  |
Ways users will interact with value-unit-pairs with examples
Unit displays | Ways users will interact with value unit pairs
When | When the value is(was) pre-populated by the system | When a value is NOT pre-populated by the system or was user created |
|---|---|---|
creating | the Unit displays | Default display guidance should determine the value / unit combination
 In this case, if the total area is equal to or less than 10,890 sqft, it is displayed in ft2. | the user set unit preference should be used What is the “user set unit preference”? Is it a record-to-record unit(a record has not been created on a creation view) or user’s farm unit system preference (metric/imperial)?  |
read-only | the Unit displays | Default display guidance should be used  | the user set unit preference should be used  |
editing | the Unit displays | Default display guidance should be used  | the user set unit preference should be used  |
automatic conversion - we will go with easier solution.
We do not need to see automatic conversion at all for now? (how about the total area?)
Instances of unit displays
(This is not 100 percent accurate)
| Path | Number of unique <Unit /> (59) & Views | Type | Operation | automatic conversion | Note |
|---|---|---|---|---|---|---|
1 |
file: | 4
| user input | create | NO | Currently, raw data(what the user has inputted) is stored in the DB.
|
2 |
file: | 3  | - | read | - |
|
3 |
file: | 3  |
| create | NO |
The location size does not need a unit? |
4 |
file: | 1  | user input | edit | NO |
|
5 |
file:
| 1  | - | read | - |
|
6 |
file: | 2   | user input | create | NO |
|
7 |
file: | 5 (4) 
These should be combined   | user input | create | NO |
|
8 |
condition:
file: | 1  | user input | create | NO |
|
9 |
condition:
file: | 1  | user input | create | NO |
|
10 |
condition: 
file: | 1  | user input | create | NO |
|
11 |
the component is used for #2
file: | 5   | user input | create | NO | Estimated seed 20lb (not “stored as” unit) was saved in the DB |
12 |
file: | 2  | user input | create/edit? | NO | the first unit should be $/kg or $/mt? “Estimated annual revenue” does not change by changing units. |
13 |
file: | 2 | user input | create edit | - | the style for the quantity input is different from others
|
14 |
file: | 2  | pre-populated | create | NO |
|
15 |
file: | 2  | pre-populated | create read edit | NO |
|
16 |
file: | 1  | pre-populated | create | NO |
|
17 |
file: | 4  | pre-populated / user input | create read edit | NO |
|
18 |
file: | 1  | - | read | - | 0.87cm??? After adding sensors with “150cm” and “200cm”, they were shown as “1.5m” and “2cm”. |
19 |
file: | 1  | user input | create | NO |
|
20 |
file: | 2  | pre-populated | create | - |
|
21 |
file: | 6  volume  depth | user input | create | YES |
|
22 |
file: | 1  | pre-populated (reading the data in DB) | edit | NO |
|
23 |
file: | 1  soil amendment | user input | create | NO |
|
24 |
file: | 1  | user input | create | NO |
|
25 |
file: | 1 
| user input ↔︎ disabled by checking “harvest everything” | create | NO |
|
26
|
file: | 2  completed task | - | read | - |
|
27 |
file: | 1 | user input | create | YES | When I input “2 gal” in the estimated water usage input, “2 gal” was saved in the DB. |
28 |
file: | 1  | user input | create | NO | when changing unit, “Amount to allocate” does not change. Should it? |
29 |
file: | 1  | user input | create | NO |
|
How are values stored in the database?
Unit displays | How values are stored in the database
Values are stored in the apples-to-apples manner as defined as “stored as” unit in Unit displays | Default display guidance. For instance, all distance values in the database are in “m” even if you save it in km, ft or mi on the web app.
Scenarios
User’s farm unit system preference is “imperial”. The user created a barn and the total area was 50,000 acres. He switched the unit system to “metric”. (the record-to-record unit and the unit system setting don’t match)
-> The total area should be displayed in ha as determined by the default display guidance. (not record-to-record unit?)
https://lite-farm.atlassian.net/browse/LF-2880User’s farm unit system preference is “imperial”. The user created a crop plan with planting depth 3 inches.
-> The depth should be stored in XXX.
“depth” is not in the Unit displays | Default display guidance. Is there a guidance for depth and spacing?
How is the conversion currently happening?
Unit displays | Current architecture of the units epic
The web app is handling all conversions. When creating a record, it converts the inputted value into “stored as” value, and when reading, it converts the stored value into a value in the user’s preferred unit.
Note:
I save 1 gallon to the db. The db saves that as 4.54 litres. When it's then displayed again we have 0.999843 gal…? → this is not a concern, and conversion is taken care of by convert-units library
What is the problem and why are we working on refactor?
There have been some bugs found on Unit displays, but they have not been fixed because there were times that a bug fix affected other Unit instances that were functioning before. The unit component is used in many different views and requirements for each instance are vary as seen above, and it was thought that the component holding a good amount of logic to handle all of these cases made it hard to be maintained and scaled. (As of February 10, 2023, the component is used in 59 different instances across 29 different files in the webapp.) According to Jag Dhillon, in order to fix existing bugs and not to introduce new bugs, refactor is required.
Known issues
data is not being saved in the apples-to-apples manner
Unit displays | How values are stored in the database
https://lite-farm.atlassian.net/browse/LF-2719when switching the unit system setting (imperial/metric), values are not converted correctly.
https://lite-farm.atlassian.net/browse/LF-2880
and more…!
How are we going to address this issue?
- Any proposed architectural refactor must be implementable by 1 engineer in 3 sprints (6 weeks) or less
- Architecture supports the ability for the user to define how unit switching works for them (see Unit displays | A note on changing units when a value has already been entered)
Proposed approach
Jag Dhillon says “The unit component has multiple different useEffects that are causing most of this tight interdependency”, “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.” in his proposal. His proposal is to decouple the logic from the unit component, and he suggested we move the logic to the backend.
Alternative approach
The other approach is to fix and enhance the unit component. By looking at the code and discussions in the past, it seems to me that bug fixes have been done without understanding how the unit component was intended to work. If bug fixes had been done in the right way, this would not have been such a matter.
I believe the current system is well-designed and would like to move in the direction of fixing and improving what we have now. The reasons are as follows:
Even if some logic is moved to the backend, the frontend still need to hold a good amount of logic. It would be better to keep the logic on the frontend and focus on making it robust.
Avoid changing many files which can lead to more bugs.
Reduce the time required for testing.
I will explain how the current unit component is supposed to work and how we can resolve issues.
How the data is converted when creating or updating a record
Besides the input where users input a value and select a unit, there is another input that is hidden below(see the screenshot). This hidden input gets a value that is going to be stored in the database when a value is inputted in the visible input. This hidden value exists for two purposes: 1. to validate if the inputted value is within the acceptable range in the database after being converted to the “stored as” value, 2. to be sent and stored in the database. For this purpose #1, the logic that converts inputted values to “stored as” values cannot be removed from the frontend, and it makes more sense to send the “stored as” value that is calculated on the frontend to the backend via APIs.
display: none (currently not working in the integration branch)How the data is converted when showing a value retrieved from the database
The values in the database need to be converted before being displayed in the web app. This conversion is currently happening in useEffect hooks in the unit component, but there seems to be some conflicts in the conditions. We can create a hook to handle the logic (https://felixgerschau.com/react-hooks-separation-of-concerns/) while resolving the conflicts. Resolving the issues seems more reasonable than adding a few pieces of code to tens of APIs to move the logic to the backend.
Note:
One of the conflicts is that the useEffect (line 225) sets the correct unit, but it is overridden by another useEffect (line 237).
(How the data is converted automatically when changing a unit)
Input values were automatically converted when units were changed. The logic is in a useEffect in the unit component, but it is not currently working.
Here is the comparison between the two approaches:
| proposed approach | alternative approach |
|---|---|---|
backfill data | find wrong data and write a script | |
database | no change required | |
APIs |
| no change required |
frontend |
|
|
auto conversion | DB: need migration (auto conversion property in the userFarm table) | |
backend: update the helper or middleware to take user setting into account | frontend: update the hook to take user setting into account | |
pros |
|
|
cons |
|
|
Question
range error for the inputted value
The maximum number that can be accepted changes depending on the selected unit, but the error message does not change. What is it supposed to be? (currently the error message is always 0-1000000000 for the input in the screenshot)ac: 4046860338724.812
ft2: 92903129.90644656
m2: 1000000000
ha: 10000000000000 (stored as)
(I misunderstood this part. The error is shown against the hidden value. How this works is that the max value for the visible input is converted to “stored as” value to match the unit with that of the hidden value, and the hidden value is validated.)
TODO
ask for answers to all questions and make the requirements crystal clear → create a document(how each Unit input works, how the data is stored in the DB, etc) that can also be used as a reference when testing
update the style? https://lite-farm.atlassian.net/browse/LF-2910 → NO
save data in an apples-to-apples manner
show data correctly on the frontend (should be able to support automatic conversion in the future)
write tests
figure out which data needs to be backfilled and how → write a script.
https://lite-farm.atlassian.net/browse/LF-2719
https://docs.google.com/spreadsheets/d/1XZ-Pgy4xAqYlLCQUeYb3a2pZAknimCRlAwlVdP8xZT4/edit#gid=391799990
should we correct data in production/beta or both? Do we need to investigate when we started saving wrong data?we need to deploy APIs & app after backfilling is complete. Does anyone know how?
fix related bugs
Strategy
This task is expected to require a lot of changes. For easier reviews, I am going to create a feature branch and raise PRs against the branch.
| PR | merge to | estimate in story points |
|---|---|---|---|
1 | extract logic from the current unit component and create a hook | feature branch | 2 |
2 | update the unit component to make it work as a presentation component | feature branch | 2 |
3 | fix the logic in the hook Check:
| feature branch | 8 |
4 | test all instances and make adjustments and fix issues (hopefully write tests)
| feature branch, and then create a PR against integration branch | 8 - 13 |
5 | Pop-up when changing system of measure at your farm warning that everything will default based on the value. | integration |
|