org_category key cleanup #3928
org_category key cleanup #3928
Conversation
WalkthroughThe org_category field is removed across models and code paths. Category handling shifts to a nested object (tx.category with name). Type updates, mappings, routing, and telemetry now read category from category?.name. Affected areas include models, deep-link routing, transaction transformation, add/edit flows, dashboard tasks, and split expense. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant P as UI Page (Add/Edit/Dashboard)
participant S as DeepLinkService
participant R as Router
U->>P: Open expense item
P->>S: getExpenseRoute(etxn with tx.category?.name)
Note over P,S: Category source now: tx.category?.name (object-based)
S->>S: category = tx.category?.name?.toLowerCase()
alt category == "mileage"
S-->>R: route to add_edit_mileage
else category == "per diem"
S-->>R: route to add_edit_per_diem
else
S-->>R: route to add_edit_expense
end
R-->>P: Navigate to target page
P-->>U: Render target form
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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 |
…le-app into Fyle-org_category-clean-1
|
PR title must start with "fix:", "feat:", "chore:", "refactor:", or "test:" (case-insensitive) |
|
PR description must contain a link to a ClickUp (case-insensitive) |
…le-app into Fyle-org_category-clean-1
|
PR title must start with "fix:", "feat:", "chore:", "refactor:", or "test:" (case-insensitive) |
|
PR description must contain a link to a ClickUp (case-insensitive) |
|
PR title must start with "fix:", "feat:", "chore:", "refactor:", or "test:" (case-insensitive) |
|
PR description must contain a link to a ClickUp (case-insensitive) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/app/core/services/deep-link.service.spec.ts (1)
220-228: Fix object spread order to actually set the test categoryRight now,
...unflattenedTxnData.tx.categoryoverwritesname: item.category. Flip the order so the test asserts the intended category. Style with substance, like the Superstar.Apply this diff:
- category: { name: item.category, ...unflattenedTxnData.tx.category }, + category: { ...unflattenedTxnData.tx.category, name: item.category },src/app/fyle/add-edit-expense/add-edit-expense.page.ts (2)
4067-4073: Analytics: ensure Category is always populatedFallback keeps tracking clean even when
category.nameis absent. Style meets reliability.Apply this diff:
- Category: etxn.tx.category?.name, + Category: etxn.tx.category?.name ?? etxn.tx.fyle_category,
4397-4403: Tracking on create: same fallback for CategoryOne more place, same melody.
Apply this diff:
- Category: etxn.tx.category?.name, + Category: etxn.tx.category?.name ?? etxn.tx.fyle_category,src/app/fyle/add-edit-per-diem/add-edit-per-diem.page.ts (1)
604-611: Per Diem page asking for Mileage payment modes? Not today.Wrong expenseType skews allowed payment modes.
- const config = { + const config = { etxn, orgSettings, - expenseType: ExpenseType.MILEAGE, + expenseType: ExpenseType.PER_DIEM, isPaymentModeConfigurationsEnabled, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
src/app/core/models/public-policy-expense.model.ts(0 hunks)src/app/core/models/unformatted-transaction.model.ts(0 hunks)src/app/core/models/v1/transaction.model.ts(2 hunks)src/app/core/services/deep-link.service.spec.ts(1 hunks)src/app/core/services/deep-link.service.ts(1 hunks)src/app/core/services/transaction.service.ts(1 hunks)src/app/fyle/add-edit-expense/add-edit-expense.page.ts(4 hunks)src/app/fyle/add-edit-mileage/add-edit-mileage.page.ts(3 hunks)src/app/fyle/add-edit-per-diem/add-edit-per-diem.page.ts(3 hunks)src/app/fyle/dashboard/tasks/tasks.component.ts(1 hunks)src/app/fyle/split-expense/split-expense.page.ts(0 hunks)
💤 Files with no reviewable changes (3)
- src/app/core/models/public-policy-expense.model.ts
- src/app/core/models/unformatted-transaction.model.ts
- src/app/fyle/split-expense/split-expense.page.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{service,pipe,directive}.ts
📄 CodeRabbit inference engine (.cursor/rules/atomics-i18n-key-pattern.mdc)
**/*.{service,pipe,directive}.ts: i18n keys for services, pipes, and directives must use the schema: services.., pipes.., directives..
Convert kebab-case file names to camelCase and remove the suffix (Service, Pipe, Directive) when generating i18n keys for services, pipes, and directives
Use only the three roots: services, pipes, directives for i18n key grouping
Use meaningful, semantic, and context-aware key names for i18n keys; avoid generic names like message1, text1, error1
Files:
src/app/core/services/transaction.service.tssrc/app/core/services/deep-link.service.ts
**/*.component.{ts,html}
📄 CodeRabbit inference engine (.cursor/rules/component-i18n-key-naming.mdc)
**/*.component.{ts,html}: Top-level object in i18n translation files must be the component or feature folder name, derived from the file name by converting kebab-case to camelCase and removing the first matching prefix from: 'feature', 'ui', 'component'.
Keys inside the top-level object in i18n translation files must be free-form, not prefixed, and there must be no nested objects beyond the first level.
Use meaningful, semantic, and context-aware key names for i18n keys; avoid generic names like 'label1', 'text1', or 'message1'.
Do not translate strings that are already translation keys, tracking/analytics strings, strings with only special characters, comments, non-user-visible text, or variable bindings and expressions.
One object per component or feature in i18n translation files; avoid deep nesting and avoid prefixes like 'ui*' or 'feature*' in object names.
Files:
src/app/fyle/dashboard/tasks/tasks.component.ts
src/**/*.component.{ts,html}
📄 CodeRabbit inference engine (.cursor/rules/component-i18n-key-naming.mdc)
Translation keys for Angular components and templates located in 'src/**' must be placed in 'src/assets/i18n/{lang}.json' for every supported language.
Files:
src/app/fyle/dashboard/tasks/tasks.component.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
src/app/core/models/v1/transaction.model.ts (1)
101-101: Manual verification required: confirm ifcategoryis ever assignednullat runtime
Grep found no literalcategory = nullorcategory: nullin the codebase—please double-check your API payloads and form logic. Ifnullis indeed sent, widen the type to include| null; otherwise you can drop this change.
| getExpenseRoute(etxn: Partial<UnflattenedTransaction>): string[] { | ||
| const category = etxn.tx.org_category?.toLowerCase(); | ||
| const category = etxn.tx.category?.name?.toLowerCase(); | ||
| const canEditTxn = ['DRAFT', 'COMPLETE', 'APPROVER_PENDING'].includes(etxn.tx.state); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Graceful fallback when category.name is absent
Some older/edge payloads might miss category.name. Fall back to fyle_category to keep routing sharp.
Apply this diff:
- const category = etxn.tx.category?.name?.toLowerCase();
+ const category = (etxn.tx.category?.name ?? etxn.tx.fyle_category)?.toLowerCase();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| getExpenseRoute(etxn: Partial<UnflattenedTransaction>): string[] { | |
| const category = etxn.tx.org_category?.toLowerCase(); | |
| const category = etxn.tx.category?.name?.toLowerCase(); | |
| const canEditTxn = ['DRAFT', 'COMPLETE', 'APPROVER_PENDING'].includes(etxn.tx.state); | |
| getExpenseRoute(etxn: Partial<UnflattenedTransaction>): string[] { | |
| const category = (etxn.tx.category?.name ?? etxn.tx.fyle_category)?.toLowerCase(); | |
| const canEditTxn = ['DRAFT', 'COMPLETE', 'APPROVER_PENDING'].includes(etxn.tx.state); |
🤖 Prompt for AI Agents
In src/app/core/services/deep-link.service.ts around lines 135 to 137, the code
reads category = etxn.tx.category?.name?.toLowerCase() which breaks or yields
undefined for older payloads missing category.name; modify the logic to fall
back to etxn.tx.fyle_category (e.g., use etxn.tx.category?.name ??
etxn.tx.fyle_category) and then call toLowerCase() only when the resulting value
is defined (null-safe), so routing uses fyle_category when category.name is
absent while avoiding runtime errors.
| categoryDisplayName: expense.category?.display_name, | ||
| files: expense.files, | ||
| org_category: expense.category?.name, | ||
| category: expense.category, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Category object mapping: solid move, boss.
Switching to the full category object aligns the service with the new model and keeps consumers future-proof.
Run this quick scan to ensure there are no lingering references to org_category that could break callers:
🏁 Script executed:
#!/bin/bash
rg -n --type=ts -S '\borg_category\b|tx\.org_category\b' -g '!**/dist/**' -g '!**/node_modules/**'Length of output: 148
Boss, no files got scanned—let’s expand the search across all .ts files:
🏁 Script executed:
#!/bin/bash
rg -n '\borg_category\b|tx\.org_category\b' -g '**/*.ts' -g '!**/dist/**' -g '!**/node_modules/**'Length of output: 8483
Replace all org_category references in specs and mock-data with the new category object.
Boss, tests under src/app/fyle and data in src/app/core/mock-data still use tx.org_category—update them to use tx.category (e.g. tx.category.name) to match the service change.
🤖 Prompt for AI Agents
In src/app/core/services/transaction.service.ts around line 609, the service now
uses expense.category but many tests and mock fixtures still reference
tx.org_category; update all occurrences in specs under src/app/fyle and in mock
data under src/app/core/mock-data to use the new category object (e.g. replace
tx.org_category.name with tx.category.name, tx.org_category.id with
tx.category.id, or adjust to the nested shape used by the service), and make
corresponding changes to mock-data objects so they include a category property
with the expected fields; ensure tests/assertions and any type imports reflect
the renamed property.
| let category: string; | ||
|
|
||
| if (expense.tx.org_category) { | ||
| category = expense.tx.org_category.toLowerCase(); | ||
| if (expense.tx.category?.name) { | ||
| category = expense.tx.category.name.toLowerCase(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Simplify and harden category routing
Inline with a fallback to fyle_category so navigation doesn’t miss a beat.
Apply this diff:
- let category: string;
-
- if (expense.tx.category?.name) {
- category = expense.tx.category.name.toLowerCase();
- }
+ const category = (expense.tx.category?.name ?? expense.tx.fyle_category)?.toLowerCase();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let category: string; | |
| if (expense.tx.org_category) { | |
| category = expense.tx.org_category.toLowerCase(); | |
| if (expense.tx.category?.name) { | |
| category = expense.tx.category.name.toLowerCase(); | |
| } | |
| const category = (expense.tx.category?.name ?? expense.tx.fyle_category)?.toLowerCase(); |
🤖 Prompt for AI Agents
In src/app/fyle/add-edit-expense/add-edit-expense.page.ts around lines 2858 to
2862, the category extraction currently only sets category when
expense.tx.category.name exists; change it to always initialize category using
the category name with a fallback to expense.tx.fyle_category and defensively
handle undefined before calling toLowerCase. For example, set category to
(expense.tx.category?.name || expense.tx.fyle_category || '').toLowerCase() so
routing always has a lowercase string to use without throwing when values are
missing.
| Type: 'Receipt', | ||
| Amount: etxn.tx.amount, | ||
| Currency: etxn.tx.currency, | ||
| Category: etxn.tx.org_category, | ||
| Category: etxn.tx.category?.name, | ||
| Time_Spent: this.getTimeSpentOnPage() + ' secs', | ||
| Used_Autofilled_Category: | ||
| etxn.tx.category_id && this.presetCategoryId && etxn.tx.category_id === this.presetCategoryId, |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Tracking on add: same fallback for Category
Consistency, machi. Make the pipeline smile.
Apply this diff:
- Category: etxn.tx.category?.name,
+ Category: etxn.tx.category?.name ?? etxn.tx.fyle_category,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Type: 'Receipt', | |
| Amount: etxn.tx.amount, | |
| Currency: etxn.tx.currency, | |
| Category: etxn.tx.org_category, | |
| Category: etxn.tx.category?.name, | |
| Time_Spent: this.getTimeSpentOnPage() + ' secs', | |
| Used_Autofilled_Category: | |
| etxn.tx.category_id && this.presetCategoryId && etxn.tx.category_id === this.presetCategoryId, | |
| Type: 'Receipt', | |
| Amount: etxn.tx.amount, | |
| Currency: etxn.tx.currency, | |
| Category: etxn.tx.category?.name ?? etxn.tx.fyle_category, | |
| Time_Spent: this.getTimeSpentOnPage() + ' secs', | |
| Used_Autofilled_Category: | |
| etxn.tx.category_id && this.presetCategoryId && etxn.tx.category_id === this.presetCategoryId, |
🤖 Prompt for AI Agents
In src/app/fyle/add-edit-expense/add-edit-expense.page.ts around lines 4317 to
4323, the Category field on add uses etxn.tx.category?.name only; update it to
use the same fallback as elsewhere by setting Category to etxn.tx.category?.name
|| this.presetCategory?.name (or the equivalent null-coalescing expression used
in the file) so the preset category name is used when tx.category is absent,
leaving the rest of the object (Type, Amount, Currency, Time_Spent,
Used_Autofilled_Category) unchanged.
| let category: string; | ||
|
|
||
| if (expense.tx.org_category) { | ||
| category = expense.tx.org_category.toLowerCase(); | ||
| if (expense.tx.category?.name) { | ||
| category = expense.tx.category.name.toLowerCase(); | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Style with substance: fallback routing for category.
Mirror the per-diem hardening so navigation never guesses wrong.
- let category: string;
-
- if (expense.tx.category?.name) {
- category = expense.tx.category.name.toLowerCase();
- }
+ const category =
+ (expense.tx.category?.name || expense.tx.fyle_category || expense.tx.sub_category)?.toLowerCase();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let category: string; | |
| if (expense.tx.org_category) { | |
| category = expense.tx.org_category.toLowerCase(); | |
| if (expense.tx.category?.name) { | |
| category = expense.tx.category.name.toLowerCase(); | |
| } | |
| const category = | |
| (expense.tx.category?.name || expense.tx.fyle_category || expense.tx.sub_category)?.toLowerCase(); |
🤖 Prompt for AI Agents
In src/app/fyle/add-edit-mileage/add-edit-mileage.page.ts around lines 469-474,
the code conditionally sets category only when expense.tx.category?.name exists
which can leave category undefined and allow downstream routing to "guess";
change to always assign category using a safe fallback and lowercasing, e.g. set
category = (expense.tx.category?.name || '').toLowerCase(); or an explicit
fallback token like 'unknown' depending on routing expectations, so navigation
never relies on an undefined value.
| Amount: etxn.tx.amount, | ||
| Currency: etxn.tx.currency, | ||
| Category: etxn.tx.org_category, | ||
| Category: etxn.tx.category?.name, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Telemetry, kaboom! Never log an empty category.
Add a robust default for Category in edit/create tracking.
- Category: etxn.tx.category?.name,
+ Category: etxn.tx.category?.name || etxn.tx.fyle_category || 'Mileage',Also applies to: 2727-2727
🤖 Prompt for AI Agents
In src/app/fyle/add-edit-mileage/add-edit-mileage.page.ts around lines 2460 and
2727 the telemetry payload sets Category: etxn.tx.category?.name which can be
undefined or empty; replace this with a robust default by using a fallback
(e.g., etxn.tx.category?.name || 'Uncategorized' or a localized "No Category"
string) so the telemetry never logs an empty category value; ensure the fallback
is applied consistently in both locations.
|
|
||
| if (expense.tx.org_category) { | ||
| category = expense.tx.org_category.toLowerCase(); | ||
| if (expense.tx.category?.name) { | ||
| category = expense.tx.category.name.toLowerCase(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Good switch, but make it fail-proof, boss. Add a fallback for routing.
If category object is missing, navigation silently falls back to generic expense. Use fyle_category/sub_category as backup so review flow doesn’t misroute.
- let category: string;
-
- if (expense.tx.category?.name) {
- category = expense.tx.category.name.toLowerCase();
- }
+ const category =
+ (expense.tx.category?.name || expense.tx.fyle_category || expense.tx.sub_category)?.toLowerCase();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (expense.tx.org_category) { | |
| category = expense.tx.org_category.toLowerCase(); | |
| if (expense.tx.category?.name) { | |
| category = expense.tx.category.name.toLowerCase(); | |
| } | |
| const category = | |
| (expense.tx.category?.name || expense.tx.fyle_category || expense.tx.sub_category)?.toLowerCase(); |
🤖 Prompt for AI Agents
In src/app/fyle/add-edit-per-diem/add-edit-per-diem.page.ts around lines 461 to
464, the current code only sets category from expense.tx.category.name which can
be missing and cause misrouting; update the logic to fall back to
expense.tx.fyle_category or expense.tx.fyle_sub_category (in that order) when
category or its name is absent, normalize the chosen value to lowercase, and use
that fallback value for routing so the review flow doesn’t silently default to
generic expense.
| Amount: etxn.tx.amount, | ||
| Currency: etxn.tx.currency, | ||
| Category: etxn.tx.org_category, | ||
| Category: etxn.tx.category?.name, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Telemetry shouldn’t go mute. Add a Category fallback.
On fresh add/edit, category object can be absent; log a sensible default.
- Category: etxn.tx.category?.name,
+ Category: etxn.tx.category?.name || etxn.tx.fyle_category || 'Per Diem',Also applies to: 2064-2064
🤖 Prompt for AI Agents
In src/app/fyle/add-edit-per-diem/add-edit-per-diem.page.ts around lines 1894
and 2064, the telemetry log uses etxn.tx.category?.name which can be undefined
on fresh add/edit; replace it with a safe fallback (e.g. use the category name
if present otherwise a sensible default like "Uncategorized" or "Unknown
Category") so the logged Category is never empty.
| let category; | ||
|
|
||
| if (initial.tx.org_category) { | ||
| category = initial.tx.org_category.toLowerCase(); | ||
| if (initial.tx.category?.name) { | ||
| category = initial.tx.category.name.toLowerCase(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Route robustly even if category.name is missing
Make the route logic rock-solid by falling back to fyle_category. One liner, maximum impact.
Apply this diff:
- let category;
-
- if (initial.tx.category?.name) {
- category = initial.tx.category.name.toLowerCase();
- }
+ const category = (initial.tx.category?.name ?? initial.tx.fyle_category)?.toLowerCase();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let category; | |
| if (initial.tx.org_category) { | |
| category = initial.tx.org_category.toLowerCase(); | |
| if (initial.tx.category?.name) { | |
| category = initial.tx.category.name.toLowerCase(); | |
| } | |
| const category = (initial.tx.category?.name ?? initial.tx.fyle_category)?.toLowerCase(); |
🤖 Prompt for AI Agents
In src/app/fyle/dashboard/tasks/tasks.component.ts around lines 555 to 559, the
code assumes initial.tx.category.name exists; change the logic to set category
using a single-line fallback that uses initial.tx.category?.name or
initial.tx.fyle_category (e.g., assign category = (initial.tx.category?.name ||
initial.tx.fyle_category).toLowerCase() while guarding against undefined before
calling toLowerCase) so routing works even when category.name is missing.
2 participants
Clickup
Please add link here
Code Coverage
Please add code coverage here
UI Preview
Please add screenshots for UI changes
Summary by CodeRabbit