LF-4166 (4): Implement end to end animal creation flow #3397

SayakaOno · 2024-08-27T21:45:23Z

Description

Implement format functions for animal and batch form data
Implement onSave function in AddAnimals
Add missing properties to Animal/Batch types
Correct organic status option type
Create PostAnimalBatch type and correct addAnimalBatches mutation request body type
Fix dates' format in Animal model

Jira link: https://lite-farm.atlassian.net/browse/LF-4166

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

Passes test case
UI components visually reviewed on desktop view
UI components visually reviewed on mobile view
Other (please explain)

Checklist:

I have commented my code, particularly in hard-to-understand areas
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
The precommit and linting ran successfully
I have added or updated language tags for text that's part of the UI
I have added "MISSING" for all new language tags to languages I don't speak
I have added the GNU General Public License to all new files

* adjust storybook mock data

Duncan-Brain · 2024-08-29T19:32:48Z

packages/webapp/src/store/api/types/index.ts

  animal_removal_reason_id: number | null;
  removal_explanation: string | null;
  removal_date: string | null;
 }

+export interface PostBatchSexDetail {
+  animal_batch_sex_detail?: { sex_id: number; count: number }[];


my only question is if this is to match up a name difference from frontend and backend -- is it possible to fix it to be one name?

That's actually a bug...!!! Sex detail will not be added unless the key is sex_detail.
Thank you so much for pointing that out!

…_animal_creation_flow

Duncan-Brain

So treating this as the the last 🤞 ticket on add animals -- just pointing out everything I see. Approving in case we want to deal with them separately.

When I merge integration:

animal use is not being filtered (eggs is viewable on pigs for example)
summary screen is doing that awkward fullwidth thing where the sizes are not quite right and stuff is cut off on left and right.
is there no snackbar when we successfully add animals?

Duncan-Brain · 2024-08-29T22:36:42Z

packages/webapp/src/containers/Animals/AddAnimals/types.ts

@@ -89,7 +89,7 @@ export type Option = {
  [DetailsFields.USE]: ReactSelectOption<number>;
  [DetailsFields.TAG_COLOR]: ReactSelectOption<number>;
  [DetailsFields.TAG_TYPE]: ReactSelectOption<number>;
-  [DetailsFields.ORGANIC_STATUS]: ReactSelectOption<number>;
+  [DetailsFields.ORGANIC_STATUS]: ReactSelectOption<'Non-Organic' | 'Transitional' | 'Organic'>;


Could consider making 'Non-Organic' | 'Transitional' | 'Organic' a reusable type -- its used twice here and would be useful for any locations that are going to be added like pasture.

Definitely. I got a bit lazy about finding the right place for it, sorry. I'll make sure to add it!

SayakaOno

Thank you Duncan!

I think I can address the first two points. As the PR owner, I'm okay with making the changes in this PR or in a separate one, but as a reviewer, I prefer a separate PR. Please let me know if you guys have any preference!

Regarding the success snackbar, I assumed the summary screen would replace it. Let me confirm with Loïc!

SayakaOno · 2024-08-29T22:51:29Z

packages/webapp/src/containers/Animals/AddAnimals/types.ts

@@ -89,7 +89,7 @@ export type Option = {
  [DetailsFields.USE]: ReactSelectOption<number>;
  [DetailsFields.TAG_COLOR]: ReactSelectOption<number>;
  [DetailsFields.TAG_TYPE]: ReactSelectOption<number>;
-  [DetailsFields.ORGANIC_STATUS]: ReactSelectOption<number>;
+  [DetailsFields.ORGANIC_STATUS]: ReactSelectOption<'Non-Organic' | 'Transitional' | 'Organic'>;


Definitely. I got a bit lazy about finding the right place for it, sorry. I'll make sure to add it!

kathyavini

Wow, it feels so good when the full flow comes together in such a logical and readable PR -- I guess this was all the smart PR breaking up you did!!! ❤️

The only thing that makes me sad is how much you had to pass around broughtInId! Did we mess up at some point architecting an id-based field as a radio? It kind of makes me want to do this:

// useAnimalOptions.ts
  if (optionTypes.includes('origin')) {
    options.originOptions = orgins.map(({ id, key }) => ({
      value: { id, key },
      label: t(`animal:ORIGIN.${key}`),
    }));
  }

And then update RadioGroup to accept it. But that's insane, right? 🤣 To give an object value to a radio?

It would make the component and utils so clean though 😂

// utils.ts
const isBroughtIn = data[DetailsFields.ORIGIN]?.key === 'BROUGHT_IN';

return {
  birth_date: convertFormDate(data[DetailsFields.DATE_OF_BIRTH]),
  origin_id: data[DetailsFields.ORIGIN]?.id,

Well, probably passing around a few more props to everything is better than violating the natural type of radios, isn't it?

Anyway the formatting functions are so tidy, I love them ❤️ Great job!

packages/webapp/src/containers/Animals/AddAnimals/utils.ts

Co-authored-by: Joyce Yuki <[email protected]>

SayakaOno

Thank you Joyce for reviewing!

For the radio, using objects as values doesn't feel right, but maybe we could save an option itself as origin...? Let me try to clean up the code for broughtInId!

packages/webapp/src/containers/Animals/AddAnimals/utils.ts

kathyavini

It's only optional about changing broughtInId! I think the radio had annoyed me in the container/component too (the need for the getOriginEnum()) so I had been thinking equally as well how sweet it would be to delete all of that as well in favour of being able to just

// Origins.tsx
watchedOrigin?.key === AnimalOrigins.BROUGHT_IN ? // ...

(I had also wondered if using broughtInId could have simplified that component compared to the enum, but then figured altering the radios would be even shorter for both!)

I guess we could also construct a compound string of ${id}_${key} too -- that could be a bit more of a compromise that keeps the radio value a primitive type.

I'm surprised to not know exactly which pattern is considered best practice for radios, though, whether we should be keeping the enum table separate from the ids and re-fetching like this, or something else 🤔💭

Anyway I do think any adjusts are optional, as the current pattern is clear and works well!

Edit:

but maybe we could save an option itself as origin...

Oh wait do you mean compound key values as well here? 😄

SayakaOno · 2024-08-30T21:11:29Z

I wanted to address Duncan's inline comment, and I did it, but the last merge commit got the weird commit message somewhere... I want to amend the message, but I'll just leave it not to force push 😢

@kathyavini
I tried wrapping the RadioGroup with Controller and passing option itself instead of id like this:

and watchedOrigin became an object ({value, label, key}), but RadioGroup takes control and name, so I think it would require refactoring... (I'm not sure if it's feasible to make it more flexible)

I am not sure what the radio's best practice is either... let's revisit it later!

kathyavini

@SayakaOno oh I see, yes a wrapping-unwrapping seems a lot more sound but that does sound like a complicated refactor on Radio/RadioGroup! With the objects I just tested the value going all the way to <Radio /> and an alter on maybe key and checked?

But I'm really leaning towards compound string keys as being the most lightweight -- I'll open a demo PR after this one is merged to show what that looks like, and it will be fun to discuss with Anto what is best practice. No rush and probably when you get back then! 😄

(HAVE FUN!!!! 👋)

SayakaOno · 2024-08-30T23:44:27Z

Thank you Joyce!! ❤️
I personally don't enjoy working with compound keys...😂
I'm wondering if it would be an option to make the function a hook so that we could useQuery...

I'm looking forward to finding good solutions!!

Add animal fixes

Duncan-Brain

Thanks for doing this all! Looks amazing!

Going to address elsewhere but just to document that the bug existed at this point at least:

On logout during addAnimals flow the logout redirect does not work as expected. The url stays as the add animals url and the litefarm top banner is the only thing present:

kathyavini

Yay, into verification for Add Animals!! 🎉❤️

SayakaOno self-assigned this Aug 27, 2024

SayakaOno added the enhancement New feature or request label Aug 27, 2024

SayakaOno changed the title ~~LF-4166 (4): Implement end to end animal creation flow~~ [WIP] LF-4166 (4): Implement end to end animal creation flow Aug 27, 2024

SayakaOno force-pushed the LF-4166/Complete_Implement_End-to-End_animal_creation_flow branch 2 times, most recently from cc1283e to 5018ba8 Compare August 28, 2024 19:33

SayakaOno added 12 commits August 28, 2024 12:34

LF-4166 Add missing properties to Animal/Batch types

19eb066

LF-4166 Correct organic status Option type

c7e41f8

* adjust storybook mock data

LF-4166 Correct addAnimalBatches mutation request body type

5f7e8f9

LF-4166 Create format functions for animal and batch

50b9556

LF-4166 Implement onSave function in AddAnimals

6fcb104

LF-4166 Fix format animal/batch functions

1c7b4dc

LF-4166 Fix dates' format in Animal model

2582ce0

LF-4166 Finalize organic status type

b694cca

LF-4166 Rename animal util functions

6c9ff30

LF-4166 Create PostAnimalBatch type

5260f26

LF-4166 Correct types

b266c93

LF-4166 Move birth_date to origin section in format function

8d85441

SayakaOno force-pushed the LF-4166/Complete_Implement_End-to-End_animal_creation_flow branch from 5018ba8 to 8d85441 Compare August 28, 2024 19:34

SayakaOno changed the title ~~[WIP] LF-4166 (4): Implement end to end animal creation flow~~ LF-4166 (4): Implement end to end animal creation flow Aug 28, 2024

LF-4166 Add missing properties to AnimalBatch type

9f9560b

SayakaOno marked this pull request as ready for review August 28, 2024 21:25

SayakaOno requested review from a team as code owners August 28, 2024 21:25

SayakaOno requested review from Duncan-Brain and kathyavini and removed request for a team August 28, 2024 21:25

Duncan-Brain reviewed Aug 29, 2024

View reviewed changes

Duncan-Brain previously approved these changes Aug 29, 2024

View reviewed changes

LF-4166 Fix animal batch POST request body and type

032ed32

SayakaOno dismissed Duncan-Brain’s stale review via 032ed32 August 29, 2024 20:47

SayakaOno requested a review from Duncan-Brain August 29, 2024 20:51

Merge branch 'integration' into LF-4166/Complete_Implement_End-to-End…

2baba57

…_animal_creation_flow

Duncan-Brain previously approved these changes Aug 29, 2024

View reviewed changes

SayakaOno commented Aug 29, 2024

View reviewed changes

kathyavini reviewed Aug 29, 2024

View reviewed changes

packages/webapp/src/containers/Animals/AddAnimals/utils.ts Outdated Show resolved Hide resolved

packages/webapp/src/containers/Animals/AddAnimals/utils.ts Outdated Show resolved Hide resolved

packages/webapp/src/containers/Animals/AddAnimals/utils.ts Show resolved Hide resolved

Apply suggestions from code review

9fc8d20

Co-authored-by: Joyce Yuki <[email protected]>

SayakaOno dismissed Duncan-Brain’s stale review via 9fc8d20 August 30, 2024 19:26

SayakaOno commented Aug 30, 2024

View reviewed changes

packages/webapp/src/containers/Animals/AddAnimals/utils.ts Show resolved Hide resolved

packages/webapp/src/containers/Animals/AddAnimals/utils.ts Outdated Show resolved Hide resolved

packages/webapp/src/containers/Animals/AddAnimals/utils.ts Outdated Show resolved Hide resolved

LF-4166 Create OrganicStatus type

ad2b487

kathyavini previously approved these changes Aug 30, 2024

View reviewed changes

SayakaOno dismissed kathyavini’s stale review via ad2b487 August 30, 2024 20:45

LF-4166 /animal_type_use_relationship

e7e4078

kathyavini previously approved these changes Aug 30, 2024

View reviewed changes

SayakaOno added 2 commits August 30, 2024 15:50

Filter uses in animal details

1211b98

Adjust add animal summary image width

f73d909

SayakaOno mentioned this pull request Aug 30, 2024

Add animal fixes #3402

Merged

16 tasks

Merge pull request #3402 from LiteFarmOrg/Add_animal_fix

79566c6

Add animal fixes

Duncan-Brain dismissed kathyavini’s stale review via 79566c6 September 3, 2024 18:17

Duncan-Brain approved these changes Sep 3, 2024

View reviewed changes

kathyavini approved these changes Sep 3, 2024

View reviewed changes

kathyavini added this pull request to the merge queue Sep 3, 2024

Merged via the queue into integration with commit cba7e47 Sep 3, 2024
4 of 5 checks passed

kathyavini mentioned this pull request Sep 3, 2024

Discussion - Radio Options #3408

Draft

16 tasks

SayakaOno mentioned this pull request Oct 3, 2024

LF-4166(5): Add add animals success snackbar #3488

Merged

16 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LF-4166 (4): Implement end to end animal creation flow #3397

LF-4166 (4): Implement end to end animal creation flow #3397

SayakaOno commented Aug 27, 2024 •

edited

Loading

Duncan-Brain Aug 29, 2024

SayakaOno Aug 29, 2024

Duncan-Brain left a comment

Duncan-Brain Aug 29, 2024

SayakaOno Aug 29, 2024

SayakaOno left a comment

SayakaOno Aug 29, 2024

kathyavini left a comment •

edited

Loading

SayakaOno left a comment

kathyavini left a comment •

edited

Loading

SayakaOno commented Aug 30, 2024

kathyavini left a comment •

edited

Loading

SayakaOno commented Aug 30, 2024

Duncan-Brain left a comment

kathyavini left a comment

LF-4166 (4): Implement end to end animal creation flow #3397

LF-4166 (4): Implement end to end animal creation flow #3397

Conversation

SayakaOno commented Aug 27, 2024 • edited Loading

Duncan-Brain Aug 29, 2024

Choose a reason for hiding this comment

SayakaOno Aug 29, 2024

Choose a reason for hiding this comment

Duncan-Brain left a comment

Choose a reason for hiding this comment

Duncan-Brain Aug 29, 2024

Choose a reason for hiding this comment

SayakaOno Aug 29, 2024

Choose a reason for hiding this comment

SayakaOno left a comment

Choose a reason for hiding this comment

SayakaOno Aug 29, 2024

Choose a reason for hiding this comment

kathyavini left a comment • edited Loading

Choose a reason for hiding this comment

SayakaOno left a comment

Choose a reason for hiding this comment

kathyavini left a comment • edited Loading

Choose a reason for hiding this comment

SayakaOno commented Aug 30, 2024

kathyavini left a comment • edited Loading

Choose a reason for hiding this comment

SayakaOno commented Aug 30, 2024

Duncan-Brain left a comment

Choose a reason for hiding this comment

kathyavini left a comment

Choose a reason for hiding this comment

SayakaOno commented Aug 27, 2024 •

edited

Loading

kathyavini left a comment •

edited

Loading

kathyavini left a comment •

edited

Loading

kathyavini left a comment •

edited

Loading