Versions Compared

Old Version 4

changes.mady.by.user Lite Farm

Saved on

compared with

New Version Current

changes.mady.by.user Sayaka Ono

Saved on

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

...

...

...

...

...

...

...

...

...

...

...

...

...

...

...

...

...

...

...

...

...

...

...

...

...

...

...

...

...

...

...

...

...

...

...

...

...

...

Author: Sayaka Ono

Table of Contents
minLevel1
maxLevel7

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 settingUser’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)

disabled

“hide label”

automatic conversion

Screen Recording 2023-02-10 at 5.14.32 PM.mov

Ways users will interact with value-unit-pairs with examples

https://lite-farm.atlassian.net/wiki/spaces/LITEFARM/pages/658309153/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 https://lite-farm.atlassian.net/wiki/spaces/LITEFARM/pages/658309153/Unit+displays#Default-display-guidance should determine the value / unit combination

the user set unit preference should be used

(warning) 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 https://lite-farm.atlassian.net/wiki/spaces/LITEFARM/pages/658309153/Unit+displays#Default-display-guidance should be used
(warning) Once pre-populated value is modified, is it considered as a “user created” value? Currently, even if I change the unit to “ft2” and save, “ac” is still shown. Is it a bug?

the user set unit preference should be used





editing

the https://lite-farm.atlassian.net/wiki/spaces/LITEFARM/pages/658309153/Unit+displays#Default-display-guidance should be used

the user set unit preference should be used

  • automatic conversion - we will go with easier solution.
    (warning)We do not need to see automatic conversion at all for now? (how about the total area?)

Instances of unit displays

Expand
titleList of <Unit /> (59 instances in 29 files)

(This is not 100 percent accurate)

Path
&
file (29)
(/packages/webapp/src/components)

Number of unique <Unit /> (59)

&

Views

Type
(pre-populated / user input)

Operation
(Create/Read/Edit)

automatic conversion

Note

1

crop/:cropId(?)add_management_plan/bed_method

file:
/Crop/BedPlan/PureBedForm.jsx

4

user input

create

NO

🕷️ Currently, raw data(what the user has inputted) is stored in the DB.

  • After switching unit system(User’s farm unit system preference), data conversion does not happen.

2

/tasks/:taskId?/read_only

file:
/Crop/BedPlan/PurePlanGuidanceForm.jsx

3

-

read

-

3

/crop/:cropId?/add_management_plan/broadcast_method

file:
/Crop/BroadcastPlan/PureBroadcastForm.jsx

3

  • area used & Estimated seed required: pre-populated

  • user input

create

NO

  • “estimated seed required is calculated by inputting “seeding rate”, but it does not take the unit into account.

(warning) The location size does not need a unit?

4

/crop/:cropId?/management_plan/:planId?/edit

file:
/Crop/ManagementDetail/EditManagementPlanDetail.jsx

1

user input

edit

NO

5

/crop/:cropId?/management_plan/:planId?/details

file:
/Crop/ManagementDetail/ManagementPlanDetail.jsx

1

-

read

-

6

/crop/:cropId?/add_management_plan/planted_already

file:
/Crop/PlantedAlready/index.jsx

2

user input

create

NO

7

/crop/:cropId?/add_management_plan/container_method

file:
/Crop/PlantInContainer/PureContainerForm.jsx

5 (4)

These should be combined ⬇️

user input

create

NO

  1. # of plants → “1”. Estimated seed required input is displayed.
    # of plants → “10”
    the input disappears. 🕷️?

  2. (warning) after switching a unit for the calculated input, the calculation does not change.

    Screen Recording 2023-02-27 at 8.56.16 PM.mov


8

/crop/:cropId?/add_management_plan/plant_date

condition:
already_in_ground && is_wild && !needs_transplant

file:
/Crop/PlantingDate/NextHarvest.jsx

1

user input

create

NO

9

/crop/:cropId?/add_management_plan/plant_date

condition:
!(already_in_ground && is_wild && !needs_transplant)

file:
/Crop/PlantingDate/PurePlantingDate.jsx

1

user input

create

NO

10

/crop/:cropId?/add_management_plan/final_planting_method

condition:

is_planting_method_known === false && showIsPlantingMethodKnown && isFinalPlantingMethod

file:
/Crop/PlantingMethod/PureManagementPlanPlantingMethod.jsx

1

user input

create

NO

11

/crop/:cropId?/add_management_plan/row_method

the component is used for #2

file:
/Crop/RowMethod/PureRowForm.jsx

5

user input

create

NO

🕷️ Estimated seed 20lb (not “stored as” unit) was saved in the DB

12

/finances/estimated_revenue/plan/:planId

file:
/Finances/UpdateEstimatedCropRevenue/index.jsx

2

user input

create/edit?

NO

(warning) the first unit should be $/kg or $/mt? “Estimated annual revenue” does not change by changing units.

13

/add_sale

/edit_sale

file:
/Inputs/CropVarietySale/index.jsx

2

user input

create

edit

-

(warning) the style for the quantity input is different from others

  • <Unit /> does not need to be used for $…

14

/create_location/ceremonial_area etc.

file:
/LocationDetailLayout/AreaDetails/AreaDetails.jsx

2

pre-populated

create

NO

15

/create_location/buffer_zone

/buffer_zone/:bufferId?/details

file:
/LocationDetailLayout/LineDetails/BufferZone/index.jsx

2

pre-populated

create

read

edit

NO

16

/create_location/fence

file:
/LocationDetailLayout/LineDetails/Fence/index.jsx

1

pre-populated

create

NO

17

/create_location/watercourse

/watercourse/84cb7da8-abe9-11ed-8b70-e66db4bef551/details

file:
/LocationDetailLayout/LineDetails/Watercourse/index.jsx

4

