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

[$250] Don't allow the Approver on a policy to leave a workspace chat #43852

Closed
6 tasks done
lanitochka17 opened this issue Jun 17, 2024 · 37 comments
Closed
6 tasks done
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@lanitochka17
Copy link

lanitochka17 commented Jun 17, 2024

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.84-2
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

Precondition:

  • User is invited to a workspace
  1. Go to staging.new.expensify.com
  2. Go to workspace chat as workspace member
  3. Go offline
  4. Go to + > Submit expense > Manual
  5. Proceed to confirmation page
  6. Click on the workspace chat
  7. Click Leave

Expected Result:

The option to leave should not show up for a workspace chat unless you were invited into the room by a mention and not part of the approval workflow

Actual Result:

You see the option to leave the room

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

20240618_000151.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0194a281c56dd7cd3e
  • Upwork Job ID: 1802768531325940224
  • Last Price Increase: 2024-07-08
Issue OwnerCurrent Issue Owner: @sobitneupane
@lanitochka17 lanitochka17 added DeployBlockerCash This issue or pull request should block deployment DeployBlocker Indicates it should block deploying the API labels Jun 17, 2024
Copy link

melvin-bot bot commented Jun 17, 2024

Triggered auto assignment to @thienlnam (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

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.

@lanitochka17
Copy link
Author

@thienlnam FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

@lanitochka17
Copy link
Author

We think that this bug might be related to #wave-collect - Release 1

@thienlnam
Copy link
Contributor

This seemed like it would have been from a BE PR, but I haven't seen anything that would change how this works. Going to open up externally for some more eyes on the App checklist

@thienlnam thienlnam added the External Added to denote the issue can be worked on by a contributor label Jun 17, 2024
Copy link

melvin-bot bot commented Jun 17, 2024

Job added to Upwork: https://www.upwork.com/jobs/~0194a281c56dd7cd3e

@melvin-bot melvin-bot bot changed the title Expense - Expense can still be submitted after leaving workspace chat [$250] Expense - Expense can still be submitted after leaving workspace chat Jun 17, 2024
@thienlnam thienlnam removed the DeployBlocker Indicates it should block deploying the API label Jun 17, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 17, 2024
Copy link

melvin-bot bot commented Jun 17, 2024

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

@thienlnam thienlnam added Daily KSv2 and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Jun 17, 2024
@thienlnam
Copy link
Contributor

Demoting since this is an edge case bug and doesn't need to block deploy

@jacobkim9881
Copy link
Contributor

Proposal

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

Offline when a user clicking "Submit expense" button from composer, and then clicking "Leave" button on Details page and then continuing submit expense, submit expense works though the user is already left the workspace.

What is the root cause of that problem?

if (isChatRoom) {
const isWorkspaceMemberLeavingWorkspaceRoom = (report.visibility === CONST.REPORT.VISIBILITY.RESTRICTED || isPolicyExpenseChat) && isPolicyEmployee;
Report.leaveRoom(report.reportID, isWorkspaceMemberLeavingWorkspaceRoom);
return;
}
Report.leaveGroupChat(report.reportID);

Here a user left the workspace (it means report.notificationPreference = hidden) but on policy the user doesn't get removed from employees. And Report.leaveGroupChat worked not Report.leaveRoom for workspace because the condition didn't check workspace or not. Since the user isn't removed from policy, the user can submit expense still.

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

We should let Report.leaveRoom work and the user remove itself of the user. From this code

if (isChatRoom) {
const isWorkspaceMemberLeavingWorkspaceRoom = (report.visibility === CONST.REPORT.VISIBILITY.RESTRICTED || isPolicyExpenseChat) && isPolicyEmployee;
Report.leaveRoom(report.reportID, isWorkspaceMemberLeavingWorkspaceRoom);
return;
}
Report.leaveGroupChat(report.reportID);

Add isPolicyExpenseChat condition for checking leaving from workspace and add Member.removeMembers(accountIDsToRemove, report.policyID); for removing itself from policy. Add Navgation.resetToHome(); then just after clicking "Leave", RHPs will be closed.

        if (isChatRoom || isPolicyExpenseChat) {
           const isWorkspaceMemberLeavingWorkspaceRoom = (report.visibility === CONST.REPORT.VISIBILITY.RESTRICTED || isPolicyExpenseChat) && isPolicyEmployee;
           const accountIDsToRemove = [session?.accountID];
           Member.removeMembers(accountIDsToRemove, report.policyID);
           Report.leaveRoom(report.reportID, isWorkspaceMemberLeavingWorkspaceRoom);
  	   Navgation.resetToHome();
           return;
        }
        Report.leaveGroupChat(report.reportID);

Then when clicking Leave from workspace Report.leaveRoom does work and the user goes to report screen not to proceed to submit expense.

What alternative solutions did you explore? (Optional)

N/A

2024-06-18.11.40.47.mov

@thienlnam thienlnam added the Bug Something is broken. Auto assigns a BugZero manager. label Jun 18, 2024
Copy link

melvin-bot bot commented Jun 18, 2024

Triggered auto assignment to @laurenreidexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@sobitneupane
Copy link
Contributor

I could not reproduce the issue because the leave option is no longer available on the details page.

Screen.Recording.2024-06-19.at.15.30.48.mov

@laurenreidexpensify
Copy link
Contributor

@thienlnam what do you think re: @sobitneupane's comment above?

@thienlnam
Copy link
Contributor

Hmm, it looks like you were invited to that workspace too. Did we end up removing / moving the leave button recently? Maybe this change?

@laurenreidexpensify laurenreidexpensify removed the Bug Something is broken. Auto assigns a BugZero manager. label Jun 20, 2024
@melvin-bot melvin-bot bot added the Overdue label Jun 24, 2024
@thienlnam
Copy link
Contributor

Not overdue - pending proposals

@melvin-bot melvin-bot bot removed the Overdue label Jun 24, 2024
@thienlnam thienlnam changed the title [$250] You are allowed to leave a workspace chat [$250] Don't allow the Approver on a policy to leave a workspace chat Jun 24, 2024
@thienlnam
Copy link
Contributor

The other cases were addressed here #44237, so now we just need to handle the approver case

Copy link

melvin-bot bot commented Jun 28, 2024

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

@melvin-bot melvin-bot bot added the Overdue label Jun 28, 2024
@isabelastisser
Copy link
Contributor

Still waiting for proposals.

@sobitneupane
Copy link
Contributor

Not overdue - pending proposals

@melvin-bot melvin-bot bot removed the Overdue label Jul 1, 2024
@jacobkim9881
Copy link
Contributor

so now we just need to handle the approver case

@thienlnam Could you clarify how to handle the approver case? As I understand, the approver is assigned by workspace owner. Is it right? Also as I understand the case is when the approver can leave the workspace while submitting expense. Please fix me if my understanding is wrong.

Copy link

melvin-bot bot commented Jul 1, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

Copy link

melvin-bot bot commented Jul 1, 2024

@sobitneupane @thienlnam @isabelastisser this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@mvtglobally
Copy link

Issue not reproducible during KI retests. (First week)

@melvin-bot melvin-bot bot added the Overdue label Jul 3, 2024
Copy link

melvin-bot bot commented Jul 4, 2024

@sobitneupane, @thienlnam, @isabelastisser Whoops! This issue is 2 days overdue. Let's get this updated quick!

@isabelastisser
Copy link
Contributor

Bump @mvtglobally, can you please retest? Thanks!

Copy link

melvin-bot bot commented Jul 8, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

Copy link

melvin-bot bot commented Jul 8, 2024

@sobitneupane, @thienlnam, @isabelastisser 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

@sobitneupane
Copy link
Contributor

#43852 (comment)

@melvin-bot melvin-bot bot removed the Overdue label Jul 9, 2024
@isabelastisser
Copy link
Contributor

Bump @mvtglobally. can you please retest? thanks!

@mvtglobally
Copy link

Issue not reproducible during KI retests. (Second week)

@kbecciv
Copy link

kbecciv commented Jul 11, 2024

Here is a video, issue no longer reproducible

Screen.Recording.2024-07-10.at.9.39.38.PM.mp4

@melvin-bot melvin-bot bot added the Overdue label Jul 11, 2024
@melvin-bot melvin-bot bot removed the Overdue label Jul 11, 2024
@github-project-automation github-project-automation bot moved this from Release 1: Spring 2024 (May) to Done in [#whatsnext] #wave-collect Jul 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
No open projects
Archived in project
Development

No branches or pull requests

8 participants