fix: update fyle_category to system_category for categories in the expense form#4113
fix: update fyle_category to system_category for categories in the expense form#4113
Conversation
WalkthroughVanakkam! Ithis a grand refactoring of the expense form template, thalaivaa! We have replaced camelCase property names with snake_case and switched the categorical logic from fyle_category to system_category throughout the form. The functionality remains intact—only the naming conventions have been revolutionized across multiple conditional blocks. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔁 Code Duplication Report - Angular
📊 No significant change in code duplication. |
| fg.controls.category?.value?.displayName && | ||
| autoCodedData.category === fg.controls.category.value.displayName | ||
| fg.controls.category?.value?.display_name && | ||
| autoCodedData.category === fg.controls.category.value.display_name |
There was a problem hiding this comment.
auto coded message was not being shown
| <app-virtual-select | ||
| [cacheName]="'recentCategoryList'" | ||
| [defaultLabelProp]="'displayName'" | ||
| [defaultLabelProp]="'display_name'" |
There was a problem hiding this comment.
no issue noticed due to this remaining as displayName but fixed anyways
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/app/fyle/add-edit-expense/add-edit-expense.page.html (2)
731-1100:⚠️ Potential issue | 🟠 MajorMass change needs proof, boss: add coverage + UI screenshots before merge.
This PR changes multiple category-gated UI branches, but no test updates are included in the provided diff. Please add targeted spec coverage for these conditions and attach before/after screenshots for the affected expense-form states.
I can draft the exact test matrix (categories × fields shown/hidden) if you want.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/fyle/add-edit-expense/add-edit-expense.page.html` around lines 731 - 1100, The PR changes multiple category-gated UI branches (template bindings around txnFields and fg controls such as flight_journey_travel_class, flight_return_travel_class, train_travel_class, bus_travel_class, from_dt/to_dt and location_1/location_2) but adds no test coverage or screenshots; add targeted unit/spec tests in the AddEditExpensePage (or its spec file) that assert which fields are rendered or hidden for representative system_category values (e.g., 'Taxi', flight/train/bus categories) and validation states (touched/invalid), and attach before/after UI screenshots for at least the main affected states (expanded/collapsed, category variations) to the PR so reviewers can verify visual and behavioral changes.
731-1027:⚠️ Potential issue | 🔴 CriticalThe systemCategories array is incomplete and will silently hide mandatory fields for most travel/category combinations.
The template guards at lines 731, 741, 760, 790, 799, 835, 882, 893, 922, 962, and 1027 check if a category's
system_categoryis in thesystemCategoriesarray returned byCategoriesService.getSystemCategories(). This array only includes['Bus', 'Airlines', 'Lodging', 'Train'], but the backend supports[Airlines, Food, Lodging, Mail, Mileage, Others, Per Diem, Taxi, Unspecified, activity].When a user selects a category like Taxi, Mileage, or Per Diem, these mandatory fields won't render because the
includes()check fails. Expenses with those categories will lose location, date, and travel class fields entirely—even if marked mandatory by transaction fields configuration.Update
getSystemCategories()andgetBreakfastSystemCategories()inCategoriesServiceto return the complete set of possiblesystem_categoryvalues from the backend.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/fyle/add-edit-expense/add-edit-expense.page.html` around lines 731 - 1027, The template hides mandatory fields because CategoriesService.getSystemCategories() (and getBreakfastSystemCategories()) returns an incomplete array; update these two methods to return the full set of backend-supported system_category values (Airlines, Food, Lodging, Mail, Mileage, Others, Per Diem, Taxi, Unspecified, activity) instead of ['Bus','Airlines','Lodging','Train'], and ensure the returned arrays are used where templates call systemCategories?.includes(...) so mandatory fields render correctly for Taxi/Mileage/Per Diem categories.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/app/fyle/add-edit-expense/add-edit-expense.page.html`:
- Around line 731-1100: The PR changes multiple category-gated UI branches
(template bindings around txnFields and fg controls such as
flight_journey_travel_class, flight_return_travel_class, train_travel_class,
bus_travel_class, from_dt/to_dt and location_1/location_2) but adds no test
coverage or screenshots; add targeted unit/spec tests in the AddEditExpensePage
(or its spec file) that assert which fields are rendered or hidden for
representative system_category values (e.g., 'Taxi', flight/train/bus
categories) and validation states (touched/invalid), and attach before/after UI
screenshots for at least the main affected states (expanded/collapsed, category
variations) to the PR so reviewers can verify visual and behavioral changes.
- Around line 731-1027: The template hides mandatory fields because
CategoriesService.getSystemCategories() (and getBreakfastSystemCategories())
returns an incomplete array; update these two methods to return the full set of
backend-supported system_category values (Airlines, Food, Lodging, Mail,
Mileage, Others, Per Diem, Taxi, Unspecified, activity) instead of
['Bus','Airlines','Lodging','Train'], and ensure the returned arrays are used
where templates call systemCategories?.includes(...) so mandatory fields render
correctly for Taxi/Mileage/Per Diem categories.
2 participants
Clickup
https://app.clickup.com/t/86d23b6r5
Code Coverage
Please add code coverage here
UI Preview
Please add screenshots for UI changes
Summary by CodeRabbit