Skip to content

fix: Change copy of system message that generates when a previously uncategorized expense is categorized #78457

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dangrous merged 5 commits into from
Jan 7, 2026
Merged

fix: Change copy of system message that generates when a previously uncategorized expense is categorized #78457

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9946af1
fix: Change copy of system message that generates when a previously u…
TaduJR Dec 25, 2025
60725af
fix: Use 'set' instead of 'changed' for previously uncategorized expe…
TaduJR Dec 27, 2025
f063fcc
Merge branch 'main' of https://github.com/TaduJR/App into fix-Change-…
TaduJR Jan 1, 2026
1ff2e2e
fix: Use 'set' instead of 'changed' for previously uncategorized expe…
TaduJR Jan 1, 2026
6ff4aa2
Merge main into fix branch
TaduJR Jan 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 10 additions & 13 deletions src/libs/ModifiedExpenseMessage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,24 +69,21 @@ function buildMessageFragmentForValue(
shouldConvertToLowercase = true,
) {
const newValueToDisplay = valueInQuotes ? `"${newValue}"` : newValue;
const oldValueToDisplay = valueInQuotes ? `"${oldValue}"` : oldValue;

// If the valueName is category and the old value was Uncategorized, show it in lowercase without quotes
let oldValueToDisplay;
if (valueName.includes(translate('common.category').toLowerCase()) && isCategoryMissing(oldValue)) {
oldValueToDisplay = oldValue.toLowerCase();
} else if (valueInQuotes) {
oldValueToDisplay = `"${oldValue}"`;
} else {
oldValueToDisplay = oldValue;
}
// eslint-disable-next-line @typescript-eslint/no-deprecated
Copy link
Contributor

@dangrous dangrous Jan 7, 2026

Choose a reason for hiding this comment

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

NAB - it looks like we use this a bunch of times outside of this logic, so it's not just for this. But, can we use the non-deprecated useLocalize here (and there) instead? Or is that not an option

Copy link
Contributor Author

@TaduJR TaduJR Jan 7, 2026

Choose a reason for hiding this comment

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

Good Idea!

Should I raise another PR after investigation?

Copy link
Contributor

@dangrous dangrous Jan 8, 2026

Choose a reason for hiding this comment

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

that would be great!

const isCategoryField = valueName.includes(translateLocal('common.category').toLowerCase());

const displayValueName = shouldConvertToLowercase ? valueName.toLowerCase() : valueName;
const isOldValuePartialMerchant = valueName === translate('common.merchant') && oldValue === CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT;
const isOldCategoryMissing = isCategoryField && isCategoryMissing(oldValue);
const isNewCategoryMissing = isCategoryField && isCategoryMissing(newValue);

// In case of a partial merchant value, we want to avoid user seeing the "(none)" value in the message.
if (!oldValue || isOldValuePartialMerchant) {
const fragment = translate('iou.setTheRequest', {valueName: displayValueName, newValueToDisplay});
setFragments.push(fragment);
if (!oldValue || isOldValuePartialMerchant || isOldCategoryMissing) {
if (!(isOldCategoryMissing && isNewCategoryMissing)) {
const fragment = translate('iou.setTheRequest', {valueName: displayValueName, newValueToDisplay});
setFragments.push(fragment);
}
} else if (!newValue || newValue === CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT) {
const fragment = translate('iou.removedTheRequest', {valueName: displayValueName, oldValueToDisplay});
removalFragments.push(fragment);
Expand Down
39 changes: 39 additions & 0 deletions tests/unit/ModifiedExpenseMessageTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -739,6 +739,45 @@ describe('ModifiedExpenseMessage', () => {
});
});

describe('when the category is changed from Uncategorized with AI attribution', () => {
const reportAction = {
...createRandomReportAction(1),
actionName: CONST.REPORT.ACTIONS.TYPE.MODIFIED_EXPENSE,
originalMessage: {
category: '6403 Travel - Member Services',
oldCategory: 'Uncategorized',
source: CONST.CATEGORY_SOURCE.AI,
} as OriginalMessageModifiedExpense,
};

it('returns the correct text message without showing previously uncategorized', () => {
const expectedResult = `set the category based on past activity to "6403 Travel - Member Services"`;

const result = getForReportAction({reportAction, policyID: report.policyID});

expect(result).toEqual(expectedResult);
});
});

describe('when the category is cleared from Uncategorized (both missing)', () => {
const reportAction = {
...createRandomReportAction(1),
actionName: CONST.REPORT.ACTIONS.TYPE.MODIFIED_EXPENSE,
originalMessage: {
category: '',
oldCategory: 'Uncategorized',
} as OriginalMessageModifiedExpense,
};

it('returns the generic changed expense message since no meaningful change occurred', () => {
const expectedResult = `changed the expense`;

const result = getForReportAction({reportAction, policyID: report.policyID});

expect(result).toEqual(expectedResult);
});
});

describe('when the category is removed with AI attribution', () => {
const reportAction = {
...createRandomReportAction(1),
Expand Down
Loading