[$1000] User B still able to split the bill even when admin changes the settings

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Go to staging dot on web chrome and login with User A
2. Create a new workspace and invite User B
3. Create a new room with the workspace created in restricted mode
4. On another window, login with User B and click on the room chat
5. Now from User B side, click on + icon and click on split bill. You are now on the Currency page of split bill (Do not close)
6. From User A side, click on header and go to settings. Change the 'Who can post' to 'Admins only'
7. Notice that on User B side, the currency page doesn't disappear. You can continue to split the bill even when admin have changed the functionality.
8. If you follow the same steps with attachment, then you can see as soon as the admin changes the 'Who can post' to 'Admins only', the picture gets disappeared on User B side which is not sent yet.

Expected Result:

Split bill should get dismissed in a similar way how the attachment gets dismissed when admin changes the settings to 'Only admin'

Actual Result:

User B still able to split the bill even when admin changes the settings

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.34-1
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

green-2023-06-26_18.50.30.mp4

Recording.964.mp4

Expensify/Expensify Issue URL:
Issue reported by: @priya-zha
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1687785649051329

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01c0a939934a385c11
• Upwork Job ID: 1674641490804539392
• Last Price Increase: 2023-06-30

[melvin-bot]

Triggered auto assignment to @Christinadobrzyn (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)
• Similar to this report related to Task - [HOLD for payment 2023-07-14] [$1000] Web - UserB can still create a task in room chat when UserA changes post permission to "Admins only" #21791
• [X ] 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.

Split bill should get dismissed in a similar way how the attachment gets dismissed when admin changes the settings to 'Only admin'

What is the root cause of that problem?

The problem here is that we haven't dismissed the split bill modal once it's opened

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

We should add a condition check to allow or not, if no we will dismiss the model MoneyRequestAmountPage
In a component MoneyRequestAmountPage:

1. Define function check and dismiss modal split bill

    checkAllowDismissModal() {
        const isAllowedSplit = ReportUtils.isAllowedToComment(this.props.report);
        if (!isAllowedSplit) {
            Navigation.dismissModal(this.reportID);
        }
    }

In a componentDidUpdate and componentDidMount we will put above function

this.checkAllowDismissModal()

What alternative solutions did you explore? (Optional)

N/A

[melvin-bot]

Looks like something related to react-navigation may have been mentioned in this issue discussion.

As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our DeprecatedCustomActions.js files should not be accepted.

Feel free to drop a note in #expensify-open-source with any questions.

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[Christinadobrzyn]

I can reproduce - adding external

[Pujan92]

dupe of #21902

[priya-zha]

This is not a dupe. This issue was posted on Slack channel on June 26. Whereas #21902 was posted yesterday. Thus, 21902 is a dupe of this issue.

[Nikhil-Vats]

Proposal

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

If a user is splitting bill/assigning task in a room and permissions are changed then RHN doesn't hide dynamically.

What is the root cause of that problem?

We don't check for permission updates in the componentDidUpdate of MoneyRequestAmountPage or in the useEffect of NewTaskPage.

So if the room is archived, has any errors, or permission is changed to admin only then composer will be hidden but RHN for task or split bill won't hide.

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

We use the following function to hide the composer when room is archived, report has any errors or permissions are changed to admin only -

App/src/libs/ReportUtils.js

Lines 2319 to 2327 in 53e4154

	/**
	* Return true if the composer should be hidden
	* @param {Object} report
	* @param {Object} reportErrors
	* @returns {Boolean}
	*/
	function shouldHideComposer(report, reportErrors) {
	return isArchivedRoom(report) || !_.isEmpty(reportErrors) || !isAllowedToComment(report);
	}

We should use the same function to navigate to the report dynamically in all 3 cases. To do that, we need to add the following code in componentDidUpdate of MoneyRequestAmountPage and useEffect of NewTaskPage,NewTaskDetailsPage, etc. so that the user is navigated to report regardless of whichever step they are on.

if(ReportUtils.shouldHideComposer(report, reportErrors)) Navigation.navigate(ROUTES.getReportRoute(reportID));

I checked and report and reportErrors are available via props in both components.

We can make a new utility function for this and reuse it in both places to navigate to the parent report if permissions are changed/room is deleted or there are any errors.

What alternative solutions did you explore? (Optional)

NA

Result -

Screen.Recording.2023-06-30.at.3.53.10.AM.mov

[mollfpr]

Thank you guys for the proposals!

@Nikhil-Vats I try using Navigation.navigate, but it's adding a new stack to history. I can still press the browser back button, and it opens the money request page.

• If you want to navigate to a certain report after completing a flow related to it, e.g. RequestMoney flow with a certain group/user, you should use Navigation.dismissModal with this reportID as an argument. If, in the future, we would like to navigate to something different than the report after such flows, the API should be rather easy to change. We do it like that in order to replace the RHP flow with the new report instead of pushing it, so pressing the back button does not navigate back to the ending page of the flow. If we were to navigate to the same report, we just pop the RHP modal.

Based on our NAVIGATION guidelines, we should use Navigation.dismissModal to navigate specific reports.

We can work on @namhihi237 proposal, but I'd instead use ReportUtils.shouldHideComposer to check the report.

🎀 👀 🎀 C+ reviewed!

[melvin-bot]

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

[namhihi237]

Yes, Thanks @mollfpr reviewed the proposal, and we will go with ReportUtils.shouldHideComposer

[Christinadobrzyn]

@arosiclair will you let us know what you think of #21922 (comment)

[melvin-bot]

📣 @mollfpr 🎉 An offer has been automatically sent to your Upwork account 🎉

Reviewer - [$1000] User B still able to split the bill even when admin changes the settings

[melvin-bot]

📣 @namhihi237 You have been assigned to this job!
Please apply to this job in Upwork here and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

[melvin-bot]

📣 @Priya! 📣
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. 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.
2. 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.
3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[melvin-bot]

The BZ member will need to manually hire priya for this role Reporter. Please store your Upwork details and apply to our Upwork job so this process is automatic in the future!

[namhihi237]

@mollfpr The PR already review

[Christinadobrzyn]

@namhihi237 sorry to ask but I can't find your Upwork profile in our GHs. Can you send me a link to your Upwork profile so I can hire you? Thanks!

[namhihi237]

@Christinadobrzyn https://www.upwork.com/freelancers/~01a15b20f90b6a1737

[Christinadobrzyn]

Awesome - hired you @namhihi237! @priya-zha & @mollfpr you were automatically hired.

External Upwork job: https://www.upwork.com/jobs/~01c0a939934a385c11
Internal Upwork job: https://www.upwork.com/ab/applicants/1674641490804539392/job-details

[namhihi237]

The PR was deployed to production, I think have an issue with automatic post

[Christinadobrzyn]

I'm going to be ooo until July 31st so going to unassign and assign a new teammate.

@sophiepintoraetz At this time, we're reviewing the PR and paying based on #21922 (comment)

I'll take this back if it's still open when I return.

[melvin-bot]

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

[melvin-bot]

@arosiclair, @mollfpr, @namhihi237, @sophiepintoraetz Whoops! This issue is 2 days overdue. Let's get this updated quick!

[sophiepintoraetz]

Payments issued!