Skip to content
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.

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

Web - Task - Checkmark is not automatically updated when assignee is not set #31909

Closed
1 of 6 tasks
kbecciv opened this issue Nov 27, 2023 · 32 comments
Closed
1 of 6 tasks
Assignees
Labels
Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review

Comments

@kbecciv
Copy link

kbecciv commented Nov 27, 2023

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


Version Number: 1.4.2.3
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
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

  1. Open a chat with another account (i.e account B)
  2. Inside the chat click on plus icon in the composer
  3. Click on assign task
  4. Insert a title and click on next
  5. Confirm the task without selecting an assignee
  6. From account A click on the checkbox of the recently created task
  7. Observe the task on account B

Expected Result:

The checkbox should update and change to being marked as done on account B right after the task is marked as done from account A.

Actual Result:

The checkbox stays unmarked even after the task was marked as done from account A

Workaround:

Unknown

Platforms:

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

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6291601_1701072874875.Recording.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~015113d6ac5682a883
  • Upwork Job ID: 1729155989948653568
  • Last Price Increase: 2023-11-27
@kbecciv kbecciv added the DeployBlockerCash This issue or pull request should block deployment label Nov 27, 2023
Copy link
Contributor

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

Copy link

melvin-bot bot commented Nov 27, 2023

Triggered auto assignment to @dangrous (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

Copy link

melvin-bot bot commented Nov 27, 2023

📣 @github-actions[bot]! 📣
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.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@kbecciv
Copy link
Author

kbecciv commented Nov 27, 2023

Production

Recording.5548.mp4

@dangrous
Copy link
Contributor

hey @kbecciv can you try your production test again but don't assign the task? That seems to be the differentiator here. I'm seeing this bug replicated on production, so I want another set of eyes.

If it's indeed the case, this is a bug we should fix BUT it's not a deploy blocker. Thank you!

@abzokhattab
Copy link
Contributor

I am also able to reproduce it on prod..

I guess this is a backend pusher issue

@mountiny
Copy link
Contributor

This seems to be more so a backend issue, I will remove the deploy blocker label since its a last blocker

@mountiny mountiny added Daily KSv2 Internal Requires API changes or must be handled by Expensify staff and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Nov 27, 2023
Copy link

melvin-bot bot commented Nov 27, 2023

Job added to Upwork: https://www.upwork.com/jobs/~015113d6ac5682a883

Copy link

melvin-bot bot commented Nov 27, 2023

Triggered auto assignment to Contributor Plus for review of internal employee PR - @alitoshmatov (Internal)

@kavimuru
Copy link

@dangrous able to reproduce in production.

Recording.1656.mp4

@dangrous
Copy link
Contributor

Great, thank you! We'll move forward with investigation but not as a blocker

@dangrous
Copy link
Contributor

dangrous commented Nov 27, 2023

Okay so two weird things:

  • participantAccountIDs when the task is completed from the creator's side only includes the creator. That's why it's not being sent. Not sure where this is occurring yet.
  • If you try to complete the task from the other account, there's a silent 411 Insufficient privileges error. Once you click into the task report, though (rather than just the parent report), it's fine.

So I think somewhere the non-creator account is not being listed as a participant in the task on initial creation, even though they should be. I think that would explain both bugs. TBD on a fix

@dangrous
Copy link
Contributor

Putting @thienlnam's comment from Slack here for posterity, will hopefully have time to chug through this this week:

Yeah I think we’d need to make sure all of those auth commands return the participants of the parent chat in $response['parentParticipants']['participantsAccountIDs'] , and then the parentReportActionID will need to have some more keys that represent the state of the task like childReportState or something similar (you’ll have to check what is returned currently)

@melvin-bot melvin-bot bot added the Overdue label Dec 1, 2023
@dangrous
Copy link
Contributor

dangrous commented Dec 1, 2023

Haven't had time for this this week, I will likely be able to look into it mid-next week at the earliest (trying to finish out the whatsnext budgets project)

Copy link

melvin-bot bot commented Dec 4, 2023

@dangrous, @alitoshmatov Whoops! This issue is 2 days overdue. Let's get this updated quick!

@dangrous
Copy link
Contributor

dangrous commented Dec 5, 2023

as noted, no time yet. will get there.

Copy link

melvin-bot bot commented Dec 8, 2023

@dangrous, @alitoshmatov Whoops! This issue is 2 days overdue. Let's get this updated quick!

@dangrous
Copy link
Contributor

dangrous commented Dec 8, 2023

I was out unexpectedly this week, this is somewhat low priority so I will hopefully be able to grab it next week

Copy link

melvin-bot bot commented Dec 12, 2023

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

@dangrous
Copy link
Contributor

no update since above, hoping to get to this later this week

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Dec 12, 2023
@dangrous
Copy link
Contributor

Maybe will have time this afternoon. But it's on my mind!

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Dec 15, 2023
@dangrous
Copy link
Contributor

@thienlnam Can we get your advice on this one? I've finally had the chance to look into it, and I have a couple questions. So there are two issues:

  • The owner of a task completing it does not update the parent report for another user in the parent report.
  • Another user in the parent report does not have the permissions to complete a task, until they've opened it.
    • Above, I mentioned that it is a 411 error, but I just tried it again and am getting 666 Cannot complete action which means it's now happening before it gets to the actual check for permissions. So how can we tell auth that that user should have permissions for the task? Do we need to change something on the creation side?

@melvin-bot melvin-bot bot removed the Overdue label Dec 19, 2023
@thienlnam
Copy link
Contributor

Shouldn't this call to sendChildDetailsToParentAction send the status update to any participants in that report?

I think you might have mis-linked but that method should do exactly that - I would maybe check if this has the parent participants correctly when it is being called but it should be sending that pusher update

Another user in the parent report does not have the permissions to complete a task, until they've opened it.

This is what I'm working on in here https://github.com/Expensify/Expensify/issues/339618, it turns out there's a lot of issues that come up when a user is not shared on a report. I believe we can fix this by just sharing the user to the report in the peek() of all these commands. Haven't tested it yet but should hopefully solve this problem

@melvin-bot melvin-bot bot added the Overdue label Dec 21, 2023
@dangrous
Copy link
Contributor

Oops yeah, I meant this - https://github.com/Expensify/Web-Expensify/blob/ac26ddb1425c58704478cc4f6d46347ff155d94c/lib/ReportAPI.php#L6089 - I looked at the stuff that it gets called with, and it had the right participants (i.e. everyone) but maybe wasn't sending the status along (but I don't really get why that would happen).

And great about the second question! I can focus on the first (unless you have ideas)

@melvin-bot melvin-bot bot removed the Overdue label Dec 21, 2023
@thienlnam
Copy link
Contributor

but maybe wasn't sending the status along (but I don't really get why that would happen).

