[$1000] Web - WorkSpaces - Duplicate notification removed user in #admins WorkSpace

izarutskaya commented 1 year ago

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:

Create a new workspace.
Invite users into the workspace.
Remove a few members in batches.

Expected Result:

The member deletion notice is not repeated.

Actual Result:

The member deletion notice is repeated many times with the same person.

Workaround:

Unknown

Platforms:

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

[ ] Android / native
[ ] Android / Chrome
[ ] iOS / native
[ ] iOS / Safari
[x] MacOS / Chrome / Safari
[ ] MacOS / Desktop

Version Number: v1.3.55-3

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

duplicate noti remove member.webm

https://github.com/Expensify/App/assets/115492554/91bbee3f-bc4d-48e2-9056-a226a7a04102

Expensify/Expensify Issue URL:

Issue reported by: @Le Thi Thu Thuy

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1691485042751049

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~0187124a8a69db5bbb
Upwork Job ID: 1692607235796844544
Last Price Increase: 2023-08-24

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

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

s-alves10 commented 1 year ago

This looks like a backend issue. The pusher events are correct, but the openReport call for the admin room returns duplicated messages

melvin-bot[bot] commented 1 year ago

Job added to Upwork: https://www.upwork.com/jobs/~0187124a8a69db5bbb

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

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

lschurr commented 1 year ago

Adding Eng since this seems to be a backend issue.

lschurr commented 1 year ago

Bump on this one @joelbettner

lschurr commented 1 year ago

Hi @joelbettner are you able to take a look at this one?

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

Current assignee @Santhosh-Sellavel is eligible for the External assigner, not assigning anyone new.

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

Current assignee @Santhosh-Sellavel is eligible for the External assigner, not assigning anyone new.

joelbettner commented 1 year ago

This should be externally fixable, I believe.

joelbettner commented 1 year ago

I've been able to reproduce this on my machine. I did the following:

Created a workspace
Invited two people to it
Removed those two people from the workspace
Got a double message for one of the people that I removed

I can confirm in the logs that the call to Auth to unshare the policy/workspace was only made once per user.

7fbdcf865b42e1d7-ORD | staging-www1.lax | 2023-08-24 18:55:38 135 | joelbettner@expensify.com | Bedrock\Client - Starting a request ~~ command: 'SharePolicy' clusterName: 'auth' headers: '[authToken: '<REDACTED>' policyID: '734266BEB1340AD5' email: 'dbondy@expensify.com' role: '' employee: '' optimisticReportID: '0' optimisticReportActionID: '0' changeLogs: '[0: '[action: 'POLICYCHANGELOG_DELETE_EMPLOYEE' message: '[role: 'user' email: 'dbondy@expensify.com' name: 'David Bondy' accountID: '4314165']']']' logParam: 'joelbettner@expensify.com' commitCount: '20568174183' requestID: '7fbdcf865b42e1d7-ORD' lastIP: '104.28.153.122' writeConsistency: 'ASYNC' priority: '500' timeout: '290000']'
...
7fbdcf865b42e1d7-ORD | staging-www1.lax | 2023-08-24 18:55:39 255 | joelbettner@expensify.com | Bedrock\Client - Starting a request ~~ command: 'SharePolicy' clusterName: 'auth' headers: '[authToken: '<REDACTED>' policyID: '734266BEB1340AD5' email: 'tgolen@expensify.com' role: '' employee: '' optimisticReportID: '0' optimisticReportActionID: '0' changeLogs: '[0: '[action: 'POLICYCHANGELOG_DELETE_EMPLOYEE' message: '[role: 'user' email: 'tgolen@expensify.com' name: 'Tim Golen' accountID: '2697693']']' 1: '[action: 'POLICYCHANGELOG_DELETE_EMPLOYEE' message: '[role: 'user' email: 'dbondy@expensify.com' name: 'David Bondy' accountID: '4314165']']']' logParam: 'joelbettner@expensify.com' commitCount: '20568174415' requestID: '7fbdcf865b42e1d7-ORD' lastIP: '104.28.153.122' writeConsistency: 'ASYNC' priority: '500' timeout: '290000']'

I was also able to confirm in the database that two report actions for the same removal were made:

