[HELD ON #19298] \[$1000] Unchecking done assignment is not reflected on check-mark

[mvtglobally]

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. Login to NewDot
2. Create a couple of tasks
3. Mark as complete
4. Sign out
5. Sign in
6. Go to parent chat with tasks
7. Click checkbox to Uncheck done assignment

Expected Result:

Checkbox should be unmarked

Actual Result:

Checkbox stays marked like assignment is still done (however in fact it is reverted to pending)

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: v1.3.15-5
Reproducible in staging?: Y
Reproducible in production?: N
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
Found when executing #18806

Bug6058987_video_49.mp4

Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal team
Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01fc4b3aaa9fc84298
• Upwork Job ID: 1659567080360730624
• Last Price Increase: 2023-05-19

[melvin-bot]

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

[kevinksullivan]

OOO today for several days, so I am going to reassign.

[melvin-bot]

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

[zanyrenney]

Hey @kevinksullivan new process for managing issues when you go OOO is to keep yourself assigned. Post in growth here, as well as the SO here and the thread in #mentors here.

Basically the person you assign to can oversee/work on the issue but when you return if there is still actions to take, you should be also working on the issue :)

[zanyrenney]

Yep, reproducablereproducible:

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[getusha]

I think this is a bug from the backend

[zanyrenney]

@marcochavezf can you confirm if this is a backend bug and i'll change the label please to internal ? Thanks!

[zanyrenney]

bump @marcochavezf curious for your thoughts before amending the labels here.

[jjcoffee]

Proposal

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

Task checkboxes should work normally after signing out.

What is the root cause of that problem?

When we log back in, the linked taskReport doesn't load (doesn't appear in LHN and doesn't show up in the TaskPreview's props.taskReport and isn't in Onyx). It looks like this will be fixed in #19090.

The reportAction itself does have the correct state of the task, however, so I'd expect the checkbox to still work. TaskPreview is getting the task's completion state here:

App/src/components/ReportActionItem/TaskPreview.js

Lines 56 to 58 in c917c44

	const isTaskCompleted =
	(props.taskReport.stateNum === CONST.REPORT.STATE_NUM.SUBMITTED && props.taskReport.statusNum === CONST.REPORT.STATUS.APPROVED) ||
	(props.action.childStateNum === CONST.REPORT.STATE_NUM.SUBMITTED && props.action.childStatusNum === CONST.REPORT.STATUS.APPROVED);

I think the intention here is to get the task's state from either the taskReport itself, or from the reportAction, however what we're actually doing is saying the task is complete if either source reports that the task is complete.

The problem with this is that these sources aren't always in sync (in this case it's more to do with the BE issue, but the same could occur if going offline).

In the case of our issue, when you log back in and try and reopen the task the linked taskReport correctly gets updated to a reopened state (so the first part of the isTaskCompleted expression is false), but reportAction doesn't update its childStateNum or childStatusNum at all, so we have:

• taskReport - OPEN
• reportAction - COMPLETED

Which means isTaskCompleted always returns true, and we can never "untick" the checkbox.

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

Firstly I think we should treat the taskReport as a ground source of truth (where it is available), as it's what the API actually updates in its responses. So we'd want to update this line:

App/src/components/ReportActionItem/TaskPreview.js

Lines 56 to 58 in c917c44

	const isTaskCompleted =
	(props.taskReport.stateNum === CONST.REPORT.STATE_NUM.SUBMITTED && props.taskReport.statusNum === CONST.REPORT.STATUS.APPROVED) ||
	(props.action.childStateNum === CONST.REPORT.STATE_NUM.SUBMITTED && props.action.childStatusNum === CONST.REPORT.STATUS.APPROVED);

To something like this:

const isTaskCompleted = !_.isEmpty(props.taskReport) ?
	(props.taskReport.stateNum === CONST.REPORT.STATE_NUM.SUBMITTED && props.taskReport.statusNum === CONST.REPORT.STATUS.APPROVED) :
	(props.action.childStateNum === CONST.REPORT.STATE_NUM.SUBMITTED && props.action.childStatusNum === CONST.REPORT.STATUS.APPROVED);

Then to deal with the issue of the reportAction never updating with the task's completion state, we may want ensure that the BE response includes an update for the reportAction when we call either the ReopenTask or CompleteTask endpoints. We should also add it as both optimisticData and successData to handle offline cases, e.g. updating this:

App/src/libs/actions/Task.js

Line 155 in 5a3458e

