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

fix: update parent action for assignee report as well #24110

Merged
merged 18 commits into from
Aug 8, 2023
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
4 changes: 2 additions & 2 deletions src/components/ReportActionItem/TaskPreview.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,9 @@ function TaskPreview(props) {
disabled={ReportUtils.isCanceledTaskReport(props.taskReport)}
onPress={() => {
if (isTaskCompleted) {
Task.reopenTask(props.taskReportID, taskTitle);
Task.reopenTask(props.taskReport, taskTitle);
s-alves10 marked this conversation as resolved.
Show resolved Hide resolved
techievivek marked this conversation as resolved.
Show resolved Hide resolved
} else {
Task.completeTask(props.taskReportID, taskTitle);
Task.completeTask(props.taskReport, taskTitle);
}
}}
accessibilityLabel={props.translate('task.task')}
Expand Down
2 changes: 1 addition & 1 deletion src/components/ReportActionItem/TaskView.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ function TaskView(props) {
<Text style={styles.taskTitleDescription}>{props.translate('task.title')}</Text>
<View style={[styles.flexRow, styles.alignItemsTop, styles.flex1]}>
<Checkbox
onPress={() => (isCompleted ? Task.reopenTask(props.report.reportID, taskTitle) : Task.completeTask(props.report.reportID, taskTitle))}
onPress={() => (isCompleted ? Task.reopenTask(props.report, taskTitle) : Task.completeTask(props.report, taskTitle))}
isChecked={isCompleted}
style={styles.taskMenuItemCheckbox}
containerSize={24}
Expand Down
4 changes: 1 addition & 3 deletions src/components/TaskHeaderActionButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,7 @@ function TaskHeaderActionButton(props) {
medium
text={props.translate(ReportUtils.isCompletedTaskReport(props.report) ? 'task.markAsIncomplete' : 'task.markAsDone')}
onPress={() =>
ReportUtils.isCompletedTaskReport(props.report)
? Task.reopenTask(props.report.reportID, props.report.reportName)
: Task.completeTask(props.report.reportID, props.report.reportName)
ReportUtils.isCompletedTaskReport(props.report) ? Task.reopenTask(props.report, props.report.reportName) : Task.completeTask(props.report, props.report.reportName)
}
style={[styles.flex1]}
/>
Expand Down
12 changes: 12 additions & 0 deletions src/libs/ReportActionsUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,17 @@ function getParentReportAction(report, allReportActionsParam = undefined) {
return lodashGet(allReportActionsParam || allReportActions, [report.parentReportID, report.parentReportActionID], {});
}

/**
* Find the reportAction having the given childReportID in parent report actions
*
* @param {String} childReportID
* @param {String} parentReportID
* @returns {Object}
*/
function getParentReportActionInReport(childReportID, parentReportID) {
return _.find(allReportActions[parentReportID], (reportAction) => reportAction && `${reportAction.childReportID}` === `${childReportID}`);
}

/**
* Determines if the given report action is sent money report action by checking for 'pay' type and presence of IOUDetails object.
*
Expand Down Expand Up @@ -583,6 +594,7 @@ export {
getReportPreviewAction,
isCreatedTaskReportAction,
getParentReportAction,
getParentReportActionInReport,
isTransactionThread,
getFormattedAmount,
isSentMoneyReportAction,
Expand Down
55 changes: 41 additions & 14 deletions src/libs/ReportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -1574,24 +1574,26 @@ function updateOptimisticParentReportAction(parentReportAction, lastVisibleActio
* @param {String} reportID The reportID of the report that is updated
* @param {String} lastVisibleActionCreated Last visible action created of the child report
* @param {String} type The type of action in the child report
* @param {String} parentReportID Custom reportID to be updated
* @param {String} parentReportActionID Custom reportActionID to be updated
* @returns {Object}
*/
const getOptimisticDataForParentReportAction = (reportID, lastVisibleActionCreated, type) => {
function getOptimisticDataForParentReportAction(reportID, lastVisibleActionCreated, type, parentReportID = '', parentReportActionID = '') {
const report = getReport(reportID);
if (report && report.parentReportActionID) {
const parentReportAction = ReportActionsUtils.getParentReportAction(report);
if (parentReportAction && parentReportAction.reportActionID) {
return {
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.parentReportID}`,
value: {
[parentReportAction.reportActionID]: updateOptimisticParentReportAction(parentReportAction, lastVisibleActionCreated, type),
},
};
}
const parentReportAction = ReportActionsUtils.getParentReportAction(report);
if (_.isEmpty(parentReportAction)) {
return {};
}
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
Expand Down Expand Up @@ -2770,6 +2772,30 @@ function getParentReport(report) {
return lodashGet(allReports, `${ONYXKEYS.COLLECTION.REPORT}${report.parentReportID}`, {});
}

/**
* Find the parent report action in assignee report for a task report
s-alves10 marked this conversation as resolved.
Show resolved Hide resolved
* Returns an empty object if assignee report is the same as the share destination report
*
* @param {Object} taskReport
* @returns {Object}
*/
function getTaskParentReportActionIDInAssigneeReport(taskReport) {
const assigneeChatReportID = lodashGet(getChatByParticipants(isTaskAssignee(taskReport) ? [taskReport.ownerAccountID] : [taskReport.managerID]), 'reportID');
if (!assigneeChatReportID || assigneeChatReportID === taskReport.parentReportID) {
return {};
}

const clonedParentReportActionID = lodashGet(ReportActionsUtils.getParentReportActionInReport(taskReport.reportID, assigneeChatReportID), 'reportActionID');
if (!clonedParentReportActionID) {
return {};
}

return {
reportID: assigneeChatReportID,
reportActionID: clonedParentReportActionID,
};
}

/**
* Return true if the composer should be hidden
* @param {Object} report
Expand Down Expand Up @@ -2950,6 +2976,7 @@ export {
getMoneyRequestAction,
getBankAccountRoute,
getParentReport,
getTaskParentReportActionIDInAssigneeReport,
getReportPreviewMessage,
shouldHideComposer,
getOriginalReportID,
Expand Down
61 changes: 50 additions & 11 deletions src/libs/actions/Task.js
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,13 @@ function createTaskAndNavigate(parentReportID, title, description, assignee, ass
Navigation.dismissModal(optimisticTaskReport.reportID);
}

function completeTask(taskReportID, taskTitle) {
/**
* Complete a task
* @param {Object} taskReport task report
* @param {String} taskTitle Title of the task
*/
function completeTask(taskReport, taskTitle) {
const taskReportID = taskReport.reportID;
const message = `completed task: ${taskTitle}`;
const completedTaskReportAction = ReportUtils.buildOptimisticTaskReportAction(taskReportID, CONST.REPORT.ACTIONS.TYPE.TASKCOMPLETED, message);

Expand Down Expand Up @@ -260,9 +266,25 @@ function completeTask(taskReportID, taskTitle) {
];

// Update optimistic data for parent report action
const optimisticParentReportData = ReportUtils.getOptimisticDataForParentReportAction(taskReportID, completedTaskReportAction.created, CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD);
if (!_.isEmpty(optimisticParentReportData)) {
optimisticData.push(optimisticParentReportData);
const optimisticDataForParentReportAction = ReportUtils.getOptimisticDataForParentReportAction(taskReportID, completedTaskReportAction.created, CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD);
techievivek marked this conversation as resolved.
Show resolved Hide resolved
if (!_.isEmpty(optimisticDataForParentReportAction)) {
optimisticData.push(optimisticDataForParentReportAction);
}

// Multiple report actions can link to the same child. Both share destination (task parent) and assignee report link to the same report action.
s-alves10 marked this conversation as resolved.
Show resolved Hide resolved
// We need to find and update the other parent report action (in assignee report). More info https://github.com/Expensify/App/issues/23920#issuecomment-1663092717
const assigneeReportAction = ReportUtils.getTaskParentReportActionIDInAssigneeReport(taskReport);
techievivek marked this conversation as resolved.
Show resolved Hide resolved
if (!_.isEmpty(assigneeReportAction)) {
const optimisticDataForClonedParentReportAction = ReportUtils.getOptimisticDataForParentReportAction(
taskReportID,
completedTaskReportAction.created,
CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
assigneeReportAction.reportID,
assigneeReportAction.reportActionID,
);
if (!_.isEmpty(optimisticDataForClonedParentReportAction)) {
optimisticData.push(optimisticDataForClonedParentReportAction);
}
}

API.write(
Expand All @@ -276,11 +298,12 @@ function completeTask(taskReportID, taskTitle) {
}

/**
* Reopens a closed task
* @param {string} taskReportID ReportID of the task
* @param {string} taskTitle Title of the task
* Reopen a closed task
* @param {Object} taskReport task report
* @param {String} taskTitle Title of the task
*/
function reopenTask(taskReportID, taskTitle) {
function reopenTask(taskReport, taskTitle) {
const taskReportID = taskReport.reportID;
const message = `reopened task: ${taskTitle}`;
const reopenedTaskReportAction = ReportUtils.buildOptimisticTaskReportAction(taskReportID, CONST.REPORT.ACTIONS.TYPE.TASKREOPENED, message);

Expand Down Expand Up @@ -336,9 +359,25 @@ function reopenTask(taskReportID, taskTitle) {
];

// Update optimistic data for parent report action
const optimisticParentReportData = ReportUtils.getOptimisticDataForParentReportAction(taskReportID, reopenedTaskReportAction.created, CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD);
if (!_.isEmpty(optimisticParentReportData)) {
optimisticData.push(optimisticParentReportData);
const optimisticDataForParentReportAction = ReportUtils.getOptimisticDataForParentReportAction(taskReportID, reopenedTaskReportAction.created, CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD);
Copy link
Contributor

Choose a reason for hiding this comment

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

Coming from #25548
This PR didn't account for the case where a reportAction has more than one parent (i.e. if you change the asignee of the task), so only the first one was updated
More details in #25548 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

@eVoloshchak I didn't read the whole issue context but isn't the approved PR reintroducing the fixed bug here #23920?

Copy link
Contributor

Choose a reason for hiding this comment

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

@s77rt, we aren't counting completing/opening a task as a reply anymore (recent BE update), so this logic isn't needed
I should have included that actually, so thank you

if (!_.isEmpty(optimisticDataForParentReportAction)) {
optimisticData.push(optimisticDataForParentReportAction);
}

// Multiple report actions can link to the same child. Both share destination (task parent) and assignee report link to the same report action.
// We need to find and update the other parent report action (in assignee report). More info https://github.com/Expensify/App/issues/23920#issuecomment-1663092717
const assigneeReportAction = ReportUtils.getTaskParentReportActionIDInAssigneeReport(taskReport);
if (!_.isEmpty(assigneeReportAction)) {
const optimisticDataForClonedParentReportAction = ReportUtils.getOptimisticDataForParentReportAction(
taskReportID,
reopenedTaskReportAction.created,
CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
assigneeReportAction.reportID,
assigneeReportAction.reportActionID,
);
if (!_.isEmpty(optimisticDataForClonedParentReportAction)) {
optimisticData.push(optimisticDataForClonedParentReportAction);
}
}

API.write(
Expand Down
4 changes: 2 additions & 2 deletions src/pages/home/HeaderView.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ function HeaderView(props) {
threeDotMenuItems.push({
icon: Expensicons.Checkmark,
text: props.translate('task.markAsDone'),
onSelected: () => Task.completeTask(props.report.reportID, title),
onSelected: () => Task.completeTask(props.report, title),
});
}

Expand All @@ -113,7 +113,7 @@ function HeaderView(props) {
threeDotMenuItems.push({
icon: Expensicons.Checkmark,
text: props.translate('task.markAsIncomplete'),
onSelected: () => Task.reopenTask(props.report.reportID, title),
onSelected: () => Task.reopenTask(props.report, title),
});
}

Expand Down
Loading