Add receipt requirement columns to category spreadsheet import#88962
Add receipt requirement columns to category spreadsheet import#88962
Conversation
Support "Require receipts over" and "Require itemized receipts over" columns in the category CSV import flow for Control policies. Values are parsed using the same logic as OD: default, required/always_required, not_required, or a numeric threshold. Normalization ensures consistent states (e.g. receipts not_required forces itemized receipts to not_required). Co-authored-by: Daniel Gale-Rosen <[email protected]>
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
@codex review |
|
@suneox here's the PR if you want to take a look! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d101b5354d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (normalizedMaxAmountNoItemizedReceipt === 0 && normalizedMaxAmountNoReceipt !== undefined) { | ||
| normalizedMaxAmountNoReceipt = 0; |
There was a problem hiding this comment.
Normalize paired receipt rules when only one CSV column is mapped
The new normalization logic only runs when both parsed values are defined, so imports that map just one of the new columns can leave category rules in a state that diverges from the manual UI behavior. For example, mapping only maxAmountNoItemizedReceipt to always_required (0) will not force maxAmountNoReceipt to 0 because normalizedMaxAmountNoReceipt is undefined; similarly, mapping only maxAmountNoReceipt to not_required will not force itemized to not_required. This means the import path does not consistently apply the intended coupled-rule normalization for existing categories.
Useful? React with 👍 / 👎.
|
@MelvinBot The auto-match logic in findColumnName func at ImportColumn.tsx is missing cases for the new receipt requirement columns. When users upload a CSV with headers "Receipts Required" or "Itemised Receipt Requirement", these columns are not automatically mapped to their respective attribute Here is the list of columns from the example file: |
|
Good catch — the The fix would be adding two new case blocks (around line 143, before the case 'receiptsrequired':
case 'requirereceiptsover':
case 'maxamountnoreceipt':
attribute = CONST.CSV_IMPORT_COLUMNS.MAX_AMOUNT_NO_RECEIPT;
break;
case 'itemisedreceiptrequirement':
case 'itemizedreceiptrequirement':
case 'requireitemizedreceiptsover':
case 'maxamountnoitemizedreceipt':
attribute = CONST.CSV_IMPORT_COLUMNS.MAX_AMOUNT_NO_ITEMIZED_RECEIPT;
break;I can only make code changes when directed by the PR author, an assignee, or an existing co-author. To have me implement this, the PR assignee or author should reply with a request tagging me. |
|
@dangrous Could you please reassign this pr to me so I can work on it to resolve this issue? I’ll collaborate with Melvin Bot until it’s done and then ping you. Thanks! |
|
⏱️ Ran out of time before I could finish. This usually means the task was too complex or I got stuck exploring. Please try again with a more specific request or post in #melvinbot-ai. |
|
all set! |
|
@MelvinBot Let resolve the conflicts and update the PR according to this issue |
Resolves merge conflict in ImportedCategoriesPage.tsx (kept dataIndex variable with .trim() from main). Adds findColumnName cases so CSV headers like "Receipts Required" and "Itemised Receipt Requirement" are automatically mapped to MAX_AMOUNT_NO_RECEIPT and MAX_AMOUNT_NO_ITEMIZED_RECEIPT. Co-authored-by: suneox <[email protected]> Co-authored-by: Cong Pham <[email protected]>
|
Resolved the merge conflict and added the auto-match cases for receipt requirement CSV columns. Changes:
|
Explanation of Change
Adds support for two new optional columns in the category spreadsheet import for Control policies: Require receipts over (
maxAmountNoReceipt) and Require itemized receipts over (maxAmountNoItemizedReceipt).When admins import categories via CSV, they can now include these columns with values matching the OD import logic:
default→ reset to policy default (null)required/always_required→ always require (0)not_required→ never require (DISABLED_MAX_EXPENSE_VALUE)2500) → require over that amountThe import applies the same normalization as the manual UI:
If neither column is mapped, existing receipt settings are preserved (no change to current behavior).
Fixed Issues
$ #86106
Tests
// TODO: The human co-author must fill out the tests you ran before marking this PR as "ready for review"
// Please describe what tests you performed that validates your changed worked.
Offline tests
// TODO
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari