Skip to content
Merged
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
13 changes: 12 additions & 1 deletion src/libs/actions/Task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {getReportName} from '@libs/ReportNameUtils';
import * as ReportUtils from '@libs/ReportUtils';
import {buildOptimisticSnapshotData} from '@libs/SearchQueryUtils';
import playSound, {SOUNDS} from '@libs/Sound';
import type {AvatarSource} from '@libs/UserAvatarUtils';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
Expand Down Expand Up @@ -74,6 +75,8 @@ type CreateTaskAndNavigateParams = {
isCreatedUsingMarkdown?: boolean;
quickAction?: OnyxEntry<OnyxTypes.QuickAction>;
ancestors?: ReportUtils.Ancestor[];
currentUserDisplayName: string | undefined;
currentUserAvatar: AvatarSource | undefined;
};

/**
Expand Down Expand Up @@ -110,6 +113,8 @@ function createTaskAndNavigate(params: CreateTaskAndNavigateParams) {
assigneeEmail,
currentUserAccountID,
currentUserEmail,
currentUserDisplayName,
currentUserAvatar,
assigneeAccountID = 0,
assigneeChatReport,
policyID = CONST.POLICY.OWNER_EMAIL_FAKE,
Expand Down Expand Up @@ -138,7 +143,13 @@ function createTaskAndNavigate(params: CreateTaskAndNavigateParams) {
let assigneeChatReportOnyxData;

// Parent ReportAction indicating that a task has been created
const optimisticTaskCreatedAction = ReportUtils.buildOptimisticCreatedReportAction({emailCreatingAction: currentUserEmail});
const optimisticTaskCreatedAction = ReportUtils.buildOptimisticCreatedReportAction({
emailCreatingAction: currentUserEmail,
currentUserAccountID,
currentUserDisplayName,
currentUserEmail,
currentUserAvatar,
});
const optimisticAddCommentReport = ReportUtils.buildOptimisticTaskCommentReportAction(taskReportID, title, assigneeAccountID, `task for ${title}`, parentReportID);
optimisticTaskReport.parentReportActionID = optimisticAddCommentReport.reportAction.reportActionID;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ function useComposerSubmit(reportID: string): (comment: string) => void {
assigneeEmail: assignee?.login ?? '',
currentUserAccountID: currentUserPersonalDetails.accountID,
currentUserEmail,
currentUserDisplayName: currentUserPersonalDetails.displayName,
currentUserAvatar: currentUserPersonalDetails.avatar,
assigneeAccountID: assignee?.accountID,
assigneeChatReport,
policyID: report?.policyID,
Expand Down
2 changes: 2 additions & 0 deletions src/pages/tasks/NewTaskDetailsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ function NewTaskDetailsPage({route}: NewTaskDetailsPageProps) {
assigneeEmail: task?.assignee ?? '',
currentUserAccountID: currentUserPersonalDetails.accountID,
currentUserEmail: currentUserPersonalDetails.email ?? '',
currentUserDisplayName: currentUserPersonalDetails.displayName,
currentUserAvatar: currentUserPersonalDetails.avatar,
assigneeAccountID: task.assigneeAccountID,
assigneeChatReport: task.assigneeChatReport,
policyID: CONST.POLICY.OWNER_EMAIL_FAKE,
Expand Down
2 changes: 2 additions & 0 deletions src/pages/tasks/NewTaskPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ function NewTaskPage({route}: NewTaskPageProps) {
assigneeEmail: task?.assignee ?? '',
currentUserAccountID: currentUserPersonalDetails.accountID,
currentUserEmail: currentUserPersonalDetails.email ?? '',
currentUserDisplayName: currentUserPersonalDetails.displayName,
currentUserAvatar: currentUserPersonalDetails.avatar,
assigneeAccountID: task.assigneeAccountID,
assigneeChatReport: task.assigneeChatReport,
policyID: parentReport?.policyID,
Expand Down
140 changes: 140 additions & 0 deletions tests/actions/TaskTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,8 @@ describe('actions/Task', () => {
const mockPolicyID = 'policy_123';
const mockCurrentUserAccountID = 123;
const mockCurrentUserEmail = 'creator@example.com';
const mockCurrentUserDisplayName = 'Creator User';
const mockCurrentUserAvatar = 'https://example.com/avatar.png';

beforeEach(async () => {
jest.clearAllMocks();
Expand Down Expand Up @@ -412,6 +414,8 @@ describe('actions/Task', () => {
assigneeEmail: mockAssigneeEmail,
currentUserAccountID: mockCurrentUserAccountID,
currentUserEmail: mockCurrentUserEmail,
currentUserDisplayName: mockCurrentUserDisplayName,
currentUserAvatar: mockCurrentUserAvatar,
assigneeAccountID: mockAssigneeAccountID,
assigneeChatReport: mockAssigneeChatReport,
policyID: mockPolicyID,
Expand Down Expand Up @@ -454,6 +458,8 @@ describe('actions/Task', () => {
assigneeEmail: mockAssigneeEmail,
currentUserAccountID: mockCurrentUserAccountID,
currentUserEmail: mockCurrentUserEmail,
currentUserDisplayName: mockCurrentUserDisplayName,
currentUserAvatar: mockCurrentUserAvatar,
assigneeAccountID: mockAssigneeAccountID,
policyID: mockPolicyID,
isCreatedUsingMarkdown: false,
Expand Down Expand Up @@ -481,6 +487,8 @@ describe('actions/Task', () => {
assigneeEmail: mockAssigneeEmail,
currentUserAccountID: mockCurrentUserAccountID,
currentUserEmail: mockCurrentUserEmail,
currentUserDisplayName: mockCurrentUserDisplayName,
currentUserAvatar: mockCurrentUserAvatar,
assigneeAccountID: mockAssigneeAccountID,
policyID: mockPolicyID,
isCreatedUsingMarkdown: false,
Expand Down Expand Up @@ -512,6 +520,8 @@ describe('actions/Task', () => {
assigneeEmail: mockAssigneeEmail,
currentUserAccountID: mockCurrentUserAccountID,
currentUserEmail: mockCurrentUserEmail,
currentUserDisplayName: mockCurrentUserDisplayName,
currentUserAvatar: mockCurrentUserAvatar,
assigneeAccountID: mockAssigneeAccountID,
assigneeChatReport: undefined,
policyID: mockPolicyID,
Expand Down Expand Up @@ -572,6 +582,8 @@ describe('actions/Task', () => {
assigneeEmail: mockAssigneeEmail,
currentUserAccountID: mockCurrentUserAccountID,
currentUserEmail: mockCurrentUserEmail,
currentUserDisplayName: mockCurrentUserDisplayName,
currentUserAvatar: mockCurrentUserAvatar,
assigneeAccountID: mockAssigneeAccountID,
assigneeChatReport: mockAssigneeChatReport,
policyID: mockPolicyID,
Expand Down Expand Up @@ -610,6 +622,8 @@ describe('actions/Task', () => {
assigneeEmail: mockAssigneeEmail,
currentUserAccountID: mockCurrentUserAccountID,
currentUserEmail: mockCurrentUserEmail,
currentUserDisplayName: mockCurrentUserDisplayName,
currentUserAvatar: mockCurrentUserAvatar,
assigneeAccountID: mockAssigneeAccountID,
assigneeChatReport: mockAssigneeChatReport,
policyID: CONST.POLICY.OWNER_EMAIL_FAKE,
Expand Down Expand Up @@ -655,6 +669,8 @@ describe('actions/Task', () => {
assigneeEmail: mockCurrentUserEmail,
currentUserAccountID: mockCurrentUserAccountID,
currentUserEmail: mockCurrentUserEmail,
currentUserDisplayName: mockCurrentUserDisplayName,
currentUserAvatar: mockCurrentUserAvatar,
assigneeAccountID: mockCurrentUserAccountID, // assignee is current user
assigneeChatReport: mockAssigneeChatReport,
policyID: mockPolicyID,
Expand Down Expand Up @@ -710,6 +726,8 @@ describe('actions/Task', () => {
assigneeEmail: mockAssigneeEmail,
currentUserAccountID: mockCurrentUserAccountID,
currentUserEmail: mockCurrentUserEmail,
currentUserDisplayName: mockCurrentUserDisplayName,
currentUserAvatar: mockCurrentUserAvatar,
assigneeAccountID: mockAssigneeAccountID,
assigneeChatReport: mockAssigneeChatReport,
policyID: mockPolicyID,
Expand Down Expand Up @@ -745,6 +763,8 @@ describe('actions/Task', () => {
assigneeEmail: mockAssigneeEmail,
currentUserAccountID: mockCurrentUserAccountID,
currentUserEmail: mockCurrentUserEmail,
currentUserDisplayName: mockCurrentUserDisplayName,
currentUserAvatar: mockCurrentUserAvatar,
assigneeAccountID: mockAssigneeAccountID,
assigneeChatReport: mockAssigneeChatReport,
policyID: mockPolicyID,
Expand Down Expand Up @@ -773,6 +793,126 @@ describe('actions/Task', () => {
}),
);
});

it('should forward currentUserAccountID, currentUserEmail, currentUserDisplayName and currentUserAvatar to buildOptimisticCreatedReportAction when all are provided', () => {
// Given: All current user identity fields are provided as defined values
// When: createTaskAndNavigate is called
createTaskAndNavigate({
parentReport: {reportID: mockParentReportID},
title: mockTitle,
description: mockDescription,
assigneeEmail: mockAssigneeEmail,
currentUserAccountID: mockCurrentUserAccountID,
currentUserEmail: mockCurrentUserEmail,
currentUserDisplayName: mockCurrentUserDisplayName,
currentUserAvatar: mockCurrentUserAvatar,
assigneeAccountID: mockAssigneeAccountID,
policyID: mockPolicyID,
isCreatedUsingMarkdown: false,
quickAction: {},
});

// Then: buildOptimisticCreatedReportAction receives the exact identity values that were passed in
expect(mockBuildOptimisticCreatedReportAction).toHaveBeenCalledWith({
emailCreatingAction: mockCurrentUserEmail,
currentUserAccountID: mockCurrentUserAccountID,
currentUserDisplayName: mockCurrentUserDisplayName,
currentUserEmail: mockCurrentUserEmail,
currentUserAvatar: mockCurrentUserAvatar,
});
});

it('should forward undefined currentUserDisplayName and currentUserAvatar to buildOptimisticCreatedReportAction without substituting fallbacks', () => {
// Given: currentUserDisplayName and currentUserAvatar are explicitly undefined
// When: createTaskAndNavigate is called
createTaskAndNavigate({
parentReport: {reportID: mockParentReportID},
title: mockTitle,
description: mockDescription,
assigneeEmail: mockAssigneeEmail,
currentUserAccountID: mockCurrentUserAccountID,
currentUserEmail: mockCurrentUserEmail,
currentUserDisplayName: undefined,
currentUserAvatar: undefined,
assigneeAccountID: mockAssigneeAccountID,
policyID: mockPolicyID,
isCreatedUsingMarkdown: false,
quickAction: {},
});

// Then: buildOptimisticCreatedReportAction is invoked with the same undefined values
expect(mockBuildOptimisticCreatedReportAction).toHaveBeenCalledWith({
emailCreatingAction: mockCurrentUserEmail,
currentUserAccountID: mockCurrentUserAccountID,
currentUserDisplayName: undefined,
currentUserEmail: mockCurrentUserEmail,
currentUserAvatar: undefined,
});
});

it('should forward an empty currentUserEmail to buildOptimisticCreatedReportAction so emailCreatingAction is also empty', () => {
// Given: currentUserEmail is provided as an empty string (caller bypassing missing data)
// When: createTaskAndNavigate is called
createTaskAndNavigate({
parentReport: {reportID: mockParentReportID},
title: mockTitle,
description: mockDescription,
assigneeEmail: mockAssigneeEmail,
currentUserAccountID: mockCurrentUserAccountID,
currentUserEmail: '',
currentUserDisplayName: mockCurrentUserDisplayName,
currentUserAvatar: mockCurrentUserAvatar,
assigneeAccountID: mockAssigneeAccountID,
policyID: mockPolicyID,
isCreatedUsingMarkdown: false,
quickAction: {},
});

// Then: emailCreatingAction is the empty string we provided (no fallback to a session value)
expect(mockBuildOptimisticCreatedReportAction).toHaveBeenCalledWith({
emailCreatingAction: '',
currentUserAccountID: mockCurrentUserAccountID,
currentUserDisplayName: mockCurrentUserDisplayName,
currentUserEmail: '',
currentUserAvatar: mockCurrentUserAvatar,
});
});

it('should use the provided currentUserAccountID as actorAccountID and lastActorAccountID instead of reading from session', async () => {
// Given: a currentUserAccountID different from the session account that overrides what session says
const overrideUserAccountID = 999;

// When: createTaskAndNavigate is called with that override
createTaskAndNavigate({
parentReport: {reportID: mockParentReportID},
title: mockTitle,
description: mockDescription,
assigneeEmail: mockAssigneeEmail,
currentUserAccountID: overrideUserAccountID,
currentUserEmail: mockCurrentUserEmail,
currentUserDisplayName: mockCurrentUserDisplayName,
currentUserAvatar: mockCurrentUserAvatar,
assigneeAccountID: mockAssigneeAccountID,
policyID: mockPolicyID,
isCreatedUsingMarkdown: false,
quickAction: {},
});

await waitForBatchedUpdatesWithAct();

// Then: the optimistic parent report update uses the provided account ID, proving the function does not read the session
// eslint-disable-next-line rulesdir/no-multiple-api-calls
const [, , onyx] = (API.write as jest.Mock).mock.calls.at(0) as [unknown, unknown, OnyxData<typeof ONYXKEYS.COLLECTION.REPORT>];
const parentReportUpdate = onyx.optimisticData?.find(
(update) => update.key === `${ONYXKEYS.COLLECTION.REPORT}${mockParentReportID}` && (update.value as Report | undefined)?.lastActorAccountID !== undefined,
);
expect((parentReportUpdate?.value as Report | undefined)?.lastActorAccountID).toBe(overrideUserAccountID);
expect(mockBuildOptimisticCreatedReportAction).toHaveBeenCalledWith(
expect.objectContaining({
currentUserAccountID: overrideUserAccountID,
}),
);
});
});

describe('completeTask', () => {
Expand Down
Loading