Skip to content

LF-5274 Add animal batch selector with quantity and sale value to the revenue entry form#4170

Open
kathyavini wants to merge 37 commits into
integrationfrom
LF-5274-add-animal-batch-selector-with-quantity-and-sale-value-to-the-revenue-entry-form
Open

LF-5274 Add animal batch selector with quantity and sale value to the revenue entry form#4170
kathyavini wants to merge 37 commits into
integrationfrom
LF-5274-add-animal-batch-selector-with-quantity-and-sale-value-to-the-revenue-entry-form

Conversation

@kathyavini
Copy link
Copy Markdown
Collaborator

@kathyavini kathyavini commented May 14, 2026

Description

Completes the frontend for Animal-associated Revenues / Sales:

  1. Adds the AnimalSaleInputs container and AnimalSaleItem components mirroring their crop versions
  2. Updates the CropSaleTable shown within the transaction expandable to a more general EntitySaleTable (very simple change; only needed adjust was a label)
  3. Adds animal logical branches to useTransactions and the formatting functions mapSalesToRevenueItems (Transactions, Actual Revenue), and mapRevenueFormDataToApiCallFormat
  4. Removes the filter on and completes the frontend (title + description) for the new Animal Sales default revenue type

This PR also makes a few visual adjusts that were present on Figma or discussed in dev-design:

  • Notes field is now a Text Area with dynamic placeholder based on entity type
  • Footer of EntitySaleTable has been updated as discussed
  • The multi-selects for crops and animals have updated labels and placeholders relative to the original form

Additionally, this PR resolves the naming/folder structure issues discussed in #4150 as follows (after quite some rounds of iteration as you can see in the commit history 😜)

  • AnimalSaleInputs and CropSaleInputs have been gathered under a new Finances/EntitySaleInputs/ folder
  • The default component export from that Folder <EntitySaleInputs> fetches and shows either the Crop or Animal inputs based on the enitity type (as AI had originally proposed)
    • However rather than being passed as an unconditional prop, <EntitySaleInputs> is simply imported directly into the Revenue Form
  • Consequently, the form HAS been renamed from <GeneralRevenue> to <RevenueForm>
    • I think it's a good change overall, but I have to admit I still catch myself typing GeneralRevenue all the time out of habit! Hopefully only an issue right now since I've been working in these files for the last two weeks 🤞
    • Note: the entitySaleDefaultValues (only relevant for RevenueDetails / existing records) are still passed as a prop. This keeps them available for the initial useForm call, which doesn't update on re-render
  • If you recall from the first pass refactor on the other branch, CropSaleInputs used to wrap an entity-agnostic container called <EntitySaleInputs> (which held the multi-select and its relationship to the following sale item rows); this structure and naming felt incorrect, because as a name, CropSaleInputs feels more like a child/type/peer of EntitySaleInputs rather than a parent of it. Now the inner component has been made strictly a proper component, is called EntitySalePicker, and resides within components/Form/RevenueForm
    • I apologize if you have created any sort of mental model based on the EntitySaleInputs naming from the previous PR, but I think this will be better moving forward!

Finally, there is a small adjust for the Storybook crash:

ReferenceError: Cannot access 'loginSelector' before initialization at alertSlice.js:45:35

Claude (Opus -- Sonnet was clueless!) traced this crash to a circular/race import of the Redux store, specifically userFarmSlice importing from util/ which itself imports from the (somewhat sketchy) getFromReduxStore/ with a top-level store import. I think selecting from userFarm at two levels within the current flow made reloading on the GeneralRevenue (oops, I mean RevenueForm 😅) Story a particularly easy way to create this crash, although I think some other useFarmSlice-calling components in Storybook could potentially have triggered it as well. In any case, it requires loading the store via component import which can only happen in Storybook; in app the store is provided top-level and I can't imagine a situation where the circular import could matter... but I'm very curious to know if I'm wrong about that! 😁 @SayakaOno you said you saw it in App once?

It does seem to be fully resolved now and I haven't seen it since the import fix. (Fix was the trivial change of moving the utility that userFarmSlice needs outside of the default utils/index.js file).

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

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

kathyavini and others added 30 commits May 11, 2026 13:41
Mirrors CropSaleItem contract. Renders the animal/batch name label
above a SaleLineItem for the shared quantity + sale value inputs.
No image or tile (the animal/batch row carries only a string label).
Does not register the entity-id field itself — that registration is
performed by SaleLineItem via entityIdFieldKey.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
ANIMAL_SALE matches the backend animal_sale junction table name and
is the RHF field-prefix key for the animal sale input section.
ANIMAL_KEY is the per-row entity-id field name; it stays generic
(not animal_id) because option values are prefixed strings encoding
either an animal_id or animal_batch_id on the same row.