pre-populated / user input

create

read

edit

NO

18

/sensor/:sensorId?/details

file:
/LocationDetailLayout/PointDetails/Sensor/index.jsx

1

-

read

-

🕷️ 0.87cm???

After adding sensors with “150cm” and “200cm”, they were shown as “1.5m” and “2cm”.

19

/create_location/water_valve

file:
/LocationDetailLayout/PointDetails/WaterValve/index.jsx

1

user input

create

NO

20

/map
(location - watercourse)

file:
/Map/LineMapBoxes/index.jsx

2

pre-populated

create

-

21

/add_task/task_details

file:
/Modals/WaterUsageCalculatorModal/index.jsx

6

user input

create

YES

22

/sensor/:sensorId?/edit

file:
/Sensor/EditSensor.jsx

1

pre-populated (reading the data in DB)

edit

NO

2223

/add_task/task_details

file:
/Task/AddProduct/index.jsx

1

user input

create

NO

2324

/add_task/task_details

file:
/Task/CleaningTask/index.jsx

1

user input

create

NO

2425

/add_task/task_details

file:
/Task/HarvestingTask/index.jsx

1

user input ↔︎ disabled by checking “harvest everything”

create

NO

2526

/tasks/:taskId?/read_only

file:
/Task/HarvestingTask/ReadOnly.jsx

2

-

read

-

2627

/add_task/task_details

file:
/Task/PureIrrigationTask/index.jsx

1

Screen Recording 2023-02-10 at 5.14.32 PM.mov

user input

create

YES

🕷️ When I input “2 gal” in the estimated water usage input, “2 gal” was saved in the DB.

2728

/tasks/:taskId?/harvest_uses

file:
/Task/TaskComplete/HarvestComplete/HarvestUses.jsx

1

user input

create

NO

(warning) when changing unit, “Amount to allocate” does not change. Should it?

2829

/tasks/:taskId?/complete_harvest_quantity

file:
/Task/TaskComplete/HarvestComplete/Quantity.jsx

1

user input

create

NO

How are values stored in the database?

https://lite-farm.atlassian.net/wiki/spaces/LITEFARM/pages/658309153/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 https://lite-farm.atlassian.net/wiki/spaces/LITEFARM/pages/658309153/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

  1. 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 (warning) ha as determined by the default display guidance. (not record-to-record unit?)

    Jira Legacy
    serverSystem JIRA
    serverId815f41e5-e5fb-3402-8587-82eccc3ffab0
    keyLF-2880

  2. User’s farm unit system preference is “imperial”. The user created a crop plan with planting depth 3 inches.
    -> The depth should be stored in (warning) XXX.
    (warning) “depth” is not in the https://lite-farm.atlassian.net/wiki/spaces/LITEFARM/pages/658309153/Unit+displays#Default-display-guidance. Is there a guidance for depth and spacing?

How is the conversion currently happening?

https://lite-farm.atlassian.net/wiki/spaces/LITEFARM/pages/658309153/Unit+displays#Current-architecture-of-the-units-epic

...

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.

...

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 https://lite-farm.atlassian.net/wiki/spaces/LITEFARM/pages/658309153/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.

Expand
titleWhat is Tight coupling?

https://lite-farm.atlassian.net/wiki/spaces/LITEFARM/pages/658309153/Unit+displays#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)

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

...

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.

...

proposed approach

alternative approach

backfill data

find wrong data and write a script

database

no change required

APIs

  • [POST] convert received values to “stored as” values for each endpoint and store (create a helper or middleware)

  • [GET] convert saved values to “display” values before sending to a client (create a helper or middleware)

  • add tests for each endpoint

no change required

frontend

  • remove logic from the Unit component (make it a presentation component)

  • create a hook to handle logic, and add comments to make it clear what the code does (this still needs to calculate what the values would be when stored in the database)

  • remove logic from the Unit component (make it a presentation component)

  • create a hook to handle logic, and add comments to make it clear what the code does (this includes data conversion when submitting or showing data)

  • write tests for each unit input

auto conversion
(optional)

DB: need migration (auto conversion property in the userFarm table)
(UI: modify the user setting view to set “auto conversion”)

backend: update the helper or middleware to take user setting into account

frontend: update the hook to take user setting into account

pros

  • some logic can be removed from the frontend

  • the logic is in one place and could avoid small errors for adding lines all over the files

  • no need to test APIs

cons

  • frontend still needs to take care of some calculations

  • need to update many APIs and their tests

  • QA needs to test APIs + UI

  • should be careful not to make the same mistakes (no more rework…)

(warning) Question

  • range error for the inputted value inputted


    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)

...

    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

(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

  • (warning) update the style?

    Jira Legacy
    serverSystem JIRA
    serverId815f41e5-e5fb-3402-8587-82eccc3ffab0
    keyLF-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.

    Jira Legacy
    serverSystem JIRA
    serverId815f41e5-e5fb-3402-8587-82eccc3ffab0
    keyLF-2719

    https://docs.google.com/spreadsheets/d/1XZ-Pgy4xAqYlLCQUeYb3a2pZAknimCRlAwlVdP8xZT4/edit#gid=391799990
    (warning) 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

3fix the logic in the hook

fix the logic in the hook

Check:

  1. Does the display unit match the system of measure (e.g. cm and imperial)? If no, jump to #3

  2. Does the user specify the unit (within the system of measure), use that unit

  3. Otherwise: Display default for value

feature branch

8

4

test all instances and make adjustments and fix issues (hopefully write tests)

  • What about calculated values? (e.g. water use calculator)

feature branch, and then create a PR against integration branch

8

TODO

estimate in story points

1

backfill data

5+
depending on how much wrong data we have

- 13

5

Pop-up when changing system of measure at your farm warning that everything will default based on the value.

integration