[HOLD for #27884] [$500] Task - Disabled "Add Reaction" on deleted assignee task notification after a moment

[kbecciv]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Click on the plus sign and add a title, then share it somewhere.
2. Confirm the tasks and complete them.
3. Reopen the tasks.
4. Delete the tasks and hover over the delete notification.
5. Notice that "Add Reaction" is enabled.
6. Refresh the page and observe that "Add Reaction" is now disabled.

Expected Result:

"Add Reaction" should not be disabled when hovering over the deleted assigned tasks notification after refreshing the page. It should either remain disabled after deleting the assigned tasks without waiting a moment, or it should not be disabled at all.

Actual Result:

"Add Reaction" is disabled after a moment when hovering over the deleted assigned tasks notification.

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number:
Reproducible in staging?:
Reproducible in production?:
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Screen.Recording.2023-10-02.at.7.06.10.AM.mov

Screen.Recording.2023-10-01.at.9.05.57.PM.mov

Screen.Recording.2023-10-01.at.9.02.11.PM.mov

Recorder_02102023_064835.mp4

Screen.Recording.2023-10-01.at.8.43.39.PM.mov

Screen.Recording.2023-10-01.at.8.37.51.PM.mov

screen-capture.6.mp4

Expensify/Expensify Issue URL:
Issue reported by: @tewodrosGirmaA
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1695449091829779

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01cd509c6ee69b901c
• Upwork Job ID: 1708866409372258304
• Last Price Increase: 2023-10-16

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01cd509c6ee69b901c

[melvin-bot]

Triggered auto assignment to @stephanieelliott (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @abdulrahuman5196 (External)

[martin12345678900]

Proposal

Please re-state the problem that we are trying to solve in this issue

When adding a task, completing it and then we cancel it, first time we are able to hover and see the reactions.

What is the root cause of the problem

This is due to this code , where we don't check for type of action (in this case it's TASKCANCELLED) - ReportActionsUtils.js

function isMessageDeleted(reportAction) {
    return lodashGet(reportAction, ['message', 0, 'isDeletedParentAction'], false);
}

What changes do you think we should make in order to solve the problem?

We can add an additional check for the action name to see if the it's type is TASKCANCELLED and if it is - we don't display the reactions

function isMessageDeleted(reportAction) {
    return reportAction.actionName === CONST.REPORT.ACTIONS.TYPE.TASKCANCELLED || lodashGet(reportAction, ['message', 0, 'isDeletedParentAction'], false);
}

What alternative solutions did you explore? (Optional)

[melvin-bot]

📣 @martin12345678900! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[martin12345678900]

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/martink80

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[tienifr]

Proposal

Please re-state the problem that we are trying to solve in this issue.

"Add Reaction" is disabled after a moment when hovering over the deleted assigned tasks notification.

What is the root cause of that problem?

There're 2 root causes:

1. The logic to calculate isMessageDeleted here is incomplete. For other types of report action like task cancelling, we need to check the isDeletedParentAction of originalMessage as well, instead of just the message
   When building the optimisticCancelReportAction here, we're also not adding isDeletedParentAction: true, so the optimistic data is different from the data returned from back-end (back-end will return with isDeletedParentAction: true). That's partly why after a few seconds/or when reload the page, the add reaction on mini context menu will hide.

2. We currently have an issue where the originalReportID passed to the context menu is incorrect, the issue is here, leading to the reportAction here will be empty, this will cause wrong calculation of the context menu once reloading the page with the task edit, task cancelled action.

What changes do you think we should make in order to solve the problem?

1. We need to update the condition here to

return lodashGet(reportAction, ['message', 0, 'isDeletedParentAction'], false) || lodashGet(reportAction, ['originalMessage', 'isDeletedParentAction'], false);

and add isDeletedParentAction: true for the optimistic data here
so the online and offline behaviors will be consistent.
2. We need to fix the same as in this proposal, or HOLD this issue until that one is fixed.

What alternative solutions did you explore? (Optional)

In 1, if we want to additionally checks originalMessage.isDeletedParentAction only if the report action is task report actions, we can add the condition for that. Or can compare the actionName with TASKCANCELLED, but I prefer the originalMessage.isDeletedParentAction check because it's consistent with our existing message[0].isDeletedParentAction check, and also will work for any deleted report action with originalMessage, not just work for a report action of a specific type.

[abdulrahuman5196]

On @martin12345678900 's proposal here #28621 (comment)
I am not sure if the root cause is correct. Currently for other task actions we are not showing reaction by default, but why its working for others and specifically not working for this cancel action is not mentioned.

And I am not aligned on the solution to add action name check to the isMessageDeleted function.

[abdulrahuman5196]

On @tienifr 's proposal here #28621 (comment)
I am not sure if root cause 2 is relavant to this particular issue. But I will come back to that.

On the root cause 1. the below is an example reportAction from server for the cancel action after refreshing.

Even in the reportAction, the isDeletedParentAction flag is not present inside the message key which is used in the isMessageDeleted function. But still its working after refresh. Not sure If I had missed something in the proposal.

Could you kindly provide information on why its working(reactions not shown in context menu) on this case as well?

{
  "actionName": "TASKCANCELLED",
  "actorAccountID": 14321923,
  "automatic": false,
  "avatar": "https://d1wpcgnaa73g0y.cloudfront.net/8652017f3bd218b57b61db0c65a947339c74107c.png",
  "isAttachment": false,
  "originalMessage": {
    "taskReportID": 6690522316780141,
    "type": "TASKCANCELLED",
    "text": "deleted task: tititit",
    "html": "deleted task: tititit",
    "lastModified": "2023-10-03 15:33:42.124",
    "title": "tititit",
    "isDeletedParentAction": true,
    "reactions": [
      {
        "emoji": "fire",
        "users": [
          {
            "accountID": 14321923,
            "skinTone": -1
          }
        ]
      }
    ]
  },
  "message": [
    {
      "type": "TEXT",
      "style": "normal",
      "text": "deleted task: tititit"
    }
  ],
  "person": [
    {
      "type": "TEXT",
      "style": "strong",
      "text": "Abdul rahuman Abdul rahuman"
    }
  ],
  "reportActionID": "492571325956984651",
  "shouldShow": true,
  "created": "2023-10-03 15:33:38.283",
  "isFirstItem": false,
  "pendingAction": null,
  "timestamp": 1696347218,
  "reportActionTimestamp": 1696347218283,
  "previousReportActionID": "2571787378646686224",
  "lastModified": "2023-10-03 15:33:42.124",
  "whisperedToAccountIDs": [],
  "sequenceNumber": null,
  "childReportID": 6690522316780141,
  "childType": "task",
  "childReportName": "tititit",
  "childManagerAccountID": 14244652
}

[dukenv0307]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Emojis, emoji picker icon, and edit icon are disappeared when hovering over notification from completed task

What is the root cause of that problem?

The task system message is stored in task report but it has childReportID filed that is the reportID of task report. So getOriginalReportID returns parentReportID for this case and then all actions of task system message are failed.

App/src/libs/ReportUtils.js

Lines 3501 to 3503 in 81a6a52

	function getOriginalReportID(reportID, reportAction) {
	return isThreadFirstChat(reportAction, reportID) ? lodashGet(allReports, [`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, 'parentReportID']) : reportID;
	}

What changes do you think we should make in order to solve the problem?

Actually, #24644 is closed because we don't have priority to fix from BE.

If we want to fix this issue from FE side, we can update getOriginalReportID function to make sure it returns the correct result for task system message by getting the report action with reportID and reportActionID and check it's empty or not

function getOriginalReportID(reportID, reportAction) {
    const currentReportAction = ReportActionsUtils.getReportAction(reportID, reportAction.reportActionID);
    return isThreadFirstChat(reportAction, reportID) && _.isEmpty(currentReportAction) ? lodashGet(allReports, [`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, 'parentReportID']) : reportID;
}

App/src/libs/ReportUtils.js

Lines 3501 to 3503 in 81a6a52

	function getOriginalReportID(reportID, reportAction) {
	return isThreadFirstChat(reportAction, reportID) ? lodashGet(allReports, [`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, 'parentReportID']) : reportID;
	}

What alternative solutions did you explore? (Optional)

We can check if the report action is task system action we will return reportID.

[tienifr]

Even in the reportAction, the isDeletedParentAction flag is not present inside the message key which is used in the isMessageDeleted function. But still its working after refresh. Not sure If I had missed something in the proposal.

@abdulrahuman5196 Please see the 2 part of root cause and solution in my proposal.

When you refresh the page, if you try to log the reportAction here to the console, you'll see it's empty. So it will not show the add reaction as per this condition (empty reportAction does not have the message key).

The reportAction is returned correctly from the back-end but we're getting it incorrectly, so it's empty there.

[esh-g]

Proposal

Please re-state the problem

Emoji actions are enabled for some time for report actions indicating closing/re-opening of a task.

What is the root cause of this problem?

The root cause if that the emojis are only shown when the reportAction has a message in it. And the message is only there when the reportAction is present in props.reportActions in BaseReportActionContextMenu.js

App/src/pages/home/report/ContextMenu/BaseReportActionContextMenu.js

Lines 57 to 62 in cf9ad20

	const reportAction = useMemo(() => {
	if (_.isEmpty(props.reportActions) || props.reportActionID === '0') {
	return {};
	}
	return props.reportActions[props.reportActionID] || {};
	}, [props.reportActions, props.reportActionID]);

Now this props.reportActions comes from the originalReportID passed to the context menu here:

App/src/pages/home/report/ContextMenu/BaseReportActionContextMenu.js

Line 188 in cf9ad20

key: ({originalReportID}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${originalReportID}`,

This originalReportID comes from the ReportUtils.getOriginalReportID function which in-turn calls the ReportUtils.isThreadFirstChat function.

App/src/libs/ReportUtils.js

Lines 690 to 692 in cf9ad20

	function isThreadFirstChat(reportAction, reportID) {
	return !_.isUndefined(reportAction.childReportID) && reportAction.childReportID.toString() === reportID;
	}

Basically this function checks if the childReportID is equal to currentReportID.

Now if we see the task completed action created optimistically, we see that there is no childReportID, but after updating from backend, the childReportID appears and this task completed action becomes a child of the report where the task was first shared. Which means that the originalReportID becomes the shareDestination report of the task, which in-turn does not have this report action. Thus, props.reportActions passed to BaseReportActionContextMenu doesn't include this task and it does not have message, thus the emojis stop showing.

What changes should be done to fix this?

We should return the childReportID while building a taskReportAction optimistically here:

App/src/libs/ReportUtils.js

Lines 2436 to 2445 in cf9ad20

	function buildOptimisticTaskReportAction(taskReportID, actionName, message = '') {
	const originalMessage = {
	taskReportID,
	type: actionName,
	text: message,
	};
	return {
	actionName,
	actorAccountID: currentUserAccountID,

like this:

childReportID: taskReportID

[stephanieelliott]

Hey @abdulrahuman5196 can you please review these proposals when you get a chance?

[abdulrahuman5196]

Yes. Will do it today. On my checklist for today items.

[esh-g]

Actually I think this will get fixed in this issue: #27884
Because this will remove the childReportID from the backend and we will be allowed to add the reactions then

[melvin-bot]

@stephanieelliott, @abdulrahuman5196 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[abdulrahuman5196]

TAL now

[abdulrahuman5196]

On @dukenv0307 's proposal here #28621 (comment). I think its the opposite we want here, like We don't want to show reactions on deleted task notification similar to other task notification on the thread.

Could you confirm this @stephanieelliott ?

[stephanieelliott]

Yep, agreed @abdulrahuman5196

[abdulrahuman5196]

Reviewing again

[abdulrahuman5196]

@stephanieelliott
I agree on the callouts by @tienifr and @esh-g . We should re-test this issue once #27884 is fixed. Can we put this issue on hold for the same?

[melvin-bot]

@stephanieelliott @abdulrahuman5196 this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[stephanieelliott]

Put on Hold and adjusted labels!

[stephanieelliott]

#27884 was merged, looks like it fixed this issue -- retested and it's not happening anymore!