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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Correctly update thread ancestors reply count #38658

Merged
merged 5 commits into from
Mar 27, 2024
Merged

Correctly update thread ancestors reply count #38658

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5a71949
update all parent/ancestor thread data
bernhardoj Mar 20, 2024
cbdba9e
fix type
bernhardoj Mar 20, 2024
4abbbd4
fix lint
bernhardoj Mar 20, 2024
7e94513
allow to include transaction thread
bernhardoj Mar 20, 2024
f5afda1
fix lint
bernhardoj Mar 20, 2024
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
52 changes: 24 additions & 28 deletions src/libs/ReportUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2957,34 +2957,6 @@
};
}

/**
* Get optimistic data of parent report action
* @param reportID The reportID of the report that is updated
* @param lastVisibleActionCreated Last visible action created of the child report
* @param type The type of action in the child report
* @param parentReportID Custom reportID to be updated
* @param parentReportActionID Custom reportActionID to be updated
*/
function getOptimisticDataForParentReportAction(reportID: string, lastVisibleActionCreated: string, type: string, parentReportID = '', parentReportActionID = ''): OnyxUpdate | EmptyObject {
const report = getReport(reportID);
if (!report || isEmptyObject(report)) {
return {};
}
const parentReportAction = ReportActionsUtils.getParentReportAction(report);
if (!parentReportAction || isEmptyObject(parentReportAction)) {
return {};
}

const optimisticParentReportAction = updateOptimisticParentReportAction(parentReportAction, lastVisibleActionCreated, type);
return {
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${parentReportID || report?.parentReportID}`,
value: {
[parentReportActionID || (report?.parentReportActionID ?? '')]: optimisticParentReportAction,
},
};
}

/**
* Builds an optimistic reportAction for the parent report when a task is created
* @param taskReportID - Report ID of the task
Expand Down Expand Up @@ -5280,6 +5252,30 @@
return allAncestorIDs;
}

/**
* Get optimistic data of parent report action
* @param reportID The reportID of the report that is updated
* @param lastVisibleActionCreated Last visible action created of the child report
* @param type The type of action in the child report
*/
function getOptimisticDataForParentReportAction(reportID: string, lastVisibleActionCreated: string, type: string): OnyxUpdate[] {
const report = getReport(reportID);
const ancestors = getAllAncestorReportActionIDs(report);

Check failure on line 5263 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / typecheck 

Argument of type 'OnyxEntry<{ avatarUrl?: string | undefined; chatType?: ValueOf<{ readonly POLICY_ANNOUNCE: "policyAnnounce"; readonly POLICY_ADMINS: "policyAdmins"; readonly DOMAIN_ALL: "domainAll"; readonly POLICY_ROOM: "policyRoom"; readonly POLICY_EXPENSE_CHAT: "policyExpenseChat"; readonly SELF_DM: "selfDM"; }> | undefined; ......' is not assignable to parameter of type '({ avatarUrl?: string | undefined; chatType?: ValueOf<{ readonly POLICY_ANNOUNCE: "policyAnnounce"; readonly POLICY_ADMINS: "policyAdmins"; readonly DOMAIN_ALL: "domainAll"; readonly POLICY_ROOM: "policyRoom"; readonly POLICY_EXPENSE_CHAT: "policyExpenseChat"; readonly SELF_DM: "selfDM"; }> | undefined; ... 67 more ...'.
const totalAncestor = ancestors.reportIDs.length;

return Array.from(Array(totalAncestor), (_, index) => {
const ancestorReport = getReport(ancestors.reportIDs[index]);
const ancestorReportAction = ReportActionsUtils.getReportAction(ancestorReport.reportID, ancestors.reportActionsIDs[index]);

Check failure on line 5268 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / typecheck 

'ancestorReport' is possibly 'null'.
return {
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${ancestorReport.reportID}`,

Check failure on line 5271 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / typecheck 

'ancestorReport' is possibly 'null'.
value: {
[ancestorReportAction?.reportActionID ?? '']: updateOptimisticParentReportAction(ancestorReportAction, lastVisibleActionCreated, type),
},
};
});
}

function canBeAutoReimbursed(report: OnyxEntry<Report>, policy: OnyxEntry<Policy> | EmptyObject): boolean {
if (isEmptyObject(policy)) {
return false;
Expand Down
8 changes: 2 additions & 6 deletions src/libs/actions/Report.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,9 +504,7 @@ function addActions(reportID: string, text = '', file?: FileObject) {

// Update optimistic data for parent report action if the report is a child report
const optimisticParentReportData = ReportUtils.getOptimisticDataForParentReportAction(reportID, currentTime, CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD);
if (!isEmptyObject(optimisticParentReportData)) {
optimisticData.push(optimisticParentReportData);
}
optimisticParentReportData.forEach((parentReportData) => optimisticData.push(parentReportData));

// Update the timezone if it's been 5 minutes from the last time the user added a comment
if (DateUtils.canUpdateTimezone() && currentUserAccountID) {
Expand Down Expand Up @@ -1228,9 +1226,7 @@ function deleteReportComment(reportID: string, reportAction: ReportAction) {
optimisticReport?.lastVisibleActionCreated ?? '',
CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE,
);
if (!isEmptyObject(optimisticParentReportData)) {
optimisticData.push(optimisticParentReportData);
}
optimisticParentReportData.forEach((parentReportData) => optimisticData.push(parentReportData));
}

const parameters: DeleteCommentParams = {
Expand Down
8 changes: 2 additions & 6 deletions src/libs/actions/Task.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,9 +249,7 @@ function createTaskAndNavigate(

// If needed, update optimistic data for parent report action of the parent report.
const optimisticParentReportData = ReportUtils.getOptimisticDataForParentReportAction(parentReportID, currentTime, CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD);
if (!isEmptyObject(optimisticParentReportData)) {
optimisticData.push(optimisticParentReportData);
}
optimisticParentReportData.forEach((parentReportData) => optimisticData.push(parentReportData));

// FOR PARENT REPORT (SHARE DESTINATION)
successData.push({
Expand Down Expand Up @@ -861,9 +859,7 @@ function deleteTask(report: OnyxEntry<OnyxTypes.Report>) {
parentReport?.lastVisibleActionCreated ?? '',
CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE,
);
if (!isEmptyObject(optimisticParentReportData)) {
optimisticData.push(optimisticParentReportData);
}
optimisticParentReportData.forEach((parentReportData) => optimisticData.push(parentReportData));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I notice 1 issue when deleting a task from a thread. Even though we already optimistically update the reply count to 0, the BE then will set it again back to 1 even though the task is deleted from the thread.

Screen.Recording.2024-03-20.at.14.29.27.mov

Copy link
Contributor

Choose a reason for hiding this comment

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

cc @puneetlath Can you take a look at this bug. It seems this will require a backend fix.

}

const successData: OnyxUpdate[] = [
Expand Down
Loading