Skip to content

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

Merged
jasperhuangg merged 4 commits into
mainfrom
jasper-policyRoomsBeta
Jan 12, 2022
Merged

Add policy rooms to its own beta, and restrict this beta on dev#7130
jasperhuangg merged 4 commits into
mainfrom
jasper-policyRoomsBeta

Conversation

@jasperhuangg

@jasperhuangg jasperhuangg commented Jan 11, 2022

Copy link
Copy Markdown
Contributor

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

@TomatoToaster TomatoToaster left a comment

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

@TomatoToaster Thanks so much for verifying :)

@TomatoToaster TomatoToaster left a comment

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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