LF-4528: Hide created in error option for animals with completed tasks

[SayakaOno]

Description

• Update animal/batch models and controllers to include tasks (task_ids) in animals and batches
• Update RemoveAnimalsModal to accept hideDeleteOption prop and hide the "Created in error" option
• Update animal and batch types to add tasks
• Update useAnimalInventory hook to include tasks
• Update useAnimalOrBatchRemoval hook to check if selected animals/batches have abandoned or completed tasks
• Send hideDeleteOption prop to RemoveAnimalsModal from AnimalInventory to hide the delete option

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

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

[antsgar]

Should we do this the other way round and iterate over the selected IDs and check if the corresponding entity exists in the inventory? The selected IDs would be a subset of the whole inventory so I think that should be faster than checking the whole inventory

[SayakaOno]

I initially considered iterating over selectedInventoryIds instead of inventory as suggested, and I came up with two approaches:

1. Format inventory into a Map:
   1. Convert inventory ([{id, tasks}, {id, tasks}]) to a map like {[id]: tasks, [id]: tasks}
   2. Then iterate over selectedInventoryIds to check if the corresponding tasks exist:

   for (let id of selectedInventoryIds) {
        if (inventoryMap[id].length) {
          return true;
        }
     }
   

2. Directly iterate selectedInventoryIds without formatting:

   for (let selectedInventoryId of selectedInventoryIds) {
      const { tasks } = inventory.find(({ id }) => id === selectedInventoryId);
      if (tasks.length) {
         return true;
      }
   }
   

With the first approach, we’d be iterating over inventory, so it might be more efficient to return the result early while iterating. The second approach felt a bit less efficient, though it should work fine for this feature.

Which option would you go with?

[antsgar]

Actually now that you described the two I think your implementation is probably the most performant one! I think option 2 presented here would be the simplest one / easier to read, but I'd keep this as is

[SayakaOno]

Resolved conflicts and adjusted the useAnimalOrBatchRemoval hook to ensure compatibility with SingleAnimalView.

[kathyavini]

This logic looks great and also really efficient ☺️

The one issue I am running into is that the property tasks on animals/batches is not populated if I add the task after I've last fetched my animal inventory, and then whether I complete the task or not, the 'Created in Error' option persists since it depends on the nested task list.

The easiest fix sounds like just a tag invalidation on animals when the task is created, but I wanted to check with you if this sounds like the right approach based on what you were thinking of the nested tasks array and its other uses.

For instance, I don't know how that compares to never nesting the property on the API level and assocating entirely on the frontend (not via selector... that has been nightmare!.. but just associating via logic here). This would avoid having to re-pull animals every time a Movement Task is created or a Custom Task is created, but it would be more complex logic compared to the re-fetch 🤔

[SayakaOno]

Thank you Joyce!
I forgot to actually do it, but I was thinking about invalidating animals/batches when a task is created. I'm assuming there is a way to update nested properties with RTK query, but I prefer tag invalidation until we fully utilize RTK query for tasks.

Could you elaborate on your idea of associating animals with tasks on the frontend? Were you thinking of creating a dedicated API to fetch these relationships?

[kathyavini]

@SayakaOno I was thinking of an actual search on the animals/animal_batches property of the completed tasks + abandoned tasks! I wasn't thinking of a dedicated API, would that be more efficient? 😅

I guess my logic was that the tasks are already up to date, and the animals are already up to date, so it shouldn't be necessary to request anything more from the API. Animals is also going to be a fairly large request (potentially). But I don't necessarily mind re-fetching because I think we have been in favour of making the backend do complicated computations and with the size of tasks, this probably qualifies!

I guess the two in-between options would be a dedicated API as you said, or maybe a selector...? Does RTK (regular, not query) have some properties that would make a "tasksByAnimalId" selector more efficient?

I'm fine with the tag invalidation / re-fetch for now, though. Maybe we could table those other approaches as possibilities if it seems we want to optimize later?

[kathyavini]

Sorry it was sort of review scope creep on my part to bring up the refetching here, it can definitely be a different ticket since the logic for the Created in Error good! 🙏