Also adds ANIMAL_SALE to REVENUE_FORM_TYPES so util.getRevenueFormType
can dispatch on it.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Mirrors the simplified CropSaleInputs interface ({ sale?, disabledInput }).
The component only mounts when the selected revenue type has
entity_type === 'animal' (gated upstream by GeneralRevenue), so no
isActive flag and no skip on the RTK Query hooks.

Animals and batches are merged into a single option list with values
prefixed (animal_${id} / batch_${id}) so a single CheckboxMultiSelect
can represent both kinds of entity on one row. The decode back to
animal_id / animal_batch_id happens in util.js.

Exports getAnimalSaleDefaultValues for the parent form's
customFormChildrenDefaultValues. quantity_unit is reshaped from a
string into a SelectOption via getUnitOptionMap so the Unit dropdown
populates correctly in edit/read-only views — same shape as
getCropSaleDefaultValues.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…nputs -> EntitySaleRows

The rows component (CheckboxMultiSelect + per-entity rows) is renamed
to EntitySaleRows so the more semantically correct name EntitySaleInputs
is conceptually free, with the actual dispatcher named
RevenueSaleInputs to align with the revenue domain.

RevenueSaleInputs receives the full prop set from GeneralRevenue
({ sale, disabledInput, revenueTypes, selectedTypeOption }), derives
the entity_type, and conditionally renders CropSaleInputs or
AnimalSaleInputs. It returns null as a defensive fallback —
GeneralRevenue already gates on selectedRevenueType?.entity_type so
this path is unreachable in practice.

Also exports getRevenueSaleDefaultValues(sale, entityType) so the
default-values switch lives in one place rather than being duplicated
in RevenueDetail and AddSale.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
getRevenueFormType: dispatch entity_type === 'animal' to
REVENUE_FORM_TYPES.ANIMAL_SALE.

mapRevenueFormDataToApiCallFormat: add an animal branch that decodes
the prefixed animal_key form value back to either animal_id or
animal_batch_id (exactly one is non-null per row, mirroring the
backend CHECK constraint).

mapSalesToRevenueItems: add an animal branch that displays each
animal_sale row similarly to crop_variety_sale rows (title from
chooseIdentification, subtitle as quantity + unit, amount as
sale_value). Extends the signature with optional animals /
animalBatches arrays; the caller in useTransactions wires them in
Step 24.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replaces CropSaleInputs with the new RevenueSaleInputs dispatcher so
the edit/read-only flow renders the correct entity-specific inputs
based on the sale's revenue type. The entity_type switch for default
values now lives in getRevenueSaleDefaultValues, removing the
isCropSale ternary here.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…LE type

AddSale now uses the RevenueSaleInputs dispatcher so newly created
sales render the entity-specific inputs depending on the selected
revenue type.

Also removes the temporary getRevenueTypesSaga filter that hid the
seeded ANIMAL_SALE revenue type from the dropdown. The frontend now
fully supports animal sales end-to-end, so the filter is obsolete.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds transactionTypeEnum.animalRevenue and extends
buildRevenueTransactions to dispatch animal-typed sales to it.

useTransactions now also reads animals and animal batches from
the RTK Query cache and forwards them to mapSalesToRevenueItems so
each animal_sale row in the transaction list can show the animal's
identification label as its title.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds the en source keys for the seeded ANIMAL_SALE revenue type
(REVENUE_NAME, CUSTOM_DESCRIPTION) and the UI strings used by the
animal sale inputs (SALE.ADD_SALE.ANIMAL placeholder, ANIMAL_REQUIRED
mirror of CROP_REQUIRED).

Only the English source files are touched; the other locales are
populated by the Crowdin pipeline.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Notes field showing as a TextArea now with entity-dependent placeholder; updated spacing; no <hr>
…xChars limit to FarmNotes and MarketDirectoryInfo TextAreas
Most likely I will revert this and return to a wrapping component
We looked at this in dev-design and realized 'DAILY TOTAL' should read 'TOTAL', and summing should only happen over values, not quantities
@kathyavini kathyavini self-assigned this May 14, 2026
@kathyavini kathyavini added enhancement New feature or request new translations New translations to be sent to CrowdIn are present labels May 14, 2026
import { createRevenueDetailsUrl } from '../../../../util/siteMapConstants';