Oh yes - you're on the right track here. That method needs to send back more parameters (i.e it's not passing back the childStateNum / childStatusNum)

@melvin-bot melvin-bot bot added the Overdue label Dec 25, 2023
Copy link

melvin-bot bot commented Dec 25, 2023

@dangrous, @alitoshmatov Whoops! This issue is 2 days overdue. Let's get this updated quick!

Copy link

melvin-bot bot commented Dec 27, 2023

@dangrous, @alitoshmatov Huh... This is 4 days overdue. Who can take care of this?

Copy link

melvin-bot bot commented Dec 29, 2023

@dangrous, @alitoshmatov Still overdue 6 days?! Let's take care of this!

Copy link

melvin-bot bot commented Jan 2, 2024

@dangrous, @alitoshmatov 10 days overdue. Is anyone even seeing these? Hello?

@dangrous
Copy link
Contributor

dangrous commented Jan 2, 2024

Back now, will start trying to get this closed out tomorrow or later today

@melvin-bot melvin-bot bot removed the Overdue label Jan 2, 2024
@dangrous
Copy link
Contributor

dangrous commented Jan 4, 2024

Oh yes - you're on the right track here. That method needs to send back more parameters (i.e it's not passing back the childStateNum / childStatusNum)

Ah lovely, this was it! Putting up a PR now.

@dangrous dangrous added the Reviewing Has a PR in review label Jan 4, 2024
@dangrous
Copy link
Contributor

Just tested this, it works nicely on prod now! I am going to close this - I don't believe any payment is due but @alitoshmatov (or anyone else?) please let me know if I am incorrect and we can reopen if needed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests

7 participants