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

Add policy rooms to its own beta, and restrict this beta on dev #7130

Merged
merged 4 commits into from
Jan 12, 2022

Conversation

jasperhuangg
Copy link
Contributor

@jasperhuangg jasperhuangg commented Jan 11, 2022

Details

This restricts the usage of policy rooms to those behind the beta on dev. This includes creating policy rooms via the global create menu, and accessing them in the LHN.

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/191754

Tests/QA

N/A

Screenshots

Web

Mobile Web

Desktop

iOS

Android

@jasperhuangg jasperhuangg self-assigned this Jan 11, 2022
TomatoToaster
TomatoToaster previously approved these changes Jan 11, 2022
Copy link
Contributor

@TomatoToaster TomatoToaster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice this will work. We could also do something about filtering out the policy rooms from LHN and search too.

@TomatoToaster
Copy link
Contributor

NAB: Not sure how to tag this in the review, but could you add a line similar to here:
https://github.com/Expensify/App/blob/main/src/libs/OptionsListUtils.js#L414-L416

        if (ReportUtils.isUserCreatedPolicyRoom(report) && !Permissions.canUsePolicyRooms(betas)) {
            return;
        }

I don't think we need to keep the excludeDefaultRooms part for this condition because it's redundant with the earlier one.

TomatoToaster
TomatoToaster previously approved these changes Jan 11, 2022
@jasperhuangg jasperhuangg marked this pull request as ready for review January 11, 2022 18:17
@jasperhuangg jasperhuangg requested a review from a team as a code owner January 11, 2022 18:17
@MelvinBot MelvinBot requested review from ctkochan22 and removed request for a team January 11, 2022 18:18
@TomatoToaster
Copy link
Contributor

Cool and I verified on production, normal accounts don't have the "all" beta enabled by default while expensifail accounts do. Contributors should stop seeing these now but QA and Expensifiers should still be able to.

@jasperhuangg jasperhuangg removed the request for review from ctkochan22 January 11, 2022 18:33
@jasperhuangg
Copy link
Contributor Author

@TomatoToaster Thanks so much for verifying :)

Copy link
Contributor

@TomatoToaster TomatoToaster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm weird GH quirk removed my stale review I think

@jasperhuangg jasperhuangg merged commit 5ed23e4 into main Jan 12, 2022
@jasperhuangg jasperhuangg deleted the jasper-policyRoomsBeta branch January 12, 2022 01:12
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @jasperhuangg in version: 1.1.27-2 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @jasperhuangg in version: 1.1.27-3 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 failure ❌
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @jasperhuangg in version: 1.1.27-3 🚀

platform result
🤖 android 🤖 skipped 🚫
🖥 desktop 🖥 skipped 🚫
🍎 iOS 🍎 skipped 🚫
🕸 web 🕸 skipped 🚫

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @Jag96 in version: 1.1.29-5 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants