[HOLD for payment 2023-08-17] [$1000] Replies count doesn't update on a task shared in group.

[kavimuru]

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. Create a task
2. Select a assignee
3. Select a group to share
4. Go to the group and update the task using checkbox (replies count updates)
5. Go to assignee report and update the task using checkbox (replies count doesn't updates)

Expected Result:

The replies count should update in both the group and assignee report.

Actual Result:

The replies count updates in the group but not in assignee report.

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

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: 1.3.47-2
Reproducible in staging?: y
Reproducible in production?: y
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

task_replies_bug_demo.mp4

Recording.1409.mp4

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~013bbb12e93147362e
• Upwork Job ID: 1686236770866049024
• Last Price Increase: 2023-08-01
• Automatic offers:
• s77rt | Reviewer | 25923313
• s-alves10 | Contributor | 25923314

[melvin-bot]

Triggered auto assignment to @JmillsExpensify (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

[yh-0218]

Proposal

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

Replies count doesn't update on a task shared in group.

What is the root cause of that problem?

When we complete or reopen task, we are calling completeTask and reopenTask of Task.js
In these function, we are increasing childVisibleActionCount of action.
In the reopen case:

App/src/libs/actions/Task.js

Line 339 in 3324ea7

const optimisticParentReportData = ReportUtils.getOptimisticDataForParentReportAction(taskReportID, reopenedTaskReportAction.created, CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD);

App/src/libs/ReportUtils.js

Lines 1528 to 1543 in 3324ea7

	const getOptimisticDataForParentReportAction = (reportID, lastVisibleActionCreated, type) => {
	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),
	},
	};
	}
	}
	return {};
	};

To update childVisibleActionCount of action, we got parentReportAction from task.
This is only action of shared channel.
so only shared channel reply is increased and direct channel reply is not increased(only updated after refresh)

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

We need to childVisibleActionCount of 2 actions(shared channels action and direct channel action).
In my test case, actions ids are 6032435466773608246 and 6281777622256522668.
We can send this as parameter to update.

What alternative solutions did you explore? (Optional)

We can get origin 2 actions on update function.

[JmillsExpensify]

Hmmm, I agree that minimally we should be treating the history reportAction the same for the room and DM cases. I'll triage.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~013bbb12e93147362e

[melvin-bot]

Current assignee @JmillsExpensify is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[s-alves10]

Proposal

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

Replies count doesn't update on a task shared in group when we update the task in assignee report

What is the root cause of that problem?

When we create a task, we set the parentReportID of the newly created task report to the shareDestination reportID

App/src/pages/tasks/NewTaskPage.js

Line 99 in cb8aac1

setParentReport(lodashGet(props.reports, `report_${props.task.shareDestination}`, {}));

And we create the new comment in shareDestination report and set the parentReportActionID of the task to the reportActionID of the new comment

App/src/libs/actions/Task.js

Lines 69 to 70 in cb8aac1

	const optimisticAddCommentReport = ReportUtils.buildOptimisticTaskCommentReportAction(taskReportID, title, assignee, assigneeAccountID, `Created a task: ${title}`, parentReportID);
	optimisticTaskReport.parentReportActionID = optimisticAddCommentReport.reportAction.reportActionID;

When we update(reopen or complete) a task using the checkbox, reopenTask/completeTask function in Task.js are called.

Let's take a look at the completeTask function here
After we set optimistic data for the task report and actions, we create the optimistic data for parent report action

App/src/libs/actions/Task.js

Line 263 in cb8aac1

const optimisticParentReportData = ReportUtils.getOptimisticDataForParentReportAction(taskReportID, completedTaskReportAction.created, CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD);

If we update the task in shareDestination report, all works fine because parentReportAction is updated as well here.
But this is not the case when we update the task in assignee report. The current action of the assignee report isn't updated.

This is the root cause

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

We should update the assignee parent action when we update the task in assignee report. It would be better if we can update two parent actions(one in shareDestination and one in assignee, of course, when they are different)

Solution 1

1. Pass the additional parameters to the reopenTask/completeTask functions: reportID, reportActionID in TaskPreview

If the current action is assignee task action,

        const isAssigneeTaskAction = props.taskReport.parentReportActionID !== props.action.reportActionID;
        const reportID = isAssigneeTaskAction ? props.reportID : undefined;
        const reportActionID = isAssigneeTaskAction ? props.action.reportActionID : undefined;

        onPress={() => {
            if (isTaskCompleted) {
                Task.reopenTask(props.taskReportID, taskTitle, reportID, reportActionID);
            } else {
                Task.completeTask(props.taskReportID, taskTitle, reportID, reportActionID);
            }
        }}