const getColumns = (t, mobileView, totalAmount, quantityTotal, currencySymbol) => [
const getColumns = (t, titleLabel, mobileView, totalAmount, currencySymbol) => [
Copy link
Copy Markdown
Collaborator Author

@kathyavini kathyavini May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to making the table general to both crops and animals by passing the column title, this PR removes the quantity sum and changes "DAILY TOTAL" --> "TOTAL" as discussed in dev-design.

Image

Note: only the mobile view of this table is ever used in app (i.e. the footer is always defined by <FooterCell>, and never by the Footer property of getColumns); therefore the changes to Footer here in getColumns are just for completeness; they don't have any visual effect in app.

@@ -1,129 +0,0 @@
/*
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was not deleted, but rather renamed to RevenueForm/styles.module.scss, but I deleted so many unused CSS classes apparently the diffing algorithm could not figure out they were the same file!

@@ -0,0 +1,48 @@
/*
Copy link
Copy Markdown
Collaborator Author

@kathyavini kathyavini May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a new file, but rather a rename of GeneralRevenue/styles.module.scss. See note above!

Most of the original SCSS was not used anywhere.

export const DELETE_EXPENSE = 'DELETE_EXPENSE';
export const SET_IS_FETCHING_DATA = 'SET_IS_FETCHING_DATA';

export const REVENUE_FORM_TYPES = {
Copy link
Copy Markdown
Collaborator Author

@kathyavini kathyavini May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wasn't actually used anywhere in app. Same for the getRevenueFormType() that goes along with this constant (which makes sense since there is only the one form 😉)

Import chain is userFarm --> util/index.js (for getFirstNameWithLastInitial) --> getFromReduxStore. Not 100% sure, but I think the container layering in the new revenue components particularly revealed this with the calling of the measurementSelector from the inner EntitySalePicker component. Don't think this crash is relevant outside of Storybook though; depends on import of the store and the store provider is top-level in the actual app
Although the component > container > component sandwhich remains, I think this inner access of userFarm was the particular trigger that highlighted the circular Redux store import.
import { languageCodes } from '../../../hooks/useLanguageOptions';
import { getIntlDate } from '../../../util/date-migrate-TS';
import { getFirstNameWithLastInitial } from '../../../util';
import { getFirstNameWithLastInitial } from '../../../util/getFirstNameWithLastInitial';
Copy link
Copy Markdown
Collaborator Author

@kathyavini kathyavini May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to the Storybook crash, as described in the PR description.

…h-quantity-and-sale-value-to-the-revenue-entry-form
@kathyavini kathyavini changed the base branch from LF-5272/Replace_yes/no_crop_generated_field_with_crop/animal_toggle_on_custom_revenue_type_form to integration May 15, 2026 17:45
@kathyavini kathyavini marked this pull request as ready for review May 15, 2026 19:23
@kathyavini kathyavini requested review from a team as code owners May 15, 2026 19:23
@kathyavini kathyavini requested review from SayakaOno and removed request for a team May 15, 2026 19:23
Copy link
Copy Markdown
Collaborator

@SayakaOno SayakaOno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! I appreciated CustomFormChildren being removed - I think it made things cleaner 😄

Just curious, AnimalSaleDetail story doesn't render animal options or inputs, right?

ReferenceError: Cannot access 'loginSelector' before initialization at alertSlice.js:45:35

I think I've seen this kind of error when I accidentally created an incorrect selector with createSelector, so it wasn't a real issue 🙇‍♀️

Comment on lines +90 to +91
const selectedEntityType = selectedRevenueType?.entity_type;
const isEntitySale = selectedEntityType === 'crop' || selectedEntityType === 'animal';
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The util functions could be used 😄

Comment on lines +111 to +112
system,
currency,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

system and currency don't seem to be necessary since AnimalSaleInputs and CropSaleInputs can pass them directly.

Comment on lines +128 to +129
system={system}
currency={currency}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related to the comment in EntitySalePicker, I don't think EntitySalePicker has to pass system and currency props to children since they can be passed directly.

Comment on lines +101 to +103
return [...animalOptions, ...batchOptions].sort((a, b) =>
String(a.label).localeCompare(String(b.label)),
);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CheckboxMultiSelect has sorting logic, so this might not be necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request new translations New translations to be sent to CrowdIn are present

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants