Skip to content
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.

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

Align default IDs in Report.ts file #54306

Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
0612e06
fix: undo default values to comply with eslint rules
pac-guerreiro Dec 18, 2024
f927a37
doc: fix typo in comment
pac-guerreiro Dec 18, 2024
8d1cf91
Merge branch 'main' into pac-guerreiro/fix/50360-undo-default-values-…
pac-guerreiro Dec 19, 2024
7f97310
chore: apply suggestions and follow guidelines
pac-guerreiro Dec 19, 2024
430a195
Merge branch 'main' into pac-guerreiro/fix/50360-undo-default-values-…
pac-guerreiro Dec 19, 2024
2e07d76
Merge branch 'main' into pac-guerreiro/fix/50360-undo-default-values-…
pac-guerreiro Dec 20, 2024
4be070e
chore: resolve eslint issues
pac-guerreiro Dec 20, 2024
34de8a5
Merge branch 'main' into pac-guerreiro/fix/50360-undo-default-values-…
pac-guerreiro Jan 2, 2025
8d6f43c
Merge branch 'main' into pac-guerreiro/fix/50360-undo-default-values-…
pac-guerreiro Jan 3, 2025
b589b0f
Merge branch 'main' into pac-guerreiro/fix/50360-undo-default-values-…
pac-guerreiro Jan 7, 2025
b7e916d
Merge branch 'main' into pac-guerreiro/fix/50360-undo-default-values-…
pac-guerreiro Jan 8, 2025
3d6e6a3
refactor: make optimistic task.parentReportID required
pac-guerreiro Jan 8, 2025
95e6510
Merge branch 'main' into pac-guerreiro/fix/50360-undo-default-values-…
pac-guerreiro Jan 8, 2025
0d5a338
refactor: apply suggestions
pac-guerreiro Jan 8, 2025
f0fb184
Merge branch 'main' into pac-guerreiro/fix/50360-undo-default-values-…
pac-guerreiro Jan 14, 2025
7a4971c
Merge branch 'main' into pac-guerreiro/fix/50360-undo-default-values-…
pac-guerreiro Jan 14, 2025
805a33b
refactor: apply suggestions
pac-guerreiro Jan 15, 2025
026a422
Merge branch 'main' into pac-guerreiro/fix/50360-undo-default-values-…
pac-guerreiro Jan 16, 2025
b47b57c
refactor: apply suggestions
pac-guerreiro Jan 16, 2025
9e3b8b7
Merge branch 'main' into pac-guerreiro/fix/50360-undo-default-values-…
pac-guerreiro Jan 20, 2025
20c7866
refactor: apply suggestions
pac-guerreiro Jan 20, 2025
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
2 changes: 1 addition & 1 deletion src/libs/API/parameters/AddCommentOrAttachementParams.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type {FileObject} from '@components/AttachmentModal';

type AddCommentOrAttachementParams = {
reportID: string;
reportID?: string;
pac-guerreiro marked this conversation as resolved.
Show resolved Hide resolved
reportActionID?: string;
commentReportActionID?: string | null;
reportComment?: string;
Expand Down
2 changes: 1 addition & 1 deletion src/libs/API/parameters/SearchForRoomsToMentionParams.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
type SearchForRoomsToMentionParams = {
query: string;
policyID: string;
policyID?: string;
};

export default SearchForRoomsToMentionParams;
10 changes: 7 additions & 3 deletions src/libs/ReportActionsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -772,7 +772,11 @@ function replaceBaseURLInPolicyChangeLogAction(reportAction: ReportAction): Repo
return updatedReportAction;
}

