Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions src/libs/ModifiedExpenseMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -525,8 +525,8 @@ function getForReportAction({

const hasModifiedAttendees = isReportActionOriginalMessageAnObject && 'oldAttendees' in reportActionOriginalMessage && 'newAttendees' in reportActionOriginalMessage;
if (hasModifiedAttendees) {
const [oldAttendees, attendees] = getFormattedAttendees(reportActionOriginalMessage.newAttendees, reportActionOriginalMessage.oldAttendees);
buildMessageFragmentForValue(translate, oldAttendees, attendees, translate('iou.attendees'), false, setFragments, removalFragments, changeFragments);
const [oldAttendees, newAttendees] = getFormattedAttendees(reportActionOriginalMessage.oldAttendees, reportActionOriginalMessage.newAttendees);
buildMessageFragmentForValue(translate, newAttendees, oldAttendees, translate('iou.attendees'), false, setFragments, removalFragments, changeFragments);
Comment thread
lorretheboy marked this conversation as resolved.
}

const hasPersonalRulesModifiedFields = isReportActionOriginalMessageAnObject && 'personalRulesModifiedFields' in reportActionOriginalMessage;
Expand Down
9 changes: 7 additions & 2 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ import {openUnreportedExpense} from './actions/Report';
import {isAnonymousUser as isAnonymousUserSession} from './actions/Session';
import {removeDraftTransactionsByIDs} from './actions/TransactionEdit';
import {getOnboardingMessages} from './actions/Welcome/OnboardingFlow';
import {convertAttendeesToArray} from './AttendeeUtils';
import {convertAttendeesToArray, normalizeAttendees} from './AttendeeUtils';
import {getCategoryGLCode} from './CategoryUtils';
import {convertToDisplayStringEnLocale} from './CurrencyUtils';
import DateUtils from './DateUtils';
Expand Down Expand Up @@ -6271,7 +6271,12 @@ function getModifiedExpenseOriginalMessage(
originalMessage.merchant = transactionChanges?.merchant;
}
if ('attendees' in transactionChanges) {
originalMessage.oldAttendees = getAttendees(oldTransaction);
// Only `modifiedAttendees` counts as a previous edit, so we deliberately ignore `comment.attendees` (the default
// attendee the app adds when the expense is created). Otherwise the optimistic message would read
// "changed the attendees from <self> to X" while the server returns "set the attendees to X", causing a flicker.
// Note: this must stay an empty array rather than being omitted, because ModifiedExpenseMessage checks
// `'oldAttendees' in originalMessage` to decide whether to build the attendees fragment at all.
originalMessage.oldAttendees = normalizeAttendees(convertAttendeesToArray(oldTransaction?.modifiedAttendees));
originalMessage.newAttendees = transactionChanges?.attendees;
}

Expand Down
28 changes: 28 additions & 0 deletions tests/actions/ReportTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import type * as SearchQueryUtilsType from '@src/libs/SearchQueryUtils';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type * as OnyxTypes from '@src/types/onyx';
import type {Attendee} from '@src/types/onyx/IOU';

import type {OnyxCollection, OnyxEntry, OnyxUpdate} from 'react-native-onyx';

Expand Down Expand Up @@ -9642,6 +9643,33 @@ describe('actions/Report', () => {
});
});