function completeTask(taskReportID, parentReportID, taskTitle) {

To pass in the reportActionID:

function completeTask(taskReportID, parentReportID, reportActionID, taskTitle)

And then add that to both optimisticData and successData:

{
	onyxMethod: Onyx.METHOD.MERGE,
	key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${parentReportID}`,
	value: {[reportActionID]: {
		childStateNum: CONST.REPORT.STATE_NUM.SUBMITTED,
		childStatusNum: CONST.REPORT.STATUS.APPROVED,
	}},
}

We also need to pass the reportActionID from the TaskPreview itself here:

App/src/components/ReportActionItem/TaskPreview.js

Line 77 in 0ec61bf

TaskUtils.completeTask(props.taskReportID, parentReportID, taskTitle);

TaskUtils.completeTask(props.taskReportID, parentReportID, props.action.reportActionID, taskTitle);

The same needs to be done for reopenTask (with the relevant report state CONSTs updated).

[melvin-bot]

📣 @jjcoffee You have been assigned to this job by @marcochavezf!
Please apply to this job in Upwork 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 📖

[jjcoffee]

Thanks @marcochavezf! PR is ready for review @0xmiroslav. Note that there have been some BE changes so now the task title doesn't show after signing out, and also the "Reopened task" message won't get added - this behaviour is unrelated to the changes in this PR.

[kevinksullivan]

@jjcoffee @0xmiroslav offers sent in upwork. Thanks!

[melvin-bot]

@marcochavezf, @kevinksullivan, @jjcoffee, @0xmiroslav Whoops! This issue is 2 days overdue. Let's get this updated quick!

[0xmiros]

Not overdue. Waiting @marcochavezf's confirmation on #19671 (comment)

[kevinksullivan]

Tagged @marcochavezf in slack to bump as well

https://expensify.slack.com/archives/C049HHMV9SM/p1685751288985569?thread_ts=1684871320.623729&cid=C049HHMV9SM

[marcochavezf]

Commented here.

[jjcoffee]

@kevinksullivan Can you put this on hold for the BE issues that are being sorted in #19298? See @marcochavezf's comment. I'll then re-raise my PR once this is off hold (it will need retesting mainly in offline mode).

[kevinksullivan]

Done @jjcoffee

[melvin-bot]

@marcochavezf, @kevinksullivan, @jjcoffee, @0xmiroslav Whoops! This issue is 2 days overdue. Let's get this updated quick!

[kevinksullivan]

Still on hold

[melvin-bot]

@marcochavezf, @kevinksullivan, @jjcoffee, @0xmiroslav Whoops! This issue is 2 days overdue. Let's get this updated quick!

[kevinksullivan]

Held

[0xmiros]

Looks like backend fix for #19298 is deployed. @jjcoffee can you please check again now (including offline test)? If it's still reproducible on latest main, I think we can resume this issue and go ahead PR.

[jjcoffee]

@0xmiroslav I've retested and it is no longer reproducible in the same way. The underlying issue of the reportAction props.action.childStateNum and props.action.childStatusNum not being updated optimistically (or by the BE) is still there, but it does not manifest in practise (from what I could see in my tests).

It looks like the initial part of my fix (the tweak to isCompletedTask was added to this PR. This isn't quite correctly implemented, see this bug I've posted on Slack. We can either handle that here or in a separate issue, I'm not sure what's better?

If we don't handle it here, I'm not sure how this works with compensation as I think technically when hired and assigned, compensation is guaranteed (according to this). It does take quite a bit of time to put together PRs and do all the testing, so I would expect compensation here. cc @kevinksullivan

[kevinksullivan]

Hi @jjcoffee , can you elaborate on what you're proposing? You're saying there is a new, somewhat related bug to the one you reported, and you're looking to fix it? Or something else? Sorry if I am just misreading you here.

[jjcoffee]

@kevinksullivan Sure! I discovered a related bug whilst retesting this issue, but the test steps are quite different as it involves two users:

1. Log in to NewDot on Chrome web with User A
2. User A opens a chat with User B and creates a task assigned to User A
3. User A completes the task
4. User B signs in and opens the chat, the checkbox shows the task still open.

The RCA and fix is the same as my proposal here, that's why I thought maybe we can just handle it here.

[melvin-bot]

@marcochavezf, @kevinksullivan, @jjcoffee, @0xmiroslav Whoops! This issue is 2 days overdue. Let's get this updated quick!

[jjcoffee]

The related issue I mentioned is being handled here. Checking on Slack regarding compensation in this slightly odd case!

[melvin-bot]

@marcochavezf, @kevinksullivan, @jjcoffee, @0xmiroslav Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[kevinksullivan]

checking internally

https://expensify.slack.com/archives/C01SKUP7QR0/p1689282907772589

[melvin-bot]

@marcochavezf, @kevinksullivan, @jjcoffee, @0xmiroslav Whoops! This issue is 2 days overdue. Let's get this updated quick!

[kevinksullivan]

Concluded this is not eligible for payout since it wasn't solved in this issue. Going to close out but let me know if you have any further questions on this.