LF-5276: Save and return which crops or animals each expense is linked to models endpoints tests by SayakaOno · Pull Request #4169 · LiteFarmOrg/LiteFarm

SayakaOno · 2026-05-11T22:45:13Z

Description

Add farmExpenseAnimalModel and farmExpenseCropVarietyModel
Update farmExpense relations
Add checkFarmExpense middleware for validation
- Create crop variety model method cropVarietiesBelongToFarm
- Extract common utility functions from checkSale
Update farmExpenseController to handle allocation modifications
Add tests
(Irrelevant to the ticket) Update GET response to return [] instead of 404 when there are no expenses
- Adjust saga

PATCH sample requests:

Delete existing animal expense

{ "farm_expense_animal": [] }

or

{ "farm_expense_animal": null }

or add farm_expense_crop_variety

{ "farm_expense_crop_variety": [...] }

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

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 ordered translation keys alphabetically (optional: run pnpm i18n to help with this)
I have added the GNU General Public License to all new files

…relations and cropVarietiesBelongToFarm Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

… allocations Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Use .length check so PATCH can send farm_expense_animal: [] with farm_expense_crop_variety items to transition between allocation types. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Remove the 404 workaround in the saga now that the API consistently returns 200 with an empty array for farms with no expenses. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…less than value (expense total)

kathyavini

Really nice! The middleware is very well organized and the controller logic looks great. Thank you for handling and thinking of so many required checks -- again I'm surprised by how much can go wrong when multiple farm entities are involved 😅

In testing in Postman I (accidentally 😬) found two points in the controllers that cause a full API crash; neither is from the new code and hopefully those are the only ones!

kathyavini · 2026-05-15T23:46:50Z

Not from this PR, but could this be moved inside the try/catch?

I crashed my backend when testing in Postman sending a non-existent expense_type_id; didn't expect that to bring the whole API down 😝

Additionally (for future safety) should baseController.isDeleted also return record?.deleted? Caller is not really set up to handle that case though (not deleted but not found)...

kathyavini · 2026-05-16T00:25:47Z