function getLastVisibleAction(reportID: string, canUserPerformWriteAction?: boolean, actionsToMerge: Record<string, NullishDeep<ReportAction> | null> = {}): OnyxEntry<ReportAction> {
function getLastVisibleAction(
reportID: string | undefined,
canUserPerformWriteAction?: boolean,
actionsToMerge: Record<string, NullishDeep<ReportAction> | null> = {},
): OnyxEntry<ReportAction> {
let reportActions: Array<ReportAction | null | undefined> = [];
if (!isEmpty(actionsToMerge)) {
reportActions = Object.values(fastMerge(allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`] ?? {}, actionsToMerge ?? {}, true)) as Array<
Expand Down Expand Up @@ -802,12 +806,12 @@ function formatLastMessageText(lastMessageText: string) {
}

function getLastVisibleMessage(
reportID: string,
reportID: string | undefined,
canUserPerformWriteAction?: boolean,
actionsToMerge: Record<string, NullishDeep<ReportAction> | null> = {},
reportAction: OnyxInputOrEntry<ReportAction> | undefined = undefined,
): LastVisibleMessage {
const lastVisibleAction = reportAction ?? getLastVisibleAction(reportID, canUserPerformWriteAction, actionsToMerge);
const lastVisibleAction = reportAction ?? (reportID ? getLastVisibleAction(reportID, canUserPerformWriteAction, actionsToMerge) : undefined);
pac-guerreiro marked this conversation as resolved.
Show resolved Hide resolved
const message = getReportActionMessage(lastVisibleAction);

if (message && isReportMessageAttachment(message)) {
Expand Down
2 changes: 1 addition & 1 deletion src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4535,7 +4535,7 @@ function buildOptimisticTaskCommentReportAction(
taskTitle: string,
taskAssigneeAccountID: number,
text: string,
parentReportID: string,
parentReportID: string | undefined,
actorAccountID?: number,
createdOffset = 0,
): OptimisticReportAction {
Expand Down
98 changes: 53 additions & 45 deletions src/libs/actions/Report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,9 @@ type TaskForParameters =
type: 'task';
task: string;
taskReportID: string;
parentReportID: string;
parentReportID?: string;
parentReportActionID: string;
assigneeChatReportID: string;
assigneeChatReportID?: string;
createdTaskReportActionID: string;
completedTaskReportActionID?: string;
title: string;
Expand Down Expand Up @@ -803,11 +803,11 @@ function clearAvatarErrors(reportID: string) {
* @param participantAccountIDList The list of accountIDs that are included in a new chat, not including the user creating it
*/
function openReport(
reportID: string,
reportID: string | undefined,
reportActionID?: string,
participantLoginList: string[] = [],
newReportObject?: ReportUtils.OptimisticChatReport,
parentReportActionID = '-1',
parentReportActionID?: string,
isFromDeepLink = false,
participantAccountIDList: number[] = [],
avatar?: File | CustomRNImageManipulatorResult,
Expand Down Expand Up @@ -1064,7 +1064,7 @@ function openReport(
failureData.push({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${newReportObject.parentReportID}`,
value: {[parentReportActionID]: {childReportID: '-1', childType: ''}},
value: {[parentReportActionID]: {childReportID: undefined, childType: ''}},
pac-guerreiro marked this conversation as resolved.
Show resolved Hide resolved
});
}
}
Expand Down Expand Up @@ -1141,12 +1141,12 @@ function navigateToAndOpenReport(
const report = isEmptyObject(chat) ? newChat : chat;

// We want to pass newChat here because if anything is passed in that param (even an existing chat), we will try to create a chat on the server
openReport(report?.reportID ?? '', '', userLogins, newChat, undefined, undefined, undefined, avatarFile);
openReport(report?.reportID, '', userLogins, newChat, undefined, undefined, undefined, avatarFile);
if (shouldDismissModal) {
Navigation.dismissModalWithReport(report);
} else {
Navigation.navigateWithSwitchPolicyID({route: ROUTES.HOME});
Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(report?.reportID ?? '-1'), actionType);
Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(`${report?.reportID}`), actionType);
}
}

Expand All @@ -1164,7 +1164,7 @@ function navigateToAndOpenReportWithAccountIDs(participantAccountIDs: number[])
const report = chat ?? newChat;

// We want to pass newChat here because if anything is passed in that param (even an existing chat), we will try to create a chat on the server
openReport(report?.reportID ?? '', '', [], newChat, '0', false, participantAccountIDs);
openReport(report?.reportID, '', [], newChat, '0', false, participantAccountIDs);
Navigation.dismissModalWithReport(report);
}

