Skip to content

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

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

Merged
merged 18 commits into from
Aug 8, 2023
Merged

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

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
313d2e5
fix: update parent action for assignee report as well
sangar-1028 Aug 3, 2023
9728aa2
fix: optimistic data 'participantAccountIDs' in task report, not in r…
sangar-1028 Aug 3, 2023
574c144
fix: console warning
sangar-1028 Aug 3, 2023
24a3870
fix: conflicts
sangar-1028 Aug 3, 2023
1e380ae
fix: add comments, remove failureData for assignee report action
sangar-1028 Aug 3, 2023
abff057
fix: prettier
sangar-1028 Aug 3, 2023
a5eac40
fix: use of updateOptimisticParentReportAction for both assignee and …
sangar-1028 Aug 4, 2023
688217f
fix: prettier
sangar-1028 Aug 4, 2023
d4dd964
fix: rnaming, comments
sangar-1028 Aug 5, 2023
fbed766
fix: comments, string constructor
sangar-1028 Aug 5, 2023
6a977fd
fix: prettier
sangar-1028 Aug 5, 2023
fafcfa9
fix: update getOptimisticDataForParentReportAction
sangar-1028 Aug 7, 2023
99018a5
fix: lint and prettier
sangar-1028 Aug 7, 2023
c474825
fix: refactor completeTask/reopenTask
sangar-1028 Aug 7, 2023
e87bed8
fix: run prettier
sangar-1028 Aug 7, 2023
a5f4dfb
fix: reopen/complete task in HeaderView
sangar-1028 Aug 7, 2023
2e3f28e
fix: comments and function name
sangar-1028 Aug 7, 2023
5152ef3
fix: uncovered case for report with task owner when log in with assignee
sangar-1028 Aug 7, 2023
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
View file
Open in desktop
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);
} 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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,17 @@ function getIOUReportIDFromReportActionPreview(reportAction) {
return lodashGet(reportAction, 'originalMessage.linkedReportID', '');
}

/**
* Find the reportAction whose childReportID is taskReportID in reportActions with reportID
*
* @param {String} taskReportID
* @param {String} reportID
* @returns {Object}
*/
function getParentReportActionForTask(taskReportID, reportID) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This function is not exclusive to tasks. Can we rename it to something else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed to getParentReportActionInReport

return _.find(allReportActions[reportID], (reportAction) => reportAction && `${reportAction.childReportID}` === `${taskReportID}`);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@s-alves10 Let's write the code as if they are both strings. Parent report ids being returned as a number is a bug https://expensify.slack.com/archives/C01GTK53T8Q/p1691167905181059?thread_ts=1691167905.181059&cid=C01GTK53T8Q

@s77rt

Got it. But if we don't convert them to string, comparison would return false and can't find the parent action in assignee report. childReportID is a number as well.

I think we need to convert them to string unless the backend is fixed. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed. We may need to wait for an outcome from that slack thread. But for now let's write the right code.

}

function isCreatedTaskReportAction(reportAction) {
return reportAction.actionName === CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT && _.has(reportAction.originalMessage, 'taskReportID');
}
Expand Down Expand Up @@ -593,4 +604,5 @@ export {
isWhisperAction,
isPendingRemove,
getReportAction,
getParentReportActionForTask,
};
1 change: 1 addition & 0 deletions src/libs/ReportUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2183,6 +2183,7 @@ function buildOptimisticTaskReport(ownerAccountID, assigneeAccountID = 0, parent
description,
ownerAccountID,
managerID: assigneeAccountID,
participantAccountIDs: [assigneeAccountID],
type: CONST.REPORT.TYPE.TASK,
parentReportID,
stateNum: CONST.REPORT.STATE_NUM.OPEN,
Expand Down
85 changes: 74 additions & 11 deletions src/libs/actions/Task.js
View file
Open in desktop
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,37 @@ 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 parentReportAction = ReportActionsUtils.getParentReportAction(taskReport);
if (parentReportAction && parentReportAction.reportActionID) {
const optimisticParentReportActionData = ReportUtils.updateOptimisticParentReportAction(
parentReportAction,
completedTaskReportAction.created,
CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
);
optimisticData.push({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${taskReport.parentReportID}`,
value: {
[parentReportAction.reportActionID]: optimisticParentReportActionData,
},
});

// 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 assigneeChatReportID = lodashGet(ReportUtils.getChatByParticipants(taskReport.participantAccountIDs), 'reportID');
// We don't need to create optmistic report action for assignee report if assignee and shareDestination are the same
if (assigneeChatReportID && assigneeChatReportID !== taskReport.parentReportID) {
const clonedParentReportAction = ReportActionsUtils.getParentReportActionForTask(taskReportID, assigneeChatReportID);
if (clonedParentReportAction && clonedParentReportAction.reportActionID) {
optimisticData.push({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${assigneeChatReportID}`,
value: {
[clonedParentReportAction.reportActionID]: optimisticParentReportActionData,
},
});
}
}
}

API.write(
Expand All @@ -276,11 +310,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 +371,37 @@ 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 parentReportAction = ReportActionsUtils.getParentReportAction(taskReport);
if (parentReportAction && parentReportAction.reportActionID) {
const optimisticParentReportActionData = ReportUtils.updateOptimisticParentReportAction(
parentReportAction,
reopenedTaskReportAction.created,
CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
);
optimisticData.push({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${taskReport.parentReportID}`,
value: {
[parentReportAction.reportActionID]: optimisticParentReportActionData,
},
});

// 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 assigneeChatReportID = lodashGet(ReportUtils.getChatByParticipants(taskReport.participantAccountIDs), 'reportID');
// We don't need to create optmistic report action for assignee report if assignee and shareDestination are the same
if (assigneeChatReportID && assigneeChatReportID !== taskReport.parentReportID) {
const clonedParentReportAction = ReportActionsUtils.getParentReportActionForTask(taskReportID, assigneeChatReportID);
if (clonedParentReportAction && clonedParentReportAction.reportActionID) {
optimisticData.push({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${assigneeChatReportID}`,
value: {
[clonedParentReportAction.reportActionID]: optimisticParentReportActionData,
},
});
}
}
}

API.write(
Expand Down