+  const isAddingAnimalExpense = !!farm_expense_animal?.length;
+  const isAddingCropVarietyExpense = !!farm_expense_crop_variety?.length;
+
+  if ((isNewExpense || !newValue) && !isAddingAnimalExpense && !isAddingCropVarietyExpense) {


Since values can be 0, I think it will have to be newValue === undefined rather than !newValue to avoid early return on checking the allocations for a value that's been updated to 0.

kathyavini · 2026-05-16T00:44:32Z

+
+interface AnimalExpenseItem {
+  id?: number;
+  farm_expense_id: number;


farm_expense_id is a uuid, but it looks like it's number in three of these types (and string in the 4th!)

kathyavini · 2026-05-16T00:45:51Z

+
+interface CropVarietyExpenseItem {
+  farm_expense_id: number;
+  crop_variety_id: string | null;


Can the crop_variety_id ever be null?

No 😅 Thank you! 🙏

kathyavini · 2026-05-16T01:02:46Z

Not this PR, but this caused a server crash in my testing! This needs a return before the res.status or sending an object will crash with a

Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client

I think the controller definitely needs fixing there anyway, but I wonder why Claude went with a more permissive pattern in the middleware that allows non-arrays, do you know? 🤔

// checkFarmExpense.ts const expenses = Array.isArray(body) ? body : [body];

The request body is an array in POST, but an object in PATCH.
I initially wondered that too 😂

kathyavini · 2026-05-19T16:26:20Z

+        const isAddingCropVarietyExpense = !!farm_expense_crop_variety?.length;
+        const isAddingAnimalExpense = !!farm_expense_animal?.length;
+
+        if (farm_expense_crop_variety !== undefined || isAddingAnimalExpense) {


I didn't understand the conditionals at first glance, but this is very clever! ❤️

kathyavini · 2026-05-19T16:38:04Z


        try {
-          ({ animalIds, batchIds } = getUniqueAnimalAndBatchIdsFromAnimalSale(animal_sale));
+          ({ animalIds, batchIds } = getUniqueAnimalAndBatchIds(animal_sale));


It's just for the error message, but looks like it needs the new 2nd parameter entityName here too

kathyavini · 2026-05-19T17:00:48Z

+          return res.status(400).send('value is required when creating a farm expense');
+        }
+
+        if (farm_expense_animal?.length) {


I think an array check will be needed here as well. insertGraph is happy to accept a single object as well as array, but since length is falsy the check won't run. So sending an object gets around the farm ownership check:

"farm_expense_animal": { "animal_id": 45, // another farm's animal ID "allocated_value": 150 }

Ohhh thank you!

kathyavini · 2026-05-19T17:40:36Z

+      });
+
+      test('Returns 400 if value is missing', async () => {
+        const { value: _value, ...expenseWithoutValue } = mocks.fakeExpense({


Did you get an unused variable linting error here? For me it's fine even without the rename to _value 🤔

AI did it, so I didn't check, but I'm not getting an error either. I guess I would if it were TypeScript though 🤔

SayakaOno

Thank you so much for catching all those errors!! I think I’ve addressed them all.
Could you please re-review? Thank you 🙏

SayakaOno · 2026-05-19T21:31:14Z

The request body is an array in POST, but an object in PATCH.
I initially wondered that too 😂

SayakaOno · 2026-05-19T21:44:07Z

+
+interface CropVarietyExpenseItem {
+  farm_expense_id: number;
+  crop_variety_id: string | null;


No 😅 Thank you! 🙏

SayakaOno · 2026-05-19T21:46:25Z

+          return res.status(400).send('value is required when creating a farm expense');
+        }
+
+        if (farm_expense_animal?.length) {


Ohhh thank you!

SayakaOno · 2026-05-19T22:15:44Z

+      });
+
+      test('Returns 400 if value is missing', async () => {
+        const { value: _value, ...expenseWithoutValue } = mocks.fakeExpense({


AI did it, so I didn't check, but I'm not getting an error either. I guess I would if it were TypeScript though 🤔

kathyavini

Looks good!

SayakaOno and others added 6 commits May 11, 2026 13:24

LF-5276 Add farmExpenseCropVariety and farmExpenseAnimal models; add …

2577400

…relations and cropVarietiesBelongToFarm Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

LF-5276 Add checkFarmExpense validation middleware

04865b9

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

LF-5276 Update farmExpense controller and route to persist and return…

454361d

… allocations Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

LF-5276 Fix mutual exclusivity check to allow empty allocation arrays

733c867

Use .length check so PATCH can send farm_expense_animal: [] with farm_expense_crop_variety items to transition between allocation types. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

LF-5276 Add mock factories and tests for expense entity allocations

31c82eb

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

LF-5276 Return empty array instead of 404 when farm has no expenses

cd653ee

Remove the 404 workaround in the saga now that the API consistently returns 200 with an empty array for farms with no expenses. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

SayakaOno self-assigned this May 11, 2026

SayakaOno added the enhancement New feature or request label May 11, 2026

SayakaOno added 10 commits May 12, 2026 09:21

LF-5276 Extract util functions from checkSale

bc752c0

LF-5276 Use util functions in checkSale

927a39c

LF-5276 Fix typo in file name

6fb48f9

LF-5276 Improve updateFarmExpense performance

df6e4e6

LF-5276 Use util functions and improve type in checkFarmExpense

a06464d

LF-5276 Improve updateFarmExpense and add/modify tests

e828c38

LF-5276 Fix updateFarmExpense response

daea7d0

LF-5276 Update checkFarmExpenseBody to validate values and adjust tests

cfab440

LF-5276 Add tests for expense value validations

26b0ad4

LF-5276 Fix patch response

b1b991c

SayakaOno changed the title ~~[WIP] LF-5276: Save and return which crops or animals each expense is linked to models endpoints tests~~ LF-5276: Save and return which crops or animals each expense is linked to models endpoints tests May 12, 2026

SayakaOno marked this pull request as ready for review May 12, 2026 22:03

SayakaOno requested review from a team as code owners May 12, 2026 22:03

SayakaOno requested review from kathyavini and removed request for a team May 12, 2026 22:03

SayakaOno marked this pull request as draft May 13, 2026 16:18

LF-5276 Update checkFarmExpenseBody to allow value 0 and allocations …

d192802

…less than value (expense total)

SayakaOno marked this pull request as ready for review May 13, 2026 17:47

kathyavini reviewed May 19, 2026

View reviewed changes

SayakaOno added 2 commits May 19, 2026 14:39

LF-5276 Prevent crash in farmExpenseController

fadebce

LF-5276 Fix types in checkFarmExpense

95a8dc9

SayakaOno added 2 commits May 19, 2026 14:59

LF-5276 Fix and add validations in checkFarmExpenseBody

8454706

LF-5276 Add missing argument for getUniqueAnimalAndBatchIds

ef44b62

SayakaOno commented May 19, 2026

View reviewed changes

kathyavini approved these changes May 19, 2026

View reviewed changes

kathyavini added this pull request to the merge queue May 19, 2026

Merged via the queue into integration with commit 633c0a8 May 19, 2026
6 checks passed

Conversation

SayakaOno commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kathyavini left a comment

Choose a reason for hiding this comment

Uh oh!

kathyavini May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kathyavini May 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kathyavini May 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SayakaOno left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kathyavini left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

SayakaOno commented May 11, 2026 •

edited

Loading

kathyavini May 15, 2026 •

edited

Loading

kathyavini May 16, 2026 •

edited

Loading

kathyavini May 16, 2026 •

edited

Loading