LF-4384: End to end animal details #3534

SayakaOno · 2024-11-19T18:56:12Z

Description

Add missing desiredKeys to animalBatchController's editAnimalBatches so that all fields sent to the backend are updated
Update getRecordIfExists middleware helper to exclude removed animals
- Prevents removed animals/batches from being updated
Update WithStepperProgressBar not to use FixedHeaderContainer for single step forms which don't have headers
- Prevents two scrollbars from appearing in SingleAnimalView
Update MeatballsMenu and DropdownButton to accept disabled prop
Create CustomRouteComponentProps type to override the default history from react-router with the history object from the History library
Update parseUniqueDefaultId to handle exception (it was returning NaN when typing a new animal type, causing filteredUses - the animal use options - to be undefined)
Adjust the batch count width in AnimalSingleViewHeader. (Counts with more than five digits are not properly handled, but if necessary, we can create a new ticket to address this)
Update useAnimalOrBatchRemoval to return the API result
- Allows SingleAnimalView to handle actions based on the result
Update SingleAnimalView to:
- make the page full-width
- use FixedHeaderContainer
- implement functionality for animal removal
- replace history.push with history.back for navigation
- redirect to UnknownRecord when no animal or batch is found for the URL
- disable the menu button when editing
- hide meatballs menu for removed animals to prevent invalid actions (edit, remove)

NOTE: I didn't implement the floating button, as it was mentioned as a nice-to-have in the Jira ticket.

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

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

… for single step form

… the URL

…iting

…Batches

(fix filtered animalUseOptions)

…oved animals

…ack only after successful API call

Duncan-Brain · 2024-11-23T00:43:57Z

Sorry I haven't gotten very far yet but the uses in read-only seems to not be visible on first visit. On refresh it shows up.

antsgar

Left a few minor comments but this is great, thanks for catching all the little details that were missing to make the feature work!

When an animal is removed from the single animal view, the success toast reads "Successfully removed selected animals". Should we change this to "animal" singular when there's only one animal?

antsgar · 2024-11-25T12:54:12Z

packages/webapp/src/components/Animals/AnimalSingleViewHeader/index.tsx

-          options={options}
-          classes={{ button: isEditing ? styles.editingStatusButton : '' }}
-        />
+        {!hideMenu && (


NIT but I'd probably reverse the logic here and have a showMenu prop with a default true, just because the combo of hide + negation seems a bit counter intuitive to me

antsgar · 2024-11-25T14:00:10Z

packages/webapp/src/components/Form/ContextForm/WithStepperProgressBar.tsx

-        )
-      }
-    >
+  const renderContent = () => (


NIT and not prescriptive, but I'm not a huge fan of render functions because they're "undercover" functional components minus reusability and ability to incorporate state. I'd make this into its own component even if it lives within this same file

I wasn't sure about this... I created a wrapper component, thank you!

antsgar · 2024-11-25T14:02:36Z

packages/webapp/src/containers/Animals/SingleAnimalView/index.tsx

+
+  const { onConfirmRemoveAnimals, removalModalOpen, setRemovalModalOpen } = useAnimalOrBatchRemoval(
+    [getInventoryId()],
+    () => ({}),


Should this callback just be an optional parameter for the hook?

I didn't feel like adding ? all over the hook 😅 but I agree that's the right thing to do!
I’ve fixed this for now, and the refactor should tidy things up a bit!

antsgar · 2024-11-25T14:03:05Z

packages/webapp/src/containers/Animals/SingleAnimalView/index.tsx

+  };
+
+  const getInventoryId = () => {
+    if (!selectedAnimal && !selectedBatch) {


Since there's a redirect to another page on the effect above if the batch or animal doesn't exist, is this check necessary?

I think I was hoping to avoid doing (selectedAnimal || selectedBatch)! on line 175. It didn't work as expected, so I removed it!

antsgar · 2024-11-25T14:31:12Z

packages/webapp/src/containers/Animals/SingleAnimalView/index.tsx

+  };
+
+  const { onConfirmRemoveAnimals, removalModalOpen, setRemovalModalOpen } = useAnimalOrBatchRemoval(
+    [getInventoryId()],


Perhaps we could modify useAnimalOrBatchRemoval to receive the actual entity IDs and whether they're a batch or animal instead of the inventory IDs? Seems more straight forward since here we're generating the inventory ID just to pass it to the hook, and then the hook has to re-parse the actual IDs from it

I hesitated to refactor since it would involve changes to an unrelated file, but it’s true that I wasn’t happy with the inefficiency. Let me open another PR for this!

* Update useInitialAnimalData hook - return isFetchingAnimalsOrBatches - return defaultFormValues when ready * Update SingleAnimalView - avoid redirecting to UnknownRecord during fetching animals - render form when default values are ready

…hRemoval hook

SayakaOno

Thank you for reviewing!!

The bug Duncan found was caused by defaultValue being set after the ReactSelect was rendered. I’ve updated it to render the form only after the default values are ready.(commit)

Regarding making the success message singular, I noticed Denis created a bug ticket for the inventory view, so I'll leave the fix for that ticket.

I plan to refactor the useAnimalOrBatchRemoval hook in a separate PR, but this one should now be ready for re-reviews. Thank you! 🙏

Update: Here is the PR to refactor the hook

SayakaOno · 2024-11-25T22:58:10Z

packages/webapp/src/components/Form/ContextForm/WithStepperProgressBar.tsx

-        )
-      }
-    >
+  const renderContent = () => (


I wasn't sure about this... I created a wrapper component, thank you!

SayakaOno · 2024-11-25T23:03:27Z

packages/webapp/src/containers/Animals/SingleAnimalView/index.tsx

+  };
+
+  const getInventoryId = () => {
+    if (!selectedAnimal && !selectedBatch) {


I think I was hoping to avoid doing (selectedAnimal || selectedBatch)! on line 175. It didn't work as expected, so I removed it!

SayakaOno · 2024-11-25T23:15:17Z

packages/webapp/src/containers/Animals/SingleAnimalView/index.tsx

+  };
+
+  const { onConfirmRemoveAnimals, removalModalOpen, setRemovalModalOpen } = useAnimalOrBatchRemoval(
+    [getInventoryId()],


I hesitated to refactor since it would involve changes to an unrelated file, but it’s true that I wasn’t happy with the inefficiency. Let me open another PR for this!

SayakaOno · 2024-11-25T23:23:05Z

packages/webapp/src/containers/Animals/SingleAnimalView/index.tsx

+
+  const { onConfirmRemoveAnimals, removalModalOpen, setRemovalModalOpen } = useAnimalOrBatchRemoval(
+    [getInventoryId()],
+    () => ({}),


I didn't feel like adding ? all over the hook 😅 but I agree that's the right thing to do!
I’ve fixed this for now, and the refactor should tidy things up a bit!

kathyavini · 2024-11-26T17:22:47Z

packages/webapp/src/types/index.ts

 export enum OrganicStatus {
  'NON_ORGANIC' = 'Non-Organic',
  'TRANSITIONAL' = 'Transitional',
  'ORGANIC' = 'Organic',
 }
+
+// Custom RouteComponentProps with a custom history type from the 'history' library
+export type CustomRouteComponentProps<T extends { [K in keyof T]?: string | undefined }> = Omit<


Thank you!! ❤️

Duncan-Brain

@SayakaOno

Everything is looks great to me thanks for wrapping up! Left a couple small inline comments which aren't blockers.

~~Only other thing I saw not working was the upload image functionality -- not sure if that is addressable here.~~ nvm gotta start bucket 😂

Feel free to merge!

packages/webapp/src/components/Animals/AnimalSingleViewHeader/styles.module.scss

packages/webapp/src/containers/Animals/Inventory/useAnimalOrBatchRemoval.ts

Duncan-Brain · 2024-11-26T18:10:09Z

packages/webapp/src/components/Form/ContextForm/WithStepperProgressBar.tsx

@@ -109,6 +117,7 @@ export const WithStepperProgressBar = ({
    if (isFinalStep) {
      setIsSaving(true);
      await handleSubmit((data: FieldValues) => onSave(data, onGoForward, setFormResultData))();
+      reset(getValues());


At first I was worried that this would clear the edited values when unsuccessful (api is down). But instead It kept them? Do you think this is expected?

Screen.Recording.2024-11-26.at.1.07.32.PM.mov

reset() will reset the form to the default values. However, reset(getValues()) updates the default values to the current form values before resetting, so this behaviour is expected. You can find a brief explanation in the RULES section on this page.

Thank you Duncan for thoroughly testing this, I don't think reset, setIsSaving and setIsEditing shouldn't be called unless the API call is successful. Let me see if I can fix this!

Interesting use of reset! and sounds good!

* handle onGoForward within the component * properly handle success actions

SayakaOno · 2024-11-27T18:07:07Z

I made some additional changes, thank you!

Conflicts were resolved.
Revert the onSave change in ContextForm. (removing onGoForward should prevent the crash)
Update AnimalBreedSelect to replace undefined with null as value

SayakaOno · 2024-11-27T18:09:02Z

packages/webapp/src/components/Animals/AddAnimalsFormCard/AnimalSelect.tsx

@@ -96,7 +96,7 @@ export function AnimalBreedSelect<T extends FieldValues>({
          placeholder={isTypeSelected ? undefined : t('ADD_ANIMAL.BREED_PLACEHOLDER_DISABLED')}
          isDisabled={!isTypeSelected || isDisabled}
          onChange={(option) => onChange(option)}
-          value={value}
+          value={value || null}


undefined doesn't clear the select, but null does.

Screen.Recording.2024-11-27.at.9.53.27.AM.mov

SayakaOno · 2024-11-27T18:12:44Z

packages/webapp/src/components/Form/ContextForm/index.tsx

@@ -108,10 +107,6 @@ export const ContextForm = ({
      setIsEditing={setIsEditing}
      showCancelFlow={showCancelFlow}
      setShowCancelFlow={setShowCancelFlow}
-      onSave={(...args: any) => {
-        setActiveStepIndex(0);


I found this has changed behaviour of the other form (it displays the first page of the form for a second). I believe it's working without it, but if any problems are found, we will need another solution.

Screen.Recording.2024-11-27.at.9.45.35.AM.mov

Duncan-Brain

Thanks for fixing error behaviour!

Only thing I think I see now is that for some reason the sex_detail input is not allowing enabled in edit mode unless count is changed. We could probably make a bug ticket and just merge this though!

SayakaOno · 2024-11-27T19:20:34Z

Thank you @Duncan-Brain! I think that's one of the things Denis was mentioning yesterday, let me check with him!

antsgar · 2024-11-28T16:20:27Z

@SayakaOno I'm merging this one and we can create a separate one for the bug if that's okay!

SayakaOno added enhancement New feature or request Blocked labels Nov 19, 2024

SayakaOno self-assigned this Nov 19, 2024

SayakaOno changed the title ~~[WIP] LF-4384: End to end animal details~~ LF-4384: End to end animal details Nov 20, 2024

Base automatically changed from LF-4383-animal-details-form-container-handle-edit-of-animal-details to integration November 21, 2024 14:27

SayakaOno added 19 commits November 21, 2024 08:25

LF-4384 Update WithStepperProgressBar not to use FixedHeaderContainer…

d40e520

… for single step form

LF-4384 Add animals url to FULL_WIDTH_ROUTES in App

aab542c

LF-4384 Refactor WithStepperProgressBar

5c390de

LF-4384 Update SingleAnimalView to use FixedHeaderContainer

45ba590

LF-4384 Implement animal removal

2632814

LF-4384 Create custom RouteComponentProps type

b7b76c5

LF-4384 Replace RouteComponentProps with CustomRouteComponentProps

4248a6b

LF-4384 Replace history.push with history.back in SingleAnimalView

2f1ac45

LF-4384 Redirect to unknown record screen when no animal or batch for…

d8e013a

… the URL

LF-4384 Update MeatballsMenu and DropdownButton to accept disabled prop

ace818d

LF-4384 Update AnimalSingleViewHeader to disable menu button while ed…

4ac69a8

…iting

LF-4384 Add missing desiredKeys to animalBatchController's editAnimal…

6054d79

…Batches

LF-4384 Update parseUniqueDefaultId to handle exception

3f72c80

(fix filtered animalUseOptions)

LF-4384 Add styles to batch count in AnimalSingleViewHeader

b3edb84

LF-4384 Adjust batch count width in AnimalSingleViewHeader

f29364a

LF-4384 Update getRecordIfExists middleware helper not to include rem…

a5452e1

…oved animals

LF-4384 Update useAnimalOrBatchRemoval to return result

ad65572

LF-4384 Update onConfirmRemoval in SingleAnimalView to call history.b…

8c43213

…ack only after successful API call

LF-4384 Hide meatballs menu for removed animals

b9d72e3

SayakaOno force-pushed the LF-4384/End-to-End_Animal_Details branch from 8b561f3 to b9d72e3 Compare November 21, 2024 16:29

SayakaOno marked this pull request as ready for review November 21, 2024 16:30

SayakaOno requested review from a team as code owners November 21, 2024 16:30

SayakaOno requested review from antsgar and removed request for a team November 21, 2024 16:30

antsgar reviewed Nov 25, 2024

View reviewed changes

SayakaOno added 5 commits November 25, 2024 14:19

LF-4384 Update prop name hideMenu to showMenu for showing meatballs menu

9241b04

LF-4384 Refactor and create StepperProgressBarWrapper

4d9fcc6

LF-4384 Make setSelectedInventoryIds prop optional in useAnimalOrBatc…

a46d530

…hRemoval hook

LF-4384 Remove unnecessary code in SingleAnimalView

9021d49

SayakaOno commented Nov 25, 2024

View reviewed changes

SayakaOno mentioned this pull request Nov 26, 2024

[Not urgent!] LF-4384(b): Refactor useAnimalOrBatchRemoval #3549

Open

antsgar self-requested a review November 26, 2024 15:04

antsgar previously approved these changes Nov 26, 2024

View reviewed changes

kathyavini reviewed Nov 26, 2024

View reviewed changes

Duncan-Brain previously approved these changes Nov 26, 2024

View reviewed changes

packages/webapp/src/components/Animals/AnimalSingleViewHeader/styles.module.scss Show resolved Hide resolved

packages/webapp/src/containers/Animals/Inventory/useAnimalOrBatchRemoval.ts Show resolved Hide resolved

Duncan-Brain reviewed Nov 26, 2024

View reviewed changes

SayakaOno added 4 commits November 26, 2024 16:07

Merge branch 'integration' into LF-4384/End-to-End_Animal_Details

b295005

LF-4384 Update onSave callback in WithStepperProgressBar

d7ed709

* handle onGoForward within the component * properly handle success actions

LF-4384 Remove setActiveStepIndex(0) in onSave in ContextForm

ebde1a2

LF-4384 Update AnimalBreedSelect to replace undefined with null as value

2325f80

SayakaOno dismissed stale reviews from Duncan-Brain and antsgar via 2325f80 November 27, 2024 17:58

SayakaOno commented Nov 27, 2024

View reviewed changes

Duncan-Brain approved these changes Nov 27, 2024

View reviewed changes

antsgar added this pull request to the merge queue Nov 28, 2024

Merged via the queue into integration with commit bd04b17 Nov 28, 2024
5 checks passed

antsgar deleted the LF-4384/End-to-End_Animal_Details branch November 28, 2024 16:21

SayakaOno mentioned this pull request Nov 28, 2024

LF-4549: Sex-Detail Field Not Editable on Edit Animal Details Page #3555

Merged

16 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LF-4384: End to end animal details #3534

LF-4384: End to end animal details #3534

SayakaOno commented Nov 19, 2024 •

edited

Loading

Duncan-Brain commented Nov 23, 2024 •

edited

Loading

antsgar left a comment

antsgar Nov 25, 2024

antsgar Nov 25, 2024

SayakaOno Nov 25, 2024

antsgar Nov 25, 2024

SayakaOno Nov 25, 2024

antsgar Nov 25, 2024

SayakaOno Nov 25, 2024

antsgar Nov 25, 2024

SayakaOno Nov 25, 2024

SayakaOno left a comment •

edited

Loading

SayakaOno Nov 25, 2024

SayakaOno Nov 25, 2024

SayakaOno Nov 25, 2024

SayakaOno Nov 25, 2024

kathyavini Nov 26, 2024

Duncan-Brain left a comment •

edited

Loading

Duncan-Brain Nov 26, 2024 •

edited

Loading

SayakaOno Nov 27, 2024

Duncan-Brain Nov 27, 2024

SayakaOno commented Nov 27, 2024

SayakaOno Nov 27, 2024

SayakaOno Nov 27, 2024

Duncan-Brain left a comment

SayakaOno commented Nov 27, 2024

antsgar commented Nov 28, 2024

LF-4384: End to end animal details #3534

LF-4384: End to end animal details #3534

Conversation

SayakaOno commented Nov 19, 2024 • edited Loading

Duncan-Brain commented Nov 23, 2024 • edited Loading

antsgar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SayakaOno left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Duncan-Brain left a comment • edited Loading

Choose a reason for hiding this comment

Duncan-Brain Nov 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SayakaOno commented Nov 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Duncan-Brain left a comment

Choose a reason for hiding this comment

SayakaOno commented Nov 27, 2024

antsgar commented Nov 28, 2024

SayakaOno commented Nov 19, 2024 •

edited

Loading

Duncan-Brain commented Nov 23, 2024 •

edited

Loading

SayakaOno left a comment •

edited

Loading

Duncan-Brain left a comment •

edited

Loading

Duncan-Brain Nov 26, 2024 •

edited

Loading