From c239c538ea3bd4de2e572ceb7c969e57fb0f0f49 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 24 Apr 2025 10:34:14 -0700 Subject: [PATCH 01/51] Add a skeleton test --- tests/actions/TaskTest.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/actions/TaskTest.ts b/tests/actions/TaskTest.ts index b02896ebd849..f263e4c3db0b 100644 --- a/tests/actions/TaskTest.ts +++ b/tests/actions/TaskTest.ts @@ -94,6 +94,10 @@ describe('actions/Task', () => { }); }); + describe('canActionTask', () => { + it('returns false if the report is a cancelled task report', () => {}); + }); + describe('completeTestDriveTask', () => { const accountID = 2; const conciergeChatReport: Report = LHNTestUtils.getFakeReport([accountID, CONST.ACCOUNT_ID.CONCIERGE]); From 4e54259d1c53b3283ba83114bf550d119d5663de Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 24 Apr 2025 10:42:55 -0700 Subject: [PATCH 02/51] Add first assertion --- tests/actions/TaskTest.ts | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tests/actions/TaskTest.ts b/tests/actions/TaskTest.ts index f263e4c3db0b..d782a3aee7b7 100644 --- a/tests/actions/TaskTest.ts +++ b/tests/actions/TaskTest.ts @@ -1,7 +1,7 @@ import {renderHook} from '@testing-library/react-native'; import Onyx from 'react-native-onyx'; import useReportIsArchived from '@hooks/useReportIsArchived'; -import {canModifyTask, completeTestDriveTask} from '@libs/actions/Task'; +import {canActionTask, canModifyTask, completeTestDriveTask} from '@libs/actions/Task'; // eslint-disable-next-line no-restricted-syntax -- this is required to allow mocking import * as API from '@libs/API'; import {WRITE_COMMANDS} from '@libs/API/types'; @@ -95,7 +95,19 @@ describe('actions/Task', () => { }); describe('canActionTask', () => { - it('returns false if the report is a cancelled task report', () => {}); + const managerAccountID = 3; + const employeeAccountID = 4; + const cancelledTaskReport = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); + + beforeAll(async () => { + // Store all the necessary data in Onyx + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${cancelledTaskReport.reportID}`, cancelledTaskReport); + }); + + it('returns false if the report is a cancelled task report', () => { + // The accountID doesn't matter here because the code will do an early return for the cancelled report + expect(canActionTask(cancelledTaskReport, managerAccountID)).toBe(false); + }); }); describe('completeTestDriveTask', () => { From c68f9de8a852bb64445c4e41881a56ec30f3e6fb Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 24 Apr 2025 10:50:03 -0700 Subject: [PATCH 03/51] Refactor test to be DRY and add more assertions --- tests/actions/TaskTest.ts | 105 ++++++++++++++++++-------------------- 1 file changed, 50 insertions(+), 55 deletions(-) diff --git a/tests/actions/TaskTest.ts b/tests/actions/TaskTest.ts index d782a3aee7b7..0d9724cd3a8e 100644 --- a/tests/actions/TaskTest.ts +++ b/tests/actions/TaskTest.ts @@ -23,58 +23,57 @@ jest.mock('@components/ConfirmedRoute.tsx'); OnyxUpdateManager(); describe('actions/Task', () => { + const managerAccountID = 1; + const employeeAccountID = 2; + + // Report with a non-archived parent + const report = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); + const reportParent = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); + + // Cancelled report with a non-archived parent + const taskReportCancelled = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); + const taskReportCancelledParent = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); + + // Report with an archived parent + const reportArchived = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); + const reportArchivedParent = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); + + // Set the manager as the owner of each report + report.ownerAccountID = managerAccountID; + taskReportCancelled.ownerAccountID = managerAccountID; + reportArchived.ownerAccountID = managerAccountID; + + // Set the parent report ID of each report + report.parentReportID = reportParent.reportID; + taskReportCancelled.parentReportID = taskReportCancelledParent.reportID; + reportArchived.parentReportID = reportArchivedParent.reportID; + + // This is what indicates that the report is a cancelled task report (see ReportUtils.isCanceledTaskReport()) + taskReportCancelled.isDeletedParentAction = true; + beforeAll(async () => { Onyx.init({ keys: ONYXKEYS, }); initOnyxDerivedValues(); + + // Store all the necessary data in Onyx + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, report); + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${reportParent.reportID}`, reportParent); + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${taskReportCancelled.reportID}`, taskReportCancelled); + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${taskReportCancelledParent.reportID}`, taskReportCancelledParent); + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${reportArchived.reportID}`, reportArchived); + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${reportArchivedParent.reportID}`, reportArchivedParent); + + // This is what indicates that a report is archived (see ReportUtils.isArchivedReport()) + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${reportArchivedParent.reportID}`, { + private_isArchived: new Date().toString(), + }); + await waitForBatchedUpdates(); }); describe('canModifyTask', () => { - const managerAccountID = 1; - const employeeAccountID = 2; - - // Report with a non-archived parent - const report = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); - const reportParent = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); - - // Cancelled report with a non-archived parent - const cancelledTaskReport = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); - const cancelledTaskReportParent = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); - - // Report with an archived parent - const reportArchived = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); - const reportArchivedParent = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); - - // Set the manager as the owner of each report - report.ownerAccountID = managerAccountID; - cancelledTaskReport.ownerAccountID = managerAccountID; - reportArchived.ownerAccountID = managerAccountID; - - // Set the parent report ID of each report - report.parentReportID = reportParent.reportID; - cancelledTaskReport.parentReportID = cancelledTaskReportParent.reportID; - reportArchived.parentReportID = reportArchivedParent.reportID; - - // This is what indicates that the report is a cancelled task report (see ReportUtils.isCanceledTaskReport()) - cancelledTaskReport.isDeletedParentAction = true; - - beforeAll(async () => { - // Store all the necessary data in Onyx - await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, report); - await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${reportParent.reportID}`, reportParent); - await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${cancelledTaskReport.reportID}`, cancelledTaskReport); - await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${cancelledTaskReportParent.reportID}`, cancelledTaskReportParent); - await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${reportArchived.reportID}`, reportArchived); - await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${reportArchivedParent.reportID}`, reportArchivedParent); - - // This is what indicates that a report is archived (see ReportUtils.isArchivedReport()) - await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${reportArchivedParent.reportID}`, { - private_isArchived: new Date().toString(), - }); - }); - it('returns false if the user modifying the task is not the author', () => { // Simulate how components call canModifyTask() by using the hook useReportIsArchived() to see if the report is archived const {result: isReportArchived} = renderHook(() => useReportIsArchived(report?.parentReportID)); @@ -85,8 +84,8 @@ describe('actions/Task', () => { expect(canModifyTask(reportArchived, managerAccountID, isReportArchived.current)).toBe(false); }); it('returns false if the report is a cancelled task report', () => { - const {result: isReportArchived} = renderHook(() => useReportIsArchived(cancelledTaskReport?.parentReportID)); - expect(canModifyTask(cancelledTaskReport, managerAccountID, isReportArchived.current)).toBe(false); + const {result: isReportArchived} = renderHook(() => useReportIsArchived(taskReportCancelled?.parentReportID)); + expect(canModifyTask(taskReportCancelled, managerAccountID, isReportArchived.current)).toBe(false); }); it('returns true if the user modifying the task is the author and the parent report is not archived or cancelled', () => { const {result: isReportArchived} = renderHook(() => useReportIsArchived(report?.parentReportID)); @@ -95,18 +94,14 @@ describe('actions/Task', () => { }); describe('canActionTask', () => { - const managerAccountID = 3; - const employeeAccountID = 4; - const cancelledTaskReport = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); - - beforeAll(async () => { - // Store all the necessary data in Onyx - await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${cancelledTaskReport.reportID}`, cancelledTaskReport); - }); - it('returns false if the report is a cancelled task report', () => { // The accountID doesn't matter here because the code will do an early return for the cancelled report - expect(canActionTask(cancelledTaskReport, managerAccountID)).toBe(false); + expect(canActionTask(taskReportCancelled, 0)).toBe(false); + }); + + it('returns false if the report has an archived parent report', () => { + // The accountID doesn't matter here because the code will do an early return for the archived report + expect(canActionTask(reportArchived, 0)).toBe(false); }); }); From 7fb50621b746ae64633a9b71faf432fa001cb31f Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 24 Apr 2025 10:51:43 -0700 Subject: [PATCH 04/51] More test organizing --- tests/actions/TaskTest.ts | 148 +++++++++++++++++++------------------- 1 file changed, 75 insertions(+), 73 deletions(-) diff --git a/tests/actions/TaskTest.ts b/tests/actions/TaskTest.ts index 0d9724cd3a8e..be82a440e963 100644 --- a/tests/actions/TaskTest.ts +++ b/tests/actions/TaskTest.ts @@ -23,85 +23,87 @@ jest.mock('@components/ConfirmedRoute.tsx'); OnyxUpdateManager(); describe('actions/Task', () => { - const managerAccountID = 1; - const employeeAccountID = 2; - - // Report with a non-archived parent - const report = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); - const reportParent = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); - - // Cancelled report with a non-archived parent - const taskReportCancelled = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); - const taskReportCancelledParent = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); - - // Report with an archived parent - const reportArchived = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); - const reportArchivedParent = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); - - // Set the manager as the owner of each report - report.ownerAccountID = managerAccountID; - taskReportCancelled.ownerAccountID = managerAccountID; - reportArchived.ownerAccountID = managerAccountID; - - // Set the parent report ID of each report - report.parentReportID = reportParent.reportID; - taskReportCancelled.parentReportID = taskReportCancelledParent.reportID; - reportArchived.parentReportID = reportArchivedParent.reportID; - - // This is what indicates that the report is a cancelled task report (see ReportUtils.isCanceledTaskReport()) - taskReportCancelled.isDeletedParentAction = true; + describe('canModify and canAction tasl', () => { + const managerAccountID = 1; + const employeeAccountID = 2; + + // Report with a non-archived parent + const report = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); + const reportParent = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); + + // Cancelled report with a non-archived parent + const taskReportCancelled = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); + const taskReportCancelledParent = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); + + // Report with an archived parent + const reportArchived = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); + const reportArchivedParent = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); + + // Set the manager as the owner of each report + report.ownerAccountID = managerAccountID; + taskReportCancelled.ownerAccountID = managerAccountID; + reportArchived.ownerAccountID = managerAccountID; + + // Set the parent report ID of each report + report.parentReportID = reportParent.reportID; + taskReportCancelled.parentReportID = taskReportCancelledParent.reportID; + reportArchived.parentReportID = reportArchivedParent.reportID; + + // This is what indicates that the report is a cancelled task report (see ReportUtils.isCanceledTaskReport()) + taskReportCancelled.isDeletedParentAction = true; + + beforeAll(async () => { + Onyx.init({ + keys: ONYXKEYS, + }); + initOnyxDerivedValues(); + + // Store all the necessary data in Onyx + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, report); + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${reportParent.reportID}`, reportParent); + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${taskReportCancelled.reportID}`, taskReportCancelled); + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${taskReportCancelledParent.reportID}`, taskReportCancelledParent); + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${reportArchived.reportID}`, reportArchived); + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${reportArchivedParent.reportID}`, reportArchivedParent); + + // This is what indicates that a report is archived (see ReportUtils.isArchivedReport()) + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${reportArchivedParent.reportID}`, { + private_isArchived: new Date().toString(), + }); - beforeAll(async () => { - Onyx.init({ - keys: ONYXKEYS, - }); - initOnyxDerivedValues(); - - // Store all the necessary data in Onyx - await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, report); - await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${reportParent.reportID}`, reportParent); - await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${taskReportCancelled.reportID}`, taskReportCancelled); - await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${taskReportCancelledParent.reportID}`, taskReportCancelledParent); - await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${reportArchived.reportID}`, reportArchived); - await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${reportArchivedParent.reportID}`, reportArchivedParent); - - // This is what indicates that a report is archived (see ReportUtils.isArchivedReport()) - await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${reportArchivedParent.reportID}`, { - private_isArchived: new Date().toString(), + await waitForBatchedUpdates(); }); - await waitForBatchedUpdates(); - }); - - describe('canModifyTask', () => { - it('returns false if the user modifying the task is not the author', () => { - // Simulate how components call canModifyTask() by using the hook useReportIsArchived() to see if the report is archived - const {result: isReportArchived} = renderHook(() => useReportIsArchived(report?.parentReportID)); - expect(canModifyTask(report, employeeAccountID, isReportArchived.current)).toBe(false); - }); - it('returns false if the parent report is archived', () => { - const {result: isReportArchived} = renderHook(() => useReportIsArchived(reportArchived?.parentReportID)); - expect(canModifyTask(reportArchived, managerAccountID, isReportArchived.current)).toBe(false); - }); - it('returns false if the report is a cancelled task report', () => { - const {result: isReportArchived} = renderHook(() => useReportIsArchived(taskReportCancelled?.parentReportID)); - expect(canModifyTask(taskReportCancelled, managerAccountID, isReportArchived.current)).toBe(false); - }); - it('returns true if the user modifying the task is the author and the parent report is not archived or cancelled', () => { - const {result: isReportArchived} = renderHook(() => useReportIsArchived(report?.parentReportID)); - expect(canModifyTask(report, managerAccountID, isReportArchived.current)).toBe(true); + describe('canModifyTask', () => { + it('returns false if the user modifying the task is not the author', () => { + // Simulate how components call canModifyTask() by using the hook useReportIsArchived() to see if the report is archived + const {result: isReportArchived} = renderHook(() => useReportIsArchived(report?.parentReportID)); + expect(canModifyTask(report, employeeAccountID, isReportArchived.current)).toBe(false); + }); + it('returns false if the parent report is archived', () => { + const {result: isReportArchived} = renderHook(() => useReportIsArchived(reportArchived?.parentReportID)); + expect(canModifyTask(reportArchived, managerAccountID, isReportArchived.current)).toBe(false); + }); + it('returns false if the report is a cancelled task report', () => { + const {result: isReportArchived} = renderHook(() => useReportIsArchived(taskReportCancelled?.parentReportID)); + expect(canModifyTask(taskReportCancelled, managerAccountID, isReportArchived.current)).toBe(false); + }); + it('returns true if the user modifying the task is the author and the parent report is not archived or cancelled', () => { + const {result: isReportArchived} = renderHook(() => useReportIsArchived(report?.parentReportID)); + expect(canModifyTask(report, managerAccountID, isReportArchived.current)).toBe(true); + }); }); - }); - describe('canActionTask', () => { - it('returns false if the report is a cancelled task report', () => { - // The accountID doesn't matter here because the code will do an early return for the cancelled report - expect(canActionTask(taskReportCancelled, 0)).toBe(false); - }); + describe('canActionTask', () => { + it('returns false if the report is a cancelled task report', () => { + // The accountID doesn't matter here because the code will do an early return for the cancelled report + expect(canActionTask(taskReportCancelled, 0)).toBe(false); + }); - it('returns false if the report has an archived parent report', () => { - // The accountID doesn't matter here because the code will do an early return for the archived report - expect(canActionTask(reportArchived, 0)).toBe(false); + it('returns false if the report has an archived parent report', () => { + // The accountID doesn't matter here because the code will do an early return for the archived report + expect(canActionTask(reportArchived, 0)).toBe(false); + }); }); }); From 3343b788873683fd3a6c8d9bb8885a35cc687862 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 24 Apr 2025 11:01:35 -0700 Subject: [PATCH 05/51] Rename variables and use task reports --- tests/actions/TaskTest.ts | 54 ++++++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/tests/actions/TaskTest.ts b/tests/actions/TaskTest.ts index be82a440e963..3fda82ad5b86 100644 --- a/tests/actions/TaskTest.ts +++ b/tests/actions/TaskTest.ts @@ -28,26 +28,26 @@ describe('actions/Task', () => { const employeeAccountID = 2; // Report with a non-archived parent - const report = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); - const reportParent = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); + const taskReport = {...LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]), type: CONST.REPORT.TYPE.TASK}; + const taskReportParent = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); // Cancelled report with a non-archived parent - const taskReportCancelled = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); + const taskReportCancelled = {...LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]), type: CONST.REPORT.TYPE.TASK}; const taskReportCancelledParent = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); // Report with an archived parent - const reportArchived = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); - const reportArchivedParent = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); + const taskReportArchived = {...LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]), type: CONST.REPORT.TYPE.TASK}; + const taskReportArchivedParent = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); // Set the manager as the owner of each report - report.ownerAccountID = managerAccountID; + taskReport.ownerAccountID = managerAccountID; taskReportCancelled.ownerAccountID = managerAccountID; - reportArchived.ownerAccountID = managerAccountID; + taskReportArchived.ownerAccountID = managerAccountID; // Set the parent report ID of each report - report.parentReportID = reportParent.reportID; + taskReport.parentReportID = taskReportParent.reportID; taskReportCancelled.parentReportID = taskReportCancelledParent.reportID; - reportArchived.parentReportID = reportArchivedParent.reportID; + taskReportArchived.parentReportID = taskReportArchivedParent.reportID; // This is what indicates that the report is a cancelled task report (see ReportUtils.isCanceledTaskReport()) taskReportCancelled.isDeletedParentAction = true; @@ -59,15 +59,15 @@ describe('actions/Task', () => { initOnyxDerivedValues(); // Store all the necessary data in Onyx - await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, report); - await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${reportParent.reportID}`, reportParent); + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${taskReport.reportID}`, taskReport); + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${taskReportParent.reportID}`, taskReportParent); await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${taskReportCancelled.reportID}`, taskReportCancelled); await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${taskReportCancelledParent.reportID}`, taskReportCancelledParent); - await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${reportArchived.reportID}`, reportArchived); - await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${reportArchivedParent.reportID}`, reportArchivedParent); + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${taskReportArchived.reportID}`, taskReportArchived); + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${taskReportArchivedParent.reportID}`, taskReportArchivedParent); // This is what indicates that a report is archived (see ReportUtils.isArchivedReport()) - await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${reportArchivedParent.reportID}`, { + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${taskReportArchivedParent.reportID}`, { private_isArchived: new Date().toString(), }); @@ -77,20 +77,20 @@ describe('actions/Task', () => { describe('canModifyTask', () => { it('returns false if the user modifying the task is not the author', () => { // Simulate how components call canModifyTask() by using the hook useReportIsArchived() to see if the report is archived - const {result: isReportArchived} = renderHook(() => useReportIsArchived(report?.parentReportID)); - expect(canModifyTask(report, employeeAccountID, isReportArchived.current)).toBe(false); + const {result: isReportArchived} = renderHook(() => useReportIsArchived(taskReport?.parentReportID)); + expect(canModifyTask(taskReport, employeeAccountID, isReportArchived.current)).toBe(false); }); it('returns false if the parent report is archived', () => { - const {result: isReportArchived} = renderHook(() => useReportIsArchived(reportArchived?.parentReportID)); - expect(canModifyTask(reportArchived, managerAccountID, isReportArchived.current)).toBe(false); + const {result: isReportArchived} = renderHook(() => useReportIsArchived(taskReportArchived?.parentReportID)); + expect(canModifyTask(taskReportArchived, managerAccountID, isReportArchived.current)).toBe(false); }); it('returns false if the report is a cancelled task report', () => { const {result: isReportArchived} = renderHook(() => useReportIsArchived(taskReportCancelled?.parentReportID)); expect(canModifyTask(taskReportCancelled, managerAccountID, isReportArchived.current)).toBe(false); }); it('returns true if the user modifying the task is the author and the parent report is not archived or cancelled', () => { - const {result: isReportArchived} = renderHook(() => useReportIsArchived(report?.parentReportID)); - expect(canModifyTask(report, managerAccountID, isReportArchived.current)).toBe(true); + const {result: isReportArchived} = renderHook(() => useReportIsArchived(taskReport?.parentReportID)); + expect(canModifyTask(taskReport, managerAccountID, isReportArchived.current)).toBe(true); }); }); @@ -102,7 +102,19 @@ describe('actions/Task', () => { it('returns false if the report has an archived parent report', () => { // The accountID doesn't matter here because the code will do an early return for the archived report - expect(canActionTask(reportArchived, 0)).toBe(false); + expect(canActionTask(taskReportArchived, 0)).toBe(false); + }); + + it('returns false if the user modifying the task is not the author', () => { + expect(canActionTask(taskReport, employeeAccountID)).toBe(false); + }); + + it('returns true if the user modifying the task is the author and the parent report is not archived or cancelled', () => { + expect(canActionTask(taskReport, managerAccountID)).toBe(true); + }); + + it('returns true if the user modifying the task is assigned to the task author and the parent report is archived or cancelled', () => { + expect(canActionTask(taskReportCancelled, managerAccountID)).toBe(true); }); }); }); From e5034c3ae6c258b923af1aab9bf2f9f4f0c4e627 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 24 Apr 2025 11:40:57 -0700 Subject: [PATCH 06/51] Finish writing tests to cover current logic --- src/libs/actions/Task.ts | 1 + tests/actions/TaskTest.ts | 73 +++++++++++++++++++++++++++++++++++---- 2 files changed, 68 insertions(+), 6 deletions(-) diff --git a/src/libs/actions/Task.ts b/src/libs/actions/Task.ts index 2d5d9c6641fc..6b29d4123446 100644 --- a/src/libs/actions/Task.ts +++ b/src/libs/actions/Task.ts @@ -988,6 +988,7 @@ function getParentReportAction(report: OnyxEntry): OnyxEntry { describe('canModify and canAction tasl', () => { const managerAccountID = 1; const employeeAccountID = 2; + const taskAssigneeAccountID = 3; - // Report with a non-archived parent + // TaskReport with a non-archived parent const taskReport = {...LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]), type: CONST.REPORT.TYPE.TASK}; const taskReportParent = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); - // Cancelled report with a non-archived parent + // Cancelled Task report with a non-archived parent const taskReportCancelled = {...LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]), type: CONST.REPORT.TYPE.TASK}; const taskReportCancelledParent = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); - // Report with an archived parent + // Task report with an archived parent const taskReportArchived = {...LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]), type: CONST.REPORT.TYPE.TASK}; const taskReportArchivedParent = LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]); + // This report has no parent + const taskReportWithNoParent = {...LHNTestUtils.getFakeReport([managerAccountID, employeeAccountID]), type: CONST.REPORT.TYPE.TASK}; + // Set the manager as the owner of each report taskReport.ownerAccountID = managerAccountID; taskReportCancelled.ownerAccountID = managerAccountID; taskReportArchived.ownerAccountID = managerAccountID; + taskReportWithNoParent.ownerAccountID = managerAccountID; // Set the parent report ID of each report taskReport.parentReportID = taskReportParent.reportID; @@ -59,6 +64,7 @@ describe('actions/Task', () => { initOnyxDerivedValues(); // Store all the necessary data in Onyx + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${taskReportWithNoParent.reportID}`, taskReportWithNoParent); await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${taskReport.reportID}`, taskReport); await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${taskReportParent.reportID}`, taskReportParent); await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${taskReportCancelled.reportID}`, taskReportCancelled); @@ -109,12 +115,67 @@ describe('actions/Task', () => { expect(canActionTask(taskReport, employeeAccountID)).toBe(false); }); - it('returns true if the user modifying the task is the author and the parent report is not archived or cancelled', () => { + it('returns true if the user modifying the task is the author', () => { expect(canActionTask(taskReport, managerAccountID)).toBe(true); }); - it('returns true if the user modifying the task is assigned to the task author and the parent report is archived or cancelled', () => { - expect(canActionTask(taskReportCancelled, managerAccountID)).toBe(true); + // The third parameter of canActionTask is the accountID of the user who is assigned to the task + // and is used when the task report has no parent + describe('testing the third parameter', () => { + it('returns true if the report has no parent and the third parameter is the same as the second', () => { + expect(canActionTask(taskReportWithNoParent, managerAccountID, managerAccountID)).toBe(true); + }); + + it('returns false if the report has no parent, the logged on user is not the author or assignee', () => { + expect(canActionTask(taskReportWithNoParent, employeeAccountID, 0)).toBe(false); + }); + + it('returns false if the report has no parent, the logged on user is not the author or assignee', () => { + expect(canActionTask(taskReportWithNoParent, employeeAccountID, managerAccountID)).toBe(false); + }); + }); + + // When the third parameter is not passed, the function uses childManagerAccountID of the parent report action + // to indicate the user assigned to the task + describe('testing without the third parameter and report action', () => { + beforeAll(async () => { + taskReport.parentReportActionID = 'a1'; + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${taskReport.reportID}`, taskReport); + + // Given that the task is assigned to a user who is not the author of the task + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${taskReport.parentReportID}`, { + a1: { + ...LHNTestUtils.getFakeReportAction(), + reportID: taskReport.parentReportID, + childManagerAccountID: taskAssigneeAccountID, + }, + }); + }); + + it('returns false if the logged in user is not the author or the one assigned to the task', () => { + expect(canActionTask(taskReport, employeeAccountID)).toBe(false); + }); + + it('returns true if the logged in user is the one assigned to the task', () => { + expect(canActionTask(taskReport, taskAssigneeAccountID)).toBe(true); + }); + }); + + // When the third parameter is not passed, the function uses report.managerID + // to indicate the user assigned to the task + describe('testing without the third parameter and report.managerId', () => { + beforeAll(async () => { + taskReport.managerID = taskAssigneeAccountID; + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${taskReport.reportID}`, taskReport); + }); + + it('returns false if the logged in user is not the author or the one assigned to the task', () => { + expect(canActionTask(taskReport, employeeAccountID)).toBe(false); + }); + + it('returns true if the logged in user is the one assigned to the task', () => { + expect(canActionTask(taskReport, taskAssigneeAccountID)).toBe(true); + }); }); }); }); From 9faaf6e49fae9c8cfc2392ab185dc187bf6acdba Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 24 Apr 2025 11:48:21 -0700 Subject: [PATCH 07/51] fix typo --- tests/actions/TaskTest.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/actions/TaskTest.ts b/tests/actions/TaskTest.ts index d6ee7e2993ed..b8abe4f23366 100644 --- a/tests/actions/TaskTest.ts +++ b/tests/actions/TaskTest.ts @@ -23,7 +23,7 @@ jest.mock('@components/ConfirmedRoute.tsx'); OnyxUpdateManager(); describe('actions/Task', () => { - describe('canModify and canAction tasl', () => { + describe('canModify and canAction task', () => { const managerAccountID = 1; const employeeAccountID = 2; const taskAssigneeAccountID = 3; From df02805d8a153a41d73f5464bcea9a8679380e73 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 24 Apr 2025 12:13:17 -0700 Subject: [PATCH 08/51] Refactor parameters for parent report and is archived --- src/libs/actions/Task.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/libs/actions/Task.ts b/src/libs/actions/Task.ts index 6b29d4123446..bfd394f35365 100644 --- a/src/libs/actions/Task.ts +++ b/src/libs/actions/Task.ts @@ -1227,17 +1227,19 @@ function canModifyTask(taskReport: OnyxEntry, sessionAccountID /** * Check if you can change the status of the task (mark complete or incomplete). Only the task owner and task assignee can do this. */ -function canActionTask(taskReport: OnyxEntry, sessionAccountID: number, taskOwnerAccountID?: number, taskAssigneeAccountID?: number): boolean { +function canActionTask( + taskReport: OnyxEntry, + sessionAccountID: number, + taskOwnerAccountID?: number, + taskAssigneeAccountID?: number, + parentReport?: OnyxEntry, + isParentReportArchived = false, +): boolean { if (ReportUtils.isCanceledTaskReport(taskReport)) { return false; } - const parentReport = getParentReport(taskReport); - - // This will get removed as part of https://github.com/Expensify/App/issues/59961 - // eslint-disable-next-line deprecation/deprecation - const reportNameValuePairs = ReportUtils.getReportNameValuePairs(parentReport?.reportID); - if (ReportUtils.isArchivedNonExpenseReport(parentReport, reportNameValuePairs)) { + if (!(ReportUtils.isExpenseReport(parentReport) || ReportUtils.isExpenseRequest(parentReport)) && isParentReportArchived) { return false; } From 16fc9c6063af7859862c2851c65a715301c7920f Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 24 Apr 2025 13:26:53 -0700 Subject: [PATCH 09/51] Add new hook --- src/hooks/useParentReport.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 src/hooks/useParentReport.ts diff --git a/src/hooks/useParentReport.ts b/src/hooks/useParentReport.ts new file mode 100644 index 000000000000..e6dcdb25487b --- /dev/null +++ b/src/hooks/useParentReport.ts @@ -0,0 +1,13 @@ +import type {OnyxEntry} from 'react-native-onyx'; +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; +import type {Report} from '@src/types/onyx'; +import useOnyx from './useOnyx'; + +function useParentReport(reportID?: string): OnyxEntry { + const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {canBeMissing: true}); + const [parentReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${report?.parentReportID ?? CONST.DEFAULT_NUMBER_ID}`, {canBeMissing: true}); + return parentReport; +} + +export default useParentReport; From 54bd38d9ab7b7fe9615ea04d1af5a531149b620e Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 24 Apr 2025 13:44:45 -0700 Subject: [PATCH 10/51] Implement hooks in the tests --- tests/actions/TaskTest.ts | 39 ++++++++++++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/tests/actions/TaskTest.ts b/tests/actions/TaskTest.ts index b8abe4f23366..ebd09a1fdf5f 100644 --- a/tests/actions/TaskTest.ts +++ b/tests/actions/TaskTest.ts @@ -1,5 +1,6 @@ import {renderHook} from '@testing-library/react-native'; import Onyx from 'react-native-onyx'; +import useParentReport from '@hooks/useParentReport'; import useReportIsArchived from '@hooks/useReportIsArchived'; import {canActionTask, canModifyTask, completeTestDriveTask} from '@libs/actions/Task'; // eslint-disable-next-line no-restricted-syntax -- this is required to allow mocking @@ -102,36 +103,52 @@ describe('actions/Task', () => { describe('canActionTask', () => { it('returns false if the report is a cancelled task report', () => { + const {result: parentReport} = renderHook(() => useParentReport(taskReportCancelled.reportID)); + const {result: isParentReportArchived} = renderHook(() => useReportIsArchived(parentReport.current?.reportID)); + // The accountID doesn't matter here because the code will do an early return for the cancelled report - expect(canActionTask(taskReportCancelled, 0)).toBe(false); + expect(canActionTask(taskReportCancelled, 0, undefined, undefined, parentReport.current, isParentReportArchived.current)).toBe(false); }); it('returns false if the report has an archived parent report', () => { + const {result: parentReport} = renderHook(() => useParentReport(taskReportArchived.reportID)); + const {result: isParentReportArchived} = renderHook(() => useReportIsArchived(parentReport.current?.reportID)); + // The accountID doesn't matter here because the code will do an early return for the archived report - expect(canActionTask(taskReportArchived, 0)).toBe(false); + expect(canActionTask(taskReportArchived, 0, undefined, undefined, parentReport.current, isParentReportArchived.current)).toBe(false); }); it('returns false if the user modifying the task is not the author', () => { - expect(canActionTask(taskReport, employeeAccountID)).toBe(false); + const {result: parentReport} = renderHook(() => useParentReport(taskReport.reportID)); + const {result: isParentReportArchived} = renderHook(() => useReportIsArchived(parentReport.current?.reportID)); + expect(canActionTask(taskReport, employeeAccountID, undefined, undefined, parentReport.current, isParentReportArchived.current)).toBe(false); }); it('returns true if the user modifying the task is the author', () => { - expect(canActionTask(taskReport, managerAccountID)).toBe(true); + const {result: parentReport} = renderHook(() => useParentReport(taskReport.reportID)); + const {result: isParentReportArchived} = renderHook(() => useReportIsArchived(parentReport.current?.reportID)); + expect(canActionTask(taskReport, managerAccountID, undefined, undefined, parentReport.current, isParentReportArchived.current)).toBe(true); }); // The third parameter of canActionTask is the accountID of the user who is assigned to the task // and is used when the task report has no parent describe('testing the third parameter', () => { it('returns true if the report has no parent and the third parameter is the same as the second', () => { - expect(canActionTask(taskReportWithNoParent, managerAccountID, managerAccountID)).toBe(true); + const {result: parentReport} = renderHook(() => useParentReport(taskReportWithNoParent.reportID)); + const {result: isParentReportArchived} = renderHook(() => useReportIsArchived(parentReport.current?.reportID)); + expect(canActionTask(taskReportWithNoParent, managerAccountID, undefined, undefined, parentReport.current, isParentReportArchived.current)).toBe(true); }); it('returns false if the report has no parent, the logged on user is not the author or assignee', () => { - expect(canActionTask(taskReportWithNoParent, employeeAccountID, 0)).toBe(false); + const {result: parentReport} = renderHook(() => useParentReport(taskReportWithNoParent.reportID)); + const {result: isParentReportArchived} = renderHook(() => useReportIsArchived(parentReport.current?.reportID)); + expect(canActionTask(taskReportWithNoParent, employeeAccountID, undefined, undefined, parentReport.current, isParentReportArchived.current)).toBe(false); }); it('returns false if the report has no parent, the logged on user is not the author or assignee', () => { - expect(canActionTask(taskReportWithNoParent, employeeAccountID, managerAccountID)).toBe(false); + const {result: parentReport} = renderHook(() => useParentReport(taskReportWithNoParent.reportID)); + const {result: isParentReportArchived} = renderHook(() => useReportIsArchived(parentReport.current?.reportID)); + expect(canActionTask(taskReportWithNoParent, employeeAccountID, undefined, undefined, parentReport.current, isParentReportArchived.current)).toBe(false); }); }); @@ -153,11 +170,15 @@ describe('actions/Task', () => { }); it('returns false if the logged in user is not the author or the one assigned to the task', () => { - expect(canActionTask(taskReport, employeeAccountID)).toBe(false); + const {result: parentReport} = renderHook(() => useParentReport(taskReport.reportID)); + const {result: isParentReportArchived} = renderHook(() => useReportIsArchived(parentReport.current?.reportID)); + expect(canActionTask(taskReport, employeeAccountID, undefined, undefined, parentReport.current, isParentReportArchived.current)).toBe(false); }); it('returns true if the logged in user is the one assigned to the task', () => { - expect(canActionTask(taskReport, taskAssigneeAccountID)).toBe(true); + const {result: parentReport} = renderHook(() => useParentReport(taskReport.reportID)); + const {result: isParentReportArchived} = renderHook(() => useReportIsArchived(parentReport.current?.reportID)); + expect(canActionTask(taskReport, taskAssigneeAccountID, undefined, undefined, parentReport.current, isParentReportArchived.current)).toBe(true); }); }); From 18ed22eab31f4cc97ca706d7b218ff8bc946b215 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 24 Apr 2025 13:47:05 -0700 Subject: [PATCH 11/51] Remove unnecessary parameters --- src/libs/actions/Task.ts | 13 ++----------- tests/actions/TaskTest.ts | 18 +++++++++--------- 2 files changed, 11 insertions(+), 20 deletions(-) diff --git a/src/libs/actions/Task.ts b/src/libs/actions/Task.ts index bfd394f35365..6ecefb2e62dd 100644 --- a/src/libs/actions/Task.ts +++ b/src/libs/actions/Task.ts @@ -1227,14 +1227,7 @@ function canModifyTask(taskReport: OnyxEntry, sessionAccountID /** * Check if you can change the status of the task (mark complete or incomplete). Only the task owner and task assignee can do this. */ -function canActionTask( - taskReport: OnyxEntry, - sessionAccountID: number, - taskOwnerAccountID?: number, - taskAssigneeAccountID?: number, - parentReport?: OnyxEntry, - isParentReportArchived = false, -): boolean { +function canActionTask(taskReport: OnyxEntry, sessionAccountID: number, parentReport?: OnyxEntry, isParentReportArchived = false): boolean { if (ReportUtils.isCanceledTaskReport(taskReport)) { return false; } @@ -1243,9 +1236,7 @@ function canActionTask( return false; } - const ownerAccountID = taskReport?.ownerAccountID ?? taskOwnerAccountID; - const assigneeAccountID = getTaskAssigneeAccountID(taskReport) ?? taskAssigneeAccountID; - return sessionAccountID === ownerAccountID || sessionAccountID === assigneeAccountID; + return sessionAccountID === taskReport?.ownerAccountID || sessionAccountID === getTaskAssigneeAccountID(taskReport); } function clearTaskErrors(reportID: string | undefined) { diff --git a/tests/actions/TaskTest.ts b/tests/actions/TaskTest.ts index ebd09a1fdf5f..1048079c59a5 100644 --- a/tests/actions/TaskTest.ts +++ b/tests/actions/TaskTest.ts @@ -107,7 +107,7 @@ describe('actions/Task', () => { const {result: isParentReportArchived} = renderHook(() => useReportIsArchived(parentReport.current?.reportID)); // The accountID doesn't matter here because the code will do an early return for the cancelled report - expect(canActionTask(taskReportCancelled, 0, undefined, undefined, parentReport.current, isParentReportArchived.current)).toBe(false); + expect(canActionTask(taskReportCancelled, 0, parentReport.current, isParentReportArchived.current)).toBe(false); }); it('returns false if the report has an archived parent report', () => { @@ -115,19 +115,19 @@ describe('actions/Task', () => { const {result: isParentReportArchived} = renderHook(() => useReportIsArchived(parentReport.current?.reportID)); // The accountID doesn't matter here because the code will do an early return for the archived report - expect(canActionTask(taskReportArchived, 0, undefined, undefined, parentReport.current, isParentReportArchived.current)).toBe(false); + expect(canActionTask(taskReportArchived, 0, parentReport.current, isParentReportArchived.current)).toBe(false); }); it('returns false if the user modifying the task is not the author', () => { const {result: parentReport} = renderHook(() => useParentReport(taskReport.reportID)); const {result: isParentReportArchived} = renderHook(() => useReportIsArchived(parentReport.current?.reportID)); - expect(canActionTask(taskReport, employeeAccountID, undefined, undefined, parentReport.current, isParentReportArchived.current)).toBe(false); + expect(canActionTask(taskReport, employeeAccountID, parentReport.current, isParentReportArchived.current)).toBe(false); }); it('returns true if the user modifying the task is the author', () => { const {result: parentReport} = renderHook(() => useParentReport(taskReport.reportID)); const {result: isParentReportArchived} = renderHook(() => useReportIsArchived(parentReport.current?.reportID)); - expect(canActionTask(taskReport, managerAccountID, undefined, undefined, parentReport.current, isParentReportArchived.current)).toBe(true); + expect(canActionTask(taskReport, managerAccountID, parentReport.current, isParentReportArchived.current)).toBe(true); }); // The third parameter of canActionTask is the accountID of the user who is assigned to the task @@ -136,19 +136,19 @@ describe('actions/Task', () => { it('returns true if the report has no parent and the third parameter is the same as the second', () => { const {result: parentReport} = renderHook(() => useParentReport(taskReportWithNoParent.reportID)); const {result: isParentReportArchived} = renderHook(() => useReportIsArchived(parentReport.current?.reportID)); - expect(canActionTask(taskReportWithNoParent, managerAccountID, undefined, undefined, parentReport.current, isParentReportArchived.current)).toBe(true); + expect(canActionTask(taskReportWithNoParent, managerAccountID, parentReport.current, isParentReportArchived.current)).toBe(true); }); it('returns false if the report has no parent, the logged on user is not the author or assignee', () => { const {result: parentReport} = renderHook(() => useParentReport(taskReportWithNoParent.reportID)); const {result: isParentReportArchived} = renderHook(() => useReportIsArchived(parentReport.current?.reportID)); - expect(canActionTask(taskReportWithNoParent, employeeAccountID, undefined, undefined, parentReport.current, isParentReportArchived.current)).toBe(false); + expect(canActionTask(taskReportWithNoParent, employeeAccountID, parentReport.current, isParentReportArchived.current)).toBe(false); }); it('returns false if the report has no parent, the logged on user is not the author or assignee', () => { const {result: parentReport} = renderHook(() => useParentReport(taskReportWithNoParent.reportID)); const {result: isParentReportArchived} = renderHook(() => useReportIsArchived(parentReport.current?.reportID)); - expect(canActionTask(taskReportWithNoParent, employeeAccountID, undefined, undefined, parentReport.current, isParentReportArchived.current)).toBe(false); + expect(canActionTask(taskReportWithNoParent, employeeAccountID, parentReport.current, isParentReportArchived.current)).toBe(false); }); }); @@ -172,13 +172,13 @@ describe('actions/Task', () => { it('returns false if the logged in user is not the author or the one assigned to the task', () => { const {result: parentReport} = renderHook(() => useParentReport(taskReport.reportID)); const {result: isParentReportArchived} = renderHook(() => useReportIsArchived(parentReport.current?.reportID)); - expect(canActionTask(taskReport, employeeAccountID, undefined, undefined, parentReport.current, isParentReportArchived.current)).toBe(false); + expect(canActionTask(taskReport, employeeAccountID, parentReport.current, isParentReportArchived.current)).toBe(false); }); it('returns true if the logged in user is the one assigned to the task', () => { const {result: parentReport} = renderHook(() => useParentReport(taskReport.reportID)); const {result: isParentReportArchived} = renderHook(() => useReportIsArchived(parentReport.current?.reportID)); - expect(canActionTask(taskReport, taskAssigneeAccountID, undefined, undefined, parentReport.current, isParentReportArchived.current)).toBe(true); + expect(canActionTask(taskReport, taskAssigneeAccountID, parentReport.current, isParentReportArchived.current)).toBe(true); }); }); From 984742d624ed5f8b7db048d1686e5bb1bd3a8806 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 24 Apr 2025 13:51:58 -0700 Subject: [PATCH 12/51] Add comment --- tests/actions/TaskTest.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/actions/TaskTest.ts b/tests/actions/TaskTest.ts index 1048079c59a5..b49315489049 100644 --- a/tests/actions/TaskTest.ts +++ b/tests/actions/TaskTest.ts @@ -103,6 +103,7 @@ describe('actions/Task', () => { describe('canActionTask', () => { it('returns false if the report is a cancelled task report', () => { + // Simulate how components call canActionTask() by using hooks to get the parent report and see if it is archived const {result: parentReport} = renderHook(() => useParentReport(taskReportCancelled.reportID)); const {result: isParentReportArchived} = renderHook(() => useReportIsArchived(parentReport.current?.reportID)); From f85e169356ebb7808fce485c5edd708a7e37cc81 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 24 Apr 2025 14:00:27 -0700 Subject: [PATCH 13/51] Make second param optional --- src/libs/actions/Task.ts | 6 +++++- tests/actions/TaskTest.ts | 15 ++++++--------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/libs/actions/Task.ts b/src/libs/actions/Task.ts index 6ecefb2e62dd..8579cc22c471 100644 --- a/src/libs/actions/Task.ts +++ b/src/libs/actions/Task.ts @@ -1227,7 +1227,11 @@ function canModifyTask(taskReport: OnyxEntry, sessionAccountID /** * Check if you can change the status of the task (mark complete or incomplete). Only the task owner and task assignee can do this. */ -function canActionTask(taskReport: OnyxEntry, sessionAccountID: number, parentReport?: OnyxEntry, isParentReportArchived = false): boolean { +function canActionTask(taskReport: OnyxEntry, sessionAccountID?: number, parentReport?: OnyxEntry, isParentReportArchived = false): boolean { + if (!sessionAccountID) { + return false; + } + if (ReportUtils.isCanceledTaskReport(taskReport)) { return false; } diff --git a/tests/actions/TaskTest.ts b/tests/actions/TaskTest.ts index b49315489049..9fdfbc5ab5ca 100644 --- a/tests/actions/TaskTest.ts +++ b/tests/actions/TaskTest.ts @@ -102,21 +102,18 @@ describe('actions/Task', () => { }); describe('canActionTask', () => { - it('returns false if the report is a cancelled task report', () => { - // Simulate how components call canActionTask() by using hooks to get the parent report and see if it is archived - const {result: parentReport} = renderHook(() => useParentReport(taskReportCancelled.reportID)); - const {result: isParentReportArchived} = renderHook(() => useReportIsArchived(parentReport.current?.reportID)); + it('returns false if there is no logged in user', () => { + expect(canActionTask(taskReportCancelled)).toBe(false); + }); + it('returns false if the report is a cancelled task report', () => { // The accountID doesn't matter here because the code will do an early return for the cancelled report - expect(canActionTask(taskReportCancelled, 0, parentReport.current, isParentReportArchived.current)).toBe(false); + expect(canActionTask(taskReportCancelled, 0)).toBe(false); }); it('returns false if the report has an archived parent report', () => { - const {result: parentReport} = renderHook(() => useParentReport(taskReportArchived.reportID)); - const {result: isParentReportArchived} = renderHook(() => useReportIsArchived(parentReport.current?.reportID)); - // The accountID doesn't matter here because the code will do an early return for the archived report - expect(canActionTask(taskReportArchived, 0, parentReport.current, isParentReportArchived.current)).toBe(false); + expect(canActionTask(taskReportArchived, 0)).toBe(false); }); it('returns false if the user modifying the task is not the author', () => { From d46050f401dd038639d5923ed2f36d8d5191744e Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 24 Apr 2025 14:00:51 -0700 Subject: [PATCH 14/51] Implement new method signature --- src/components/TaskHeaderActionButton.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/components/TaskHeaderActionButton.tsx b/src/components/TaskHeaderActionButton.tsx index 7265ef82a95c..577aaa85fd78 100644 --- a/src/components/TaskHeaderActionButton.tsx +++ b/src/components/TaskHeaderActionButton.tsx @@ -1,6 +1,8 @@ import React from 'react'; import {View} from 'react-native'; import useLocalize from '@hooks/useLocalize'; +import useParentReport from '@hooks/useParentReport'; +import useReportIsArchived from '@hooks/useReportIsArchived'; import useThemeStyles from '@hooks/useThemeStyles'; import {canWriteInReport, isCompletedTaskReport} from '@libs/ReportUtils'; import {isActiveTaskEditRoute} from '@libs/TaskUtils'; @@ -20,6 +22,8 @@ function TaskHeaderActionButton({report}: TaskHeaderActionButtonProps) { const {translate} = useLocalize(); const styles = useThemeStyles(); const session = useSession(); + const parentReport = useParentReport(report.reportID); + const isParentReportArchived = useReportIsArchived(parentReport?.reportID); if (!canWriteInReport(report)) { return null; @@ -29,7 +33,7 @@ function TaskHeaderActionButton({report}: TaskHeaderActionButtonProps) {