joel@db2.rno:~$ sudo readdb.sh "SELECT * FROM reportActions WHERE reportID = 2307376022370630;"
created                  reportID          accountID  action                           message                                                                                                                                                                                                                                            reportActionID     
-----------------------  ----------------  ---------  -------------------------------  -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------  -------------------
2023-08-24 18:53:55.500  2307376022370630  0          CREATED                                                                                                                                                                                                                                                                             8767464410592272693
2023-08-24 18:54:20.044  2307376022370630  3243510    POLICYCHANGELOG_UPDATE_NAME      {"lastModified":"2023-08-24 18:54:20.044","newName":"Test Workspace","oldName":"Expensify's Workspace 2","policyType":"free"}                                                                                                                      1038665327920586175
2023-08-24 18:55:02.456  2307376022370630  3243510    POLICYCHANGELOG_ADD_EMPLOYEE     {"accountID":"2697693","email":"tgolen@expensify.com","lastModified":"2023-08-24 18:55:02.456","name":"Tim Golen","role":"user"}                                                                                                                   8825457442702784574
2023-08-24 18:55:03.167  2307376022370630  3243510    POLICYCHANGELOG_ADD_EMPLOYEE     {"accountID":"4314165","email":"dbondy@expensify.com","lastModified":"2023-08-24 18:55:03.167","name":"David Bondy","role":"user"}                                                                                                                 926928206137458025 
2023-08-24 18:55:39.079  2307376022370630  3243510    POLICYCHANGELOG_DELETE_EMPLOYEE  {"accountID":"4314165","email":"dbondy@expensify.com","lastModified":"2023-08-24 18:55:39.079","name":"David Bondy","role":"user"}                                                                                                                 3429692946684186111
2023-08-24 18:55:40.288  2307376022370630  3243510    POLICYCHANGELOG_DELETE_EMPLOYEE  {"accountID":"4314165","email":"dbondy@expensify.com","lastModified":"2023-08-24 18:55:40.288","name":"David Bondy","role":"user"}                                                                                                                 2254763073580718613
2023-08-24 18:55:40.288  2307376022370630  3243510    POLICYCHANGELOG_DELETE_EMPLOYEE  {"accountID":"2697693","email":"tgolen@expensify.com","lastModified":"2023-08-24 18:55:40.288","name":"Tim Golen","role":"user"}                                                                                                                   2993888409129769369
2023-08-24 18:59:30.512  2307376022370630  3243510    POLICYCHANGELOG_ADD_EMPLOYEE     {"accountID":"2697693","email":"tgolen@expensify.com","lastModified":"2023-08-24 18:59:30.512","name":"Tim Golen","role":"user"}                                                                                                                   1041919926475572973
2023-08-24 18:59:32.238  2307376022370630  3243510    POLICYCHANGELOG_ADD_EMPLOYEE     {"accountID":"4314165","email":"dbondy@expensify.com","lastModified":"2023-08-24 18:59:32.238","name":"David Bondy","role":"user"}                                                                                                                 3721961973888883946
2023-08-24 19:00:42.658  2307376022370630  3243510    ADDCOMMENT                       {"html":"Hmm...I can see that for some reason the message for \"removed user David Bondy (<a href=\"mailto:dbondy@expensify.com\">dbondy@expensify.com</a>)\" was posted twice, but only once for Tim.","lastModified":"2023-08-24 19:00:42.658"}  6797427364659692266
joel@db2.rno:~$

So, somehow we are calling the command to create the report action twice, but only for one of the users being removed from the policy.

b4s36t4 commented 1 year ago

@joelbettner Can you share example payload that backend is expecting from frontend? I see when we're adding member we're sending members in this format

employees: [{"email":"email1"},{"email":"email2"}]

But for deleting the payload is this

emailList: email1,email2

Maybe there's a mistake how frontend is handling the payload?

joelbettner commented 1 year ago

@b4s36t4 As I continue to investigate, I think this might only be fixable in the backend. I'll let you know.

joelbettner commented 1 year ago

I believe I've found the problem, and it is a back end issue.

If you look at the calls to sharePolicy the first one for Bondy only has his action in the change Log.

Starting a request ~~ command: 'SharePolicy' clusterName: 'auth' headers: '[authToken: '<REDACTED>' policyID: '734266BEB1340AD5' email: 'dbondy@expensify.com' role: '' employee: '' optimisticReportID: '0' optimisticReportActionID: '0' changeLogs: '[0: '[action: 'POLICYCHANGELOG_DELETE_EMPLOYEE' message: '[role: 'user' email: 'dbondy@expensify.com' name: 'David Bondy' accountID: '4314165']']']' logParam: 'joelbettner@expensify.com' commitCount: '20568174183' requestID: '7fbdcf865b42e1d7-ORD' lastIP: '104.28.153.122' writeConsistency: 'ASYNC' priority: '500' timeout: '290000']'

However, the second call to sharePolicy for Tim has two actions in the the change log.

Starting a request ~~ command: 'SharePolicy' clusterName: 'auth' headers: '[authToken: '<REDACTED>' policyID: '734266BEB1340AD5' email: 'tgolen@expensify.com' role: '' employee: '' optimisticReportID: '0' optimisticReportActionID: '0' changeLogs: '[0: '[action: 'POLICYCHANGELOG_DELETE_EMPLOYEE' message: '[role: 'user' email: 'tgolen@expensify.com' name: 'Tim Golen' accountID: '2697693']']' 1: '[action: 'POLICYCHANGELOG_DELETE_EMPLOYEE' message: '[role: 'user' email: 'dbondy@expensify.com' name: 'David Bondy' accountID: '4314165']']']' logParam: 'joelbettner@expensify.com' commitCount: '20568174415' requestID: '7fbdcf865b42e1d7-ORD' lastIP: '104.28.153.122' writeConsistency: 'ASYNC' priority: '500' timeout: '290000']'

So, somehow, the change log is getting larger on the additional calls to PolicyAPI::sharePolicy here.

joelbettner commented 1 year ago

I can confirm that this is a bug at the API layer and I'll have a fix for it very soon.

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

@joelbettner Still overdue 6 days?! Let's take care of this!

melvin-bot[bot] commented 1 year ago

@joelbettner 12 days overdue. Walking. Toward. The. Light...

melvin-bot[bot] commented 1 year ago

@joelbettner 12 days overdue. Walking. Toward. The. Light...

melvin-bot[bot] commented 1 year ago

This issue has not been updated in over 14 days. @joelbettner eroding to Weekly issue.

melvin-bot[bot] commented 1 year ago

This issue has not been updated in over 15 days. @joelbettner eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

melvin-bot[bot] commented 11 months ago

@joelbettner, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.

Expensify / App