Add reportID props to the TaskPreview component in ReportActionItem

App/src/pages/home/report/ReportActionItem.js

Line 287 in cb8aac1

<TaskPreview

        reportID={props.report.reportID}

2. Add the new parameters reportID and reportActionID

    function completeTask(taskReportID, taskTitle, reportID, reportActionID)
    ...

    function reopenTask(taskReportID, taskTitle, reportID, reportActionID)

If reportID or reportActionID are undefined, do nothing

If not, create the optimistic data same as optimisticParentReportData created here except reportID and reportActionID

    const optimisticParentReportData = ReportUtils.getOptimisticDataForParentReportAction(taskReportID, completedTaskReportAction.created, CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD);
    if (!_.isEmpty(optimisticParentReportData)) {
        optimisticData.push(optimisticParentReportData);
        if (reportID && reportActionID) {
            const optimisticDataForAssigneeReportAction = {
                onyxMethod: Onyx.METHOD.MERGE,
                key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`,
                value: {
                    [reportActionID]: _.values(optimisticParentReportData.value)[0],
                },
            }
            optimisticData.push(optimisticDataForAssigneeReportAction);
        }
    }

Solution 2

We can always create the optmistic data for two parent report actions(in assignee and shareDestination report) as follows

1. Add the following line to the buildOptimisticTaskCommentReportAction

    reportAction.reportAction.participantAccountIDs = [taskAssigneeAccountID];

The backend returns this data. This is for offline mode

2. Pass taskReport instead of taskReportID to reopenTask/completeTask in TaskPreview and TaskView

3. Add the utility function getParentReportActionForTask in REportActionsUtils.js

function getParentReportActionForTask(taskReportID, reportID) {
    return _.find(
        allReportActions[reportID],
        (reportAction) => reportAction && `${reportAction.childReportID}` === `${taskReportID}` && reportAction.actionName === CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT,
    );
}

This is the function to find the reportAction with childReportID = taskReportID in reportActions of report(reportID)

4. Update the reopenTask/completeTask function as follows

• Rename the first argument from taskReportID to taskReport and add the first line

    const taskReportID = taskReport.reportID;

• We can get reportID and reportActionID of the parent action of the task in assignee report.

    const assigneeChatReportID = lodashGet(ReportUtils.getChatByParticipants(taskReport.participantAccountIDs), 'reportID');
    const parentReportAction = ReportActionsUtils.getParentReportActionForTask(taskReportID, assigneeChatReportID);

Now that we get reportID and reportActionID, we can create optmisticData from them as solution 1.

The entire code would look like

    // 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 assigneeChatReportID = lodashGet(ReportUtils.getChatByParticipants(taskReport.participantAccountIDs), 'reportID');
        if (assigneeChatReportID && `${assigneeChatReportID}` !== `${taskReport.parentReportID}`) {
            const parentReportAction = ReportActionsUtils.getParentReportActionForTask(taskReportID, assigneeChatReportID);
            if (parentReportAction) {
                optimisticData.push({
                    onyxMethod: Onyx.METHOD.MERGE,
                    key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${assigneeChatReportID}`,
                    value: {
                        [parentReportAction.reportActionID]: _.values(optimisticParentReportData.value)[0],
                    },
                });
                failureData.push({
                    onyxMethod: Onyx.METHOD.MERGE,
                    key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${assigneeChatReportID}`,
                    value: {
                        [parentReportAction.reportActionID]: {
                            errors: ErrorUtils.getMicroSecondOnyxError('task.messages.error'),
                        },
                    },
                });
            }
        }
    }

These solutions works well. I prefer the 2nd solution

Result

23920.mp4

What alternative solutions did you explore? (Optional)

We can consider the multiple parentReportID and parentReportActionID. In this case, I think the backend should be updated as well

[s77rt]

@yh-0218 Thanks for the proposal. Your RCA makes sense and the solution as well but it's not well explained. Can you please explain how we will get the 2 report action ids (share report action id is already stored in parentReportActionID but what about the assignee?)

[s77rt]

@s-alves10 Thanks for the proposal. Your RCA makes sense. I don't think solution 1 will work correctly as if we add a reply in the share report it should be reflected in the assignee report as well (and not just the vice-versa way). Solution 2 seems like it will do the job but I'm not sure about the idea of having to iterate through the report actions to find the correct parent.

I think there may be a bigger problem, we have two report actions claiming to be the parent of the same report.

• ShareReport have a reportAction that links to TaskReport as it's child
• AssigneeReport also have a reportAction that links to the same child ❗ ❓
• The parent of TaskReport is ShareReport.

Or:

• reportActionA: My child is report123
• reportActionB: My child is report123
• report123: My parent is ReportActionA

[s77rt]

Discussing in Slack https://expensify.slack.com/archives/C01GTK53T8Q/p1690973855809149

[s77rt]

From the slack discussion the highlighted problem seems to be a known limitation. Searching for the "fake" parent actions is the way to go.

1. The real parent action is stored in task report
2. The "fake" parent action (currently only one) is somewhere in the assignee report we will have to filter the report actions based on childReportID - We will also need to find the assignee report

@s-alves10 s 2nd solution for doing the above looks good.

🎀 👀 🎀 C+ reviewed
Link to proposal

[melvin-bot]

Triggered auto assignment to @techievivek, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[melvin-bot]

📣 @s77rt 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @s-alves10 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[melvin-bot]

📣 @Krishna2323 We're missing your Upwork ID to automatically send you an offer for the Reporter role.
Once you apply to the Upwork job, your Upwork ID will be stored and you will be automatically hired for future jobs!

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.52-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

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

If no regressions arise, payment will be issued on 2023-08-17. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

• @s77rt requires payment offer (Reviewer)
• @s-alves10 requires payment offer (Contributor)

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@s77rt] The PR that introduced the bug has been identified. Link to the PR:
• [@s77rt] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@s77rt] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@s77rt] Determine if we should create a regression test for this bug.
• [@s77rt] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@JmillsExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[s77rt]

• The PR that introduced the bug has been identified: This existed since day one Complete Task Front End  #18806 and Move task reportActions onto the task report #20014
• The offending PR has been commented on: Complete Task Front End  #18806 (comment) and Move task reportActions onto the task report #20014 (comment)
• A discussion in #expensify-bugs has been started: Not a regression, more like a missed unimplemented case
• Determine if we should create a regression test for this bug: No

[ShogunFire]

Hello, do you think that could be a regression ? #24532

[s77rt]

@ShogunFire I don't think so, that issue seems too different and possibility internal (if we have missing pusher events).

[s-alves10]

@s77rt @ShogunFire

I don't think so, too. But I think that issue can be solved similarly. I'm submitting a proposal

[s-alves10]

@s77rt

You're right. It looks like we have some missing pusher events

[Krishna2323]

@techievivek @JmillsExpensify, can you pls send a offer on Upwork for reporter role, I have applied a week ago but haven't got the offer yet.

[techievivek]

Hello, do you think that could be a regression ? #24532

Yeah, that looks like a backend bug and not something related to changes that were done here.

[techievivek]

can you pls send a offer on Upwork for reporter role, I have applied a week ago but haven't got the offer yet.

@JmillsExpensify Can you take a look into it? Thanks.

[techievivek]

Gentle bump @JmillsExpensify ^

[s-alves10]

@JmillsExpensify

Will you let me know when I can get paid?

[Krishna2323]

@JmillsExpensify, can you pls provide update for payments here. Thanks

[techievivek]

Not overdue, I have just DMed Jason to finish the payment here. Sorry for the delay.

[JmillsExpensify]

Sorry ya'll, I'm not coming back to this issue for payments/summary.

[JmillsExpensify]

Payment summary:

• Issue reporter: @Krishna2323 $250 (pre-September 1)
• Contributor: @s-alves10 $1,500 (incl. urgency bonus)
• Contributor+: @s77rt $1,500 (incl. urgency bonus)

@Krishna2323 I believe you applied to the Upwork but for the wrong amount. @s-alves10 and @s77rt offers just sent.

[Krishna2323]

@JmillsExpensify ohh sorry, how can I apply now?
I updated the job terms.

[JmillsExpensify]

Just hired you.

[Krishna2323]

@JmillsExpensify, accepted.

[JmillsExpensify]

Paid

[Krishna2323]

Thanks ✌🏻

[s-alves10]

@JmillsExpensify

Accepted

[s77rt]

@JmillsExpensify Accepted! Thanks!

[JmillsExpensify]

Everyone should be paid out at this point! Closing.