[HOLD for payment 2023-07-10] [$1000] Assign task show error when creating within a report

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Open chat with any user
2. Click + in compose
3. Assign task
4. Input the title
5. Next => Observe the error appearing: "There was an error assigning this task, please try another assignee."

Expected Result:

Error message doesn't appear
and there should not be an empty avatar in the task assignee view

Actual Result:

Observe the error appearing: "There was an error assigning this task, please try another assignee."

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.31-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

Screen.Recording.2023-06-21.at.11.10.48.mov

Expensify/Expensify Issue URL:
Issue reported by: @Victor-Nyagudi
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1687320876332399

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0152a7fadbf2438272
• Upwork Job ID: 1672104067929206784
• Last Price Increase: 2023-06-23

[melvin-bot]

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

[namhihi237]

Proposal

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

The error message doesn't appear when at new open modal up

What is the root cause of that problem?

When creating a task from the report, it will automatically assign based on the report.

App/src/pages/tasks/NewTaskPage.js

Lines 75 to 93 in 6183609

	useEffect(() => {
	setErrorMessage('');
	// If we have an assignee, we want to set the assignee data
	// If there's an issue with the assignee chosen, we want to notify the user
	if (props.task.assignee) {
	const assigneeDetails = lodashGet(OptionsListUtils.getPersonalDetailsForAccountIDs([props.task.assigneeAccountID], props.personalDetails), props.task.assigneeAccountID);
	if (!assigneeDetails) {
	return setErrorMessage(props.translate('newTaskPage.assigneeError'));
	}
	const displayDetails = TaskUtils.getAssignee(assigneeDetails);
	setAssignee(displayDetails);
	}
	// If we don't have an assignee and we are creating a task from a report
	// this allows us to auto assign a participant of the report.
	if (!props.task.assignee && props.task.parentReportID) {
	TaskUtils.setAssigneeValueWithParentReportID(props.task.parentReportID);
	}

App/src/libs/actions/Task.js

Lines 456 to 467 in 6183609

	function setAssigneeValueWithParentReportID(reportID) {
	const report = ReportUtils.getReport(reportID);
	const isDefault = !(ReportUtils.isChatRoom(report) || ReportUtils.isPolicyExpenseChat(report));
	const participants = lodashGet(report, 'participants', []);
	const hasMultipleParticipants = participants.length > 1;
	if (!isDefault || hasMultipleParticipants || report.parentReportID) {
	return;
	}
	Onyx.merge(ONYXKEYS.TASK, {assignee: participants[0]});
	}

The problem lies in this function, we have only set assignee, so when running on the if (props.task.assignee) condition, the assigneeAccountID has not been set. so assigneeDetails = null => show error

const assigneeDetails = lodashGet(OptionsListUtils.getPersonalDetailsForAccountIDs([props.task.assigneeAccountID], props.personalDetails), props.task.assigneeAccountID);

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

Create a task within a report, It will default share in the room, but will not set an assignee by default => so we will remove code here:

App/src/pages/tasks/NewTaskPage.js

Lines 89 to 94 in 6183609

	// If we don't have an assignee and we are creating a task from a report
	// this allows us to auto assign a participant of the report.
	if (!props.task.assignee && props.task.parentReportID) {
	TaskUtils.setAssigneeValueWithParentReportID(props.task.parentReportID);
	}

Result:

Screen.Recording.2023-06-27.at.01.01.43.mov

What alternative solutions did you explore? (Optional)

N/A

[sakluger]

Reproduced, cc @thienlnam in case you want to work on this one.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0152a7fadbf2438272

[melvin-bot]

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

[melvin-bot]

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

[thienlnam]

I actually can't seem to reproduce this on main - it might have been resolved already

[namhihi237]

Hi @thienlnam ,I can reproduce from my end, last commit 2c5e5aed5305b87a1bddc2b0c049a6f0ad0e322d

Screen.Recording.2023-06-23.at.23.59.48.mov

[thienlnam]

Ah I see, these reproduction steps are inaccurate

It only happens when you try to assign a task within a report

[thesahindia]

@namhihi237's proposal looks good to me!

🎀👀🎀 C+ reviewed

cc: @thienlnam

[thienlnam]

Hmm, when we create a task within a report - we don't want to automatically set an assignee. It should either be

Create a task

• Select an assignee, and by default it will share in the DM between you two

or

Create a task within a report

• It will default share in the room, but will not set an assignee by default

[namhihi237]

Hi @thienlnam, can you check it's expected with you

Screen.Recording.2023-06-27.at.00.54.56.mov

[namhihi237]

@thienlnam @thesahindia I have updated the proposal, can you please take a look

[thienlnam]

Can you also show the video verifying when the task is created with no assignee it doesn't show an empty avatar?

[namhihi237]

Yes, We should be updated in getDisplayNameForParticipant to check if no assign.

Screen.Recording.2023-06-27.at.04.11.49.mov

[thienlnam]

Cool, looks good to me!

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

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

• fix: remove assign and make empty avatar header #21659

If no regressions arise, payment will be issued on 2023-07-10. 🎊

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 - @Victor-Nyagudi
• Contributor that fixed the issue - @namhihi237
• Contributor+ that helped on the issue and/or PR - @thesahindia

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:

• [@thesahindia] The PR that introduced the bug has been identified. Link to the PR:
• [@thesahindia] 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:
• [@thesahindia] 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:
• [@thesahindia] Determine if we should create a regression test for this bug.
• [@thesahindia] 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.
• [@sakluger] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[sakluger]

@thesahindia could you please complete the BZ checklist when you have a chance? That way I can pay out on Monday. Thanks!

[thienlnam]

Sorry, I'm not following. Do we need to change anything here on this issue?

No, we're good to go - I added my thoughts on that original thread but what we ended up doing here was correct

[sakluger]

Paid @Victor-Nyagudi and @namhihi237 (with efficiency bonus). Just waiting on the BZ checklist, then I can pay @thesahindia and close out the issue!

[thesahindia]

@sakluger, no need to pay me on upwork. I will request money using new dot.

I will fill the checklist in the morning. It's EOD for me.

[sakluger]

@thesahindia got it - I closed the contract in Upwork. I'll check back in tomorrow for the BZ checklist 👍

[sakluger]

Hey @thesahindia checking in again since I'd love to close out this issue. Thanks!

[thesahindia]

• [@thesahindia] The PR that introduced the bug has been identified. Link to the PR:

#17992

• [@thesahindia] 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:

#17992 (comment)

• [@thesahindia] 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:

Don't think there's any change needed.

• [@thesahindia] Determine if we should create a regression test for this bug.

We don't need to add a test case since it's a simple flow.

• [@thesahindia] 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.

N/A

[thesahindia]

@sakluger, you can close this now.

[sakluger]

Thank you!

[JmillsExpensify]

@sakluger Do you mind providing a payment summary on this issue so that I can pay @thesahindia?

[melvin-bot]

@sakluger, @thienlnam, @namhihi237, @thesahindia Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[sakluger]

Sure thing @JmillsExpensify.

Summarizing payouts for this issue:

Bug reporter: @Victor-Nyagudi $250 (hired and paid via Upwork)
Contributor: @namhihi237 $1500 (hired and paid via Upwork)
Contributor+: @thesahindia $1500 (payable via Manual Request)

Above payments include efficiency bonus 🎉
Upwork job: https://www.upwork.com/jobs/~0152a7fadbf2438272

[sakluger]

@JmillsExpensify are we good to close this one out again, or do you want to leave it open until you've paid it out?

[sakluger]

Okay I'm going to close this one again.

[JmillsExpensify]

$1,500 payment approved for @thesahindia based on BZ summary.