describe('buildOptimisticModifiedExpenseReportAction attendees', () => {
const TRANSACTION_ID = '888';
const ownerAttendee: Attendee = {email: 'owner@example.com', displayName: 'Owner', avatarUrl: ''};
const otherAttendee: Attendee = {email: 'other@example.com', displayName: 'Other', avatarUrl: ''};

const getAttendeesOriginalMessage = (oldTransaction: OnyxTypes.Transaction, newAttendees: Attendee[]) => {
const result = ReportUtils.buildOptimisticModifiedExpenseReportAction(undefined, oldTransaction, {attendees: newAttendees}, false, undefined, undefined);
return getOriginalMessage(result as OnyxTypes.ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.MODIFIED_EXPENSE>);
};

it('treats oldAttendees as empty (set message) on the first edit, when attendees have never been modified', () => {
// Only the auto-added default attendee is present (comment.attendees), with no modifiedAttendees history.
const oldTransaction = createMock<OnyxTypes.Transaction>({transactionID: TRANSACTION_ID, comment: {attendees: [ownerAttendee]}});
const originalMessage = getAttendeesOriginalMessage(oldTransaction, [otherAttendee]);
expect(originalMessage?.oldAttendees).toEqual([]);
expect(originalMessage?.newAttendees).toEqual([otherAttendee]);
});

it('keeps oldAttendees (changed message) on subsequent edits, even when the previous value is just the owner', () => {
// After previous edits, modifiedAttendees holds the last value - here it happens to be only the owner.
const oldTransaction = createMock<OnyxTypes.Transaction>({transactionID: TRANSACTION_ID, modifiedAttendees: [ownerAttendee], comment: {attendees: [ownerAttendee]}});
const originalMessage = getAttendeesOriginalMessage(oldTransaction, [ownerAttendee, otherAttendee]);
expect(originalMessage?.oldAttendees).toEqual([ownerAttendee]);
expect(originalMessage?.newAttendees).toEqual([ownerAttendee, otherAttendee]);
});
});

describe('buildOptimisticModifiedExpenseReportAction distance currency', () => {
it('keeps the expense currency when a route switch leaves modifiedCurrency unset', () => {
const oldTransaction = createMock<OnyxTypes.Transaction>({
Expand Down
71 changes: 71 additions & 0 deletions tests/unit/ModifiedExpenseMessageTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2278,6 +2278,77 @@ describe('ModifiedExpenseMessage', () => {
});
});

describe('when attendees are changed', () => {
it('returns the correct message with old and new attendees in the right order', () => {
const reportAction = {
...createRandomReportAction(1),
actionName: CONST.REPORT.ACTIONS.TYPE.MODIFIED_EXPENSE,
originalMessage: {
oldAttendees: [{email: 'alice@example.com', displayName: 'Alice', avatarUrl: ''}],
newAttendees: [
{email: 'alice@example.com', displayName: 'Alice', avatarUrl: ''},
{email: 'bob@example.com', displayName: 'Bob', avatarUrl: ''},
],
},
};

const result = getForReportAction({
convertToDisplayString,
translate: translateLocal,
reportAction,
policy: undefined,
policyTags: undefined,
currentUserLogin: 'test@example.com',
});

expect(result).toEqual('changed the attendees to Alice, Bob (previously Alice)');
});

it('returns "set" message when attendees are added from empty', () => {
const reportAction = {
...createRandomReportAction(1),
actionName: CONST.REPORT.ACTIONS.TYPE.MODIFIED_EXPENSE,
originalMessage: {
oldAttendees: [],
newAttendees: [{email: 'alice@example.com', displayName: 'Alice', avatarUrl: ''}],
},
};

const result = getForReportAction({
convertToDisplayString,
translate: translateLocal,
reportAction,
policy: undefined,
policyTags: undefined,
currentUserLogin: 'test@example.com',
});

expect(result).toEqual('set the attendees to Alice');
});

it('returns "removed" message when attendees are cleared', () => {
const reportAction = {
...createRandomReportAction(1),
actionName: CONST.REPORT.ACTIONS.TYPE.MODIFIED_EXPENSE,
originalMessage: {
oldAttendees: [{email: 'alice@example.com', displayName: 'Alice', avatarUrl: ''}],
newAttendees: [],
},
};

const result = getForReportAction({
convertToDisplayString,
translate: translateLocal,
reportAction,
policy: undefined,
policyTags: undefined,
currentUserLogin: 'test@example.com',
});

expect(result).toEqual('removed the attendees (previously Alice)');
});
});

describe('vendor changes', () => {
// QBO policy with two named vendors used by the resolver. No `config` block — the resolver
// must look up vendors regardless of the workspace's current export mode so historical
Expand Down
Loading