Expand All @@ -1175,8 +1175,8 @@ function navigateToAndOpenReportWithAccountIDs(participantAccountIDs: number[])
* @param parentReportAction the parent comment of a thread
* @param parentReportID The reportID of the parent
*/
function navigateToAndOpenChildReport(childReportID = '-1', parentReportAction: Partial<ReportAction> = {}, parentReportID = '0') {
if (childReportID !== '-1' && childReportID !== '0') {
function navigateToAndOpenChildReport(childReportID: string | undefined, parentReportAction: Partial<ReportAction> = {}, parentReportID?: string) {
if (childReportID) {
Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(childReportID));
} else {
const participantAccountIDs = [...new Set([currentUserAccountID, Number(parentReportAction.actorAccountID)])];
Expand Down Expand Up @@ -1363,8 +1363,8 @@ function readNewestAction(reportID: string, shouldResetUnreadMarker = false) {
/**
* Sets the last read time on a report
*/
function markCommentAsUnread(reportID: string, reportActionCreated: string) {
if (reportID === '-1') {
function markCommentAsUnread(reportID: string | undefined, reportActionCreated: string) {
if (!reportID) {
Log.warn('7339cd6c-3263-4f89-98e5-730f0be15784 Invalid report passed to MarkCommentAsUnread. Not calling the API because it wil fail.');
return;
}
Expand Down Expand Up @@ -1466,36 +1466,38 @@ function handleReportChanged(report: OnyxEntry<Report>) {
return;
}

const {reportID, preexistingReportID, parentReportID, parentReportActionID} = report;

// Handle cleanup of stale optimistic IOU report and its report preview separately
if (report?.reportID && report.preexistingReportID && ReportUtils.isMoneyRequestReport(report) && report?.parentReportActionID) {
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.parentReportID}`, {
[report.parentReportActionID]: null,
if (reportID && preexistingReportID && ReportUtils.isMoneyRequestReport(report) && parentReportActionID) {
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${parentReportID}`, {
[parentReportActionID]: null,
});
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, null);
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, null);
return;
}

// It is possible that we optimistically created a DM/group-DM for a set of users for which a report already exists.
// In this case, the API will let us know by returning a preexistingReportID.
// We should clear out the optimistically created report and re-route the user to the preexisting report.
if (report?.reportID && report.preexistingReportID) {
if (reportID && preexistingReportID) {
let callback = () => {
Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, null);
Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${report.preexistingReportID}`, {...report, reportID: report.preexistingReportID, preexistingReportID: null});
Onyx.set(`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${report.reportID}`, null);
Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, null);
Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${preexistingReportID}`, {...report, reportID: preexistingReportID, preexistingReportID: null});
Onyx.set(`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`, null);
};
// Only re-route them if they are still looking at the optimistically created report
if (Navigation.getActiveRoute().includes(`/r/${report.reportID}`)) {
if (Navigation.getActiveRoute().includes(`/r/${reportID}`)) {
const currCallback = callback;
callback = () => {
currCallback();
Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(report.preexistingReportID ?? '-1'), CONST.NAVIGATION.TYPE.UP);
Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(preexistingReportID), CONST.NAVIGATION.TYPE.UP);
};

// The report screen will listen to this event and transfer the draft comment to the existing report
// This will allow the newest draft comment to be transferred to the existing report
DeviceEventEmitter.emit(`switchToPreExistingReport_${report.reportID}`, {
preexistingReportID: report.preexistingReportID,
DeviceEventEmitter.emit(`switchToPreExistingReport_${reportID}`, {
preexistingReportID,
callback,
});

Expand All @@ -1504,20 +1506,20 @@ function handleReportChanged(report: OnyxEntry<Report>) {

// In case the user is not on the report screen, we will transfer the report draft comment directly to the existing report
// after that clear the optimistically created report
const draftReportComment = allReportDraftComments?.[`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${report.reportID}`];
const draftReportComment = allReportDraftComments?.[`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`];
if (!draftReportComment) {
callback();
return;
}

saveReportDraftComment(report.preexistingReportID ?? '-1', draftReportComment, callback);
saveReportDraftComment(preexistingReportID, draftReportComment, callback);

return;
}

if (report?.reportID) {
if (reportID) {
if (ReportUtils.isConciergeChatReport(report)) {
conciergeChatReportID = report.reportID;
conciergeChatReportID = reportID;
}
}
}
Expand Down Expand Up @@ -1935,10 +1937,15 @@ function updateRoomVisibility(reportID: string, previousValue: RoomVisibility |
* @param parentReportID The reportID of the parent
* @param prevNotificationPreference The previous notification preference for the child report
*/
function toggleSubscribeToChildReport(childReportID = '-1', parentReportAction: Partial<ReportAction> = {}, parentReportID = '-1', prevNotificationPreference?: NotificationPreference) {
if (childReportID !== '-1') {
function toggleSubscribeToChildReport(
childReportID: string | undefined,
parentReportAction: Partial<ReportAction> = {},
parentReportID?: string,
prevNotificationPreference?: NotificationPreference,
) {
if (childReportID) {
openReport(childReportID);
const parentReportActionID = parentReportAction?.reportActionID ?? '-1';
const parentReportActionID = parentReportAction?.reportActionID;
if (!prevNotificationPreference || ReportUtils.isHiddenForCurrentUser(prevNotificationPreference)) {
updateNotificationPreference(childReportID, prevNotificationPreference, CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS, parentReportID, parentReportActionID);
} else {
Expand Down Expand Up @@ -2897,7 +2904,7 @@ function navigateToMostRecentReport(currentReport: OnyxEntry<Report>) {
const lastAccessedReportID = ReportUtils.findLastAccessedReport(false, false, undefined, currentReport?.reportID)?.reportID;

if (lastAccessedReportID) {
const lastAccessedReportRoute = ROUTES.REPORT_WITH_ID.getRoute(lastAccessedReportID ?? '-1');
const lastAccessedReportRoute = ROUTES.REPORT_WITH_ID.getRoute(lastAccessedReportID);
Navigation.goBack(lastAccessedReportRoute);
} else {
const isChatThread = ReportUtils.isChatThread(currentReport);
Expand Down Expand Up @@ -3072,8 +3079,7 @@ function leaveRoom(reportID: string, isWorkspaceMemberLeavingWorkspaceRoom = fal
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.parentReportID}`,
value: {
[report.parentReportActionID]: {
childReportNotificationPreference:
report?.participants?.[currentUserAccountID ?? -1]?.notificationPreference ?? ReportUtils.getDefaultNotificationPreferenceForReport(report),
childReportNotificationPreference: report?.participants?.[currentUserAccountID]?.notificationPreference ?? ReportUtils.getDefaultNotificationPreferenceForReport(report),
},
},
});
Expand Down Expand Up @@ -3548,7 +3554,7 @@ function prepareOnboardingOptimisticData(
const integrationName = userReportedIntegration ? CONST.ONBOARDING_ACCOUNTING_MAPPING[userReportedIntegration] : '';
const adminsChatReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${adminsChatReportID}`];
const targetChatReport = shouldPostTasksInAdminsRoom ? adminsChatReport : ReportUtils.getChatByParticipants([CONST.ACCOUNT_ID.CONCIERGE, currentUserAccountID]);
const {reportID: targetChatReportID = '', policyID: targetChatPolicyID = ''} = targetChatReport ?? {};
const {reportID: targetChatReportID, policyID: targetChatPolicyID} = targetChatReport ?? {};
const assignedGuideEmail = getPolicy(targetChatPolicyID)?.assignedGuide?.email ?? 'Setup Specialist';
const assignedGuidePersonalDetail = Object.values(allPersonalDetails ?? {}).find((personalDetail) => personalDetail?.login === assignedGuideEmail);
let assignedGuideAccountID: number;
Expand Down Expand Up @@ -3600,14 +3606,14 @@ function prepareOnboardingOptimisticData(
const taskDescription =
typeof task.description === 'function'
? task.description({
adminsRoomLink: `${environmentURL}/${ROUTES.REPORT_WITH_ID.getRoute(adminsChatReportID ?? '-1')}`,
workspaceCategoriesLink: `${environmentURL}/${ROUTES.WORKSPACE_CATEGORIES.getRoute(onboardingPolicyID ?? '-1')}`,
workspaceMembersLink: `${environmentURL}/${ROUTES.WORKSPACE_MEMBERS.getRoute(onboardingPolicyID ?? '-1')}`,
workspaceMoreFeaturesLink: `${environmentURL}/${ROUTES.WORKSPACE_MORE_FEATURES.getRoute(onboardingPolicyID ?? '-1')}`,
adminsRoomLink: `${environmentURL}/${ROUTES.REPORT_WITH_ID.getRoute(`${adminsChatReportID}`)}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess from user perspective we will have the same result if we just pass adminsChatReportID

Suggested change
adminsRoomLink: `${environmentURL}/${ROUTES.REPORT_WITH_ID.getRoute(`${adminsChatReportID}`)}`,
adminsRoomLink: `${environmentURL}/${ROUTES.REPORT_WITH_ID.getRoute(adminsChatReportID)}`,

Could you try to navigate the route with -1 and undefined values, will it act the same?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@VickyStash You're right but if I didn't make this change, then eslint gets upset with me for passing an undefined value 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it can be resolved by just updating the type, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

UPD: Similar case is discussed in another PR #54534 (comment)

workspaceCategoriesLink: `${environmentURL}/${ROUTES.WORKSPACE_CATEGORIES.getRoute(`${onboardingPolicyID}`)}`,
workspaceMembersLink: `${environmentURL}/${ROUTES.WORKSPACE_MEMBERS.getRoute(`${onboardingPolicyID}`)}`,
workspaceMoreFeaturesLink: `${environmentURL}/${ROUTES.WORKSPACE_MORE_FEATURES.getRoute(`${onboardingPolicyID}`)}`,
navatticURL: getNavatticURL(environment, engagementChoice),
integrationName,
workspaceAccountingLink: `${environmentURL}/${ROUTES.POLICY_ACCOUNTING.getRoute(onboardingPolicyID ?? '-1')}`,
workspaceSettingsLink: `${environmentURL}/${ROUTES.WORKSPACE_INITIAL.getRoute(onboardingPolicyID ?? '-1')}`,
workspaceAccountingLink: `${environmentURL}/${ROUTES.POLICY_ACCOUNTING.getRoute(`${onboardingPolicyID}`)}`,
workspaceSettingsLink: `${environmentURL}/${ROUTES.WORKSPACE_INITIAL.getRoute(`${onboardingPolicyID}`)}`,
})
: task.description;
const taskTitle =
Expand Down Expand Up @@ -3657,11 +3663,11 @@ function prepareOnboardingOptimisticData(
type: 'task',
task: task.type,
taskReportID: currentTask.reportID,
parentReportID: currentTask.parentReportID ?? '-1',
parentReportID: currentTask.parentReportID,
parentReportActionID: taskReportAction.reportAction.reportActionID,
assigneeChatReportID: '',
assigneeChatReportID: undefined,
pac-guerreiro marked this conversation as resolved.
Show resolved Hide resolved
createdTaskReportActionID: taskCreatedAction.reportActionID,
completedTaskReportActionID: completedTaskReportAction?.reportActionID ?? undefined,
completedTaskReportActionID: completedTaskReportAction?.reportActionID,
title: currentTask.reportName ?? '',
description: taskDescription ?? '',
}));
Expand Down Expand Up @@ -4100,7 +4106,9 @@ function searchForReports(searchInput: string, policyID?: string) {
},
];

const searchForRoomToMentionParams: SearchForRoomsToMentionParams = {query: searchInput, policyID: policyID ?? '-1'};
// API expects a string, that's why policyID must default to a string
// eslint-disable-next-line rulesdir/no-default-id-values
const searchForRoomToMentionParams: SearchForRoomsToMentionParams = {query: searchInput, policyID};
const searchForReportsParams: SearchForReportsParams = {searchInput, canCancel: true};

// We want to cancel all pending SearchForReports API calls before making another one
Expand Down
Loading
Loading