Show Domain Rooms to Domains that have at least one Approved! Accountant #14585

yuwenmemon · 2023-01-26T02:38:59Z

@techievivek please review

HOLD ON https://github.com/Expensify/Web-Expensify/pull/36242 DEPLOYED TO PROD

Details

Show Domain Rooms to any user that belongs to a domain that has at least one Approved! Accountant.

Fixed Issues

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

Tests/QA

Login with an account that belongs to a domain without an Approved Accountant but is on a private domain.
Make sure that you cannot see the #[domain.com] room in the options of the Left Hand Menu.
Make sure that you cannot see the #[domain.com] room when searching for rooms.

Login with an account that belongs to a domain with an Approved Accountant (applause.expensifail.com will work for this).
Add an expensify.com account to the #[domain.com] room by making yuwen@expensify.com(or some other Expensify account) a domain admin for applause.expensifail.com (or whatever domain you're using).
Make sure that you can see the #[domain.com] room in the options of the Left Hand Menu.
Make sure that you can see the #[domain.com] room when searching for rooms.

Login with an account that belongs to the domain gmail.com
Make sure that you cannot see any #[domain.com] room in the options of the Left Hand Menu.
Make sure that you cannot see any #[domain.com] room when searching for rooms.

Offline tests

N/A

PR Author Checklist

Screenshots/Videos

Web

Screen.Recording.2023-02-01.at.12.07.11.PM.mov

Mobile Web - Chrome

Screen.Recording.2023-02-01.at.12.03.20.PM.mov

Mobile Web - Safari

Screen.Recording.2023-02-01.at.12.12.47.PM.mov

Desktop

Screen.Recording.2023-02-01.at.10.52.52.AM.mov

iOS

Screen.Recording.2023-02-01.at.10.57.02.AM.mov

Android

Screen.Recording.2023-02-01.at.12.01.09.PM.mov

…sForApproved

… going to show it

…sForApproved

src/libs/ReportUtils.js

techievivek · 2023-02-01T05:05:52Z

Tested and the #domain.com part looks good to me. I will add the screenshots for the different platforms now.

melvin-bot · 2023-02-01T17:01:26Z

@Santhosh-Sellavel @AndrewGable One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

Santhosh-Sellavel · 2023-02-01T17:06:44Z

@yuwenmemon Can I get test credentials for this.

techievivek · 2023-02-01T17:18:53Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified tests pass on all platforms & I tested again on:
- Android / native
- Android / Chrome
- iOS / native
- iOS / Safari
- MacOS / Chrome / Safari
- MacOS / Desktop
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
- I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

Screen.Recording.2023-02-01.at.11.09.58.PM.mov

Mobile Web - Chrome

Screen.Recording.2023-02-01.at.11.20.47.PM.mov

Mobile Web - Safari

Screen.Recording.2023-02-01.at.11.16.45.PM.mov

Desktop

Screen.Recording.2023-02-01.at.11.21.59.PM.mov

iOS

Screen.Recording.2023-02-01.at.11.15.00.PM.mov

Android

Screen.Recording.2023-02-01.at.11.18.37.PM.mov

techievivek · 2023-02-01T17:20:19Z

@Santhosh-Sellavel Sorry for the ping, but this is internal, so you won't be able to test this. I am looking into it now.

yuwenmemon · 2023-02-01T17:22:29Z

@Santhosh-Sellavel Apologies, I think @techievivek or @AndrewGable will need to test as this requires access to one of our internal branches for now. Marking this as internal QA

techievivek · 2023-02-01T18:15:12Z

~~All looks good to me, going to merge this now.~~
Will have to wait for the associated PR to be on PROD.

techievivek

Looks good to me, thanks for looking into it Yuwen. 🙏

yuwenmemon · 2023-02-02T21:06:36Z

Associated Web PR hit prod, merging!

OSBotify · 2023-02-02T21:11:59Z

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

github-actions · 2023-02-02T21:41:06Z

Performance Comparison Report 📊

Significant Changes To Duration

There are no entries

Meaningless Changes To Duration

Show entries

Name	Duration
App start TTI	712.482 ms → 747.391 ms (+34.909 ms, +4.9%)
Open Search Page TTI	606.360 ms → 618.075 ms (+11.714 ms, +1.9%)
App start runJsBundle	194.688 ms → 202.969 ms (+8.281 ms, +4.3%)
App start regularAppStart	0.015 ms → 0.018 ms (+0.003 ms, +19.6%) 🟡
App start nativeLaunch	21.031 ms → 19.467 ms (-1.565 ms, -7.4%)

Show details

Name	Duration
App start TTI	Baseline Mean: 712.482 ms Stdev: 26.701 ms (3.7%) Runs: 664.3909249994904 671.3449639994651 680.0739859994501 680.441678000614 680.9307339992374 686.8484109994024 689.038382999599 691.3924100007862 696.1039370000362 698.6338550001383 699.4232379999012 702.6115899998695 703.0988059993833 703.4450870007277 707.7312809992582 711.8694679997861 712.7945230007172 713.5570640005171 715.3525220006704 715.8756150007248 716.2276850007474 717.7008379995823 719.8927439991385 720.3143279999495 720.6972320005298 740.8474259991199 745.0445559993386 747.823830999434 754.7542930003256 754.9125059992075 764.4808569997549 771.7594959996641 Current Mean: 747.391 ms Stdev: 25.015 ms (3.3%) Runs: 693.3472499996424 703.0474079996347 712.3010629992932 717.5973499994725 718.7145429998636 726.6149289999157 732.2732149995863 733.3069829996675 733.6295469999313 734.4644060004503 737.6114460006356 738.4988630004227 739.3026999998838 742.3960549999028 745.2011249996722 745.7459989991039 747.4086390007287 747.6743359994143 752.1195299997926 752.848660999909 755.0992350000888 757.2017529997975 764.3169260006398 765.0158510003239 766.8845080006868 770.4248849991709 770.8685020003468 783.5380940008909 789.9460599999875 795.2136129997671 796.4977049995214
Open Search Page TTI	Baseline Mean: 606.360 ms Stdev: 15.802 ms (2.6%) Runs: 577.7093509994447 578.8806150015444 587.8588459994644 588.0640050005168 588.3872880004346 591.3079840000719 593.4395349994302 595.7397879995406 596.2167560011148 599.3596200011671 599.9254969991744 600.7694899998605 602.5476899985224 604.0100099984556 604.7808839995414 605.1569820009172 606.4022619985044 606.9236660003662 606.9635830000043 609.3063559997827 609.9951590001583 611.2704269997776 612.819214001298 617.7786459997296 619.0113120004535 620.539875999093 621.3995769992471 625.897624000907 627.4713550005108 643.454020999372 643.785074999556 Current Mean: 618.075 ms Stdev: 20.886 ms (3.4%) Runs: 583.7787679992616 590.9011640008539 593.9162189997733 595.4546719994396 596.1632489990443 596.3197839986533 597.3099370002747 597.8035070002079 601.703409999609 602.1556810010225 604.6747229993343 605.097616000101 606.7718919999897 607.562419001013 609.1474209986627 612.1777750011533 613.4361570011824 613.6429439987987 616.4330250006169 617.8241380006075 624.0526129994541 627.0616870000958 631.0875649992377 634.4447019994259 639.2820649985224 640.61352599971 641.5711669996381 641.9835620000958 643.5244150012732 644.0009359996766 650.2895110007375 650.4015709999949 665.8812669999897
App start runJsBundle	Baseline Mean: 194.688 ms Stdev: 14.030 ms (7.2%) Runs: 171 171 172 174 179 184 184 184 185 186 187 187 189 194 194 195 195 197 197 199 199 199 200 200 202 205 211 214 216 217 219 224 Current Mean: 202.969 ms Stdev: 16.794 ms (8.3%) Runs: 175 177 181 181 182 183 186 186 189 192 193 196 198 199 199 202 204 205 206 208 210 210 215 216 220 221 222 222 223 225 234 235
App start regularAppStart	Baseline Mean: 0.015 ms Stdev: 0.001 ms (7.4%) Runs: 0.01371300034224987 0.013794001191854477 0.014038000255823135 0.01411999948322773 0.014242000877857208 0.014281999319791794 0.014364000409841537 0.014364000409841537 0.014485999941825867 0.014526000246405602 0.014566998928785324 0.01464799977838993 0.014932999387383461 0.014932999387383461 0.01493300125002861 0.01521800085902214 0.015259001404047012 0.01534000039100647 0.015461999922990799 0.015625 0.015625 0.016276000067591667 0.016316000372171402 0.01631700061261654 0.016439000144600868 0.01648000068962574 0.016683001071214676 0.016927000135183334 0.01713000051677227 0.017374999821186066 0.018187999725341797 Current Mean: 0.018 ms Stdev: 0.001 ms (5.9%) Runs: 0.016478998586535454 0.01647999882698059 0.016927000135183334 0.01696700043976307 0.016967998817563057 0.017048999667167664 0.017253000289201736 0.01729300059378147 0.017619000747799873 0.017781000584363937 0.017781998962163925 0.01798500120639801 0.01810699887573719 0.01810699887573719 0.01843300089240074 0.018595000728964806 0.0186769999563694 0.0186769999563694 0.0186769999563694 0.018880000337958336 0.019043000414967537 0.019124001264572144 0.019164999946951866 0.01961200125515461 0.019652999937534332 0.019652999937534332 0.01973400078713894 0.019735001027584076 0.01977499946951866 0.019897999241948128 0.019979000091552734
App start nativeLaunch	Baseline Mean: 21.031 ms Stdev: 2.721 ms (12.9%) Runs: 18 18 18 18 18 19 19 19 19 19 19 19 19 20 20 20 20 20 21 21 21 22 23 23 24 24 24 24 24 25 27 28 Current Mean: 19.467 ms Stdev: 1.310 ms (6.7%) Runs: 17 17 18 18 18 18 18 19 19 19 19 19 19 19 19 19 20 20 20 20 20 20 20 21 21 21 21 21 22 22

OSBotify · 2023-02-02T23:26:37Z

🚀 Deployed to staging by https://github.com/yuwenmemon in version: 1.2.64-4 🚀

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

yuwenmemon · 2023-02-03T22:39:13Z

First section ✅

yuwenmemon · 2023-02-03T22:47:24Z

Third section ✅

yuwenmemon · 2023-02-03T23:06:29Z

Second section ✅ - but I do want to test it with a real life domain not an expensifail one. Doing that with TJ here: https://expensify.slack.com/archives/C07F03W66/p1675464371178079?thread_ts=1675381316.044649&cid=C07F03W66

OSBotify · 2023-02-04T00:17:23Z

🚀 Deployed to production by https://github.com/thienlnam in version: 1.2.64-7 🚀

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

OSBotify · 2023-02-04T01:55:24Z

🚀 Deployed to production by https://github.com/thienlnam in version: 1.2.64-7 🚀

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

OSBotify · 2023-02-04T02:01:31Z

🚀 Deployed to production by https://github.com/thienlnam in version: 1.2.64-7 🚀

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

Show Domain Rooms to Domains that have at least one Approved! Accountant

630dc6e

yuwenmemon self-assigned this Jan 26, 2023

yuwenmemon added 2 commits January 26, 2023 18:56

Merge branch 'main' of github.com:Expensify/App into yuwen-domainRoom…

bca92ea

…sForApproved

Make sure that we have an Expensify email in the domain room if we're…

86d4ca9

… going to show it

techievivek self-requested a review January 27, 2023 05:17

yuwenmemon added 6 commits January 27, 2023 13:07

Merge branch 'main' of github.com:Expensify/App into yuwen-domainRoom…

bb6c1a3

…sForApproved

Fix conflicts

5d5f4c7

Add JSDoc

9b8be1c

Remove unneeded export

33df6e3

Merge branch 'main' of github.com:Expensify/App into yuwen-domainRoom…

2054e38

…sForApproved

DRY up code around default room permissions

5ba35e1

techievivek reviewed Feb 1, 2023

View reviewed changes

src/libs/ReportUtils.js Show resolved Hide resolved

Add doc and update archived room case

439d37a

yuwenmemon marked this pull request as ready for review February 1, 2023 17:00

yuwenmemon requested a review from a team as a code owner February 1, 2023 17:00

melvin-bot bot requested review from AndrewGable and Santhosh-Sellavel and removed request for a team February 1, 2023 17:01

JS Linting

e15397d

yuwenmemon added the InternalQA This pull request required internal QA label Feb 1, 2023

techievivek approved these changes Feb 1, 2023

View reviewed changes

yuwenmemon changed the title ~~[HOLD WEB#36242] Show Domain Rooms to Domains that have at least one Approved! Accountant~~ Show Domain Rooms to Domains that have at least one Approved! Accountant Feb 2, 2023

yuwenmemon merged commit aea38c5 into main Feb 2, 2023

yuwenmemon deleted the yuwen-domainRoomsForApproved branch February 2, 2023 21:07

OSBotify mentioned this pull request Feb 2, 2023

Deploy Checklist: New Expensify 2023-02-02 #14771

Closed

30 tasks

Beamanator mentioned this pull request Jun 8, 2023

[NO QA] Migrate Expensify guide & employee emails checks #20474

Merged

57 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show Domain Rooms to Domains that have at least one Approved! Accountant #14585

Show Domain Rooms to Domains that have at least one Approved! Accountant #14585

yuwenmemon commented Jan 26, 2023 •

edited by techievivek

Loading

techievivek commented Feb 1, 2023 •

edited

Loading

melvin-bot bot commented Feb 1, 2023

Santhosh-Sellavel commented Feb 1, 2023

techievivek commented Feb 1, 2023 •

edited

Loading

techievivek commented Feb 1, 2023

yuwenmemon commented Feb 1, 2023

techievivek commented Feb 1, 2023 •

edited

Loading

techievivek left a comment

yuwenmemon commented Feb 2, 2023

OSBotify commented Feb 2, 2023

github-actions bot commented Feb 2, 2023

OSBotify commented Feb 2, 2023

yuwenmemon commented Feb 3, 2023

yuwenmemon commented Feb 3, 2023

yuwenmemon commented Feb 3, 2023

OSBotify commented Feb 4, 2023

OSBotify commented Feb 4, 2023

OSBotify commented Feb 4, 2023

Show Domain Rooms to Domains that have at least one Approved! Accountant #14585

Show Domain Rooms to Domains that have at least one Approved! Accountant #14585

Conversation

yuwenmemon commented Jan 26, 2023 • edited by techievivek Loading

Details

Fixed Issues

Tests/QA

Offline tests

PR Author Checklist

Screenshots/Videos

techievivek commented Feb 1, 2023 • edited Loading

melvin-bot bot commented Feb 1, 2023

Santhosh-Sellavel commented Feb 1, 2023

techievivek commented Feb 1, 2023 • edited Loading

Reviewer Checklist

Screenshots/Videos

techievivek commented Feb 1, 2023

yuwenmemon commented Feb 1, 2023

techievivek commented Feb 1, 2023 • edited Loading

techievivek left a comment

Choose a reason for hiding this comment

yuwenmemon commented Feb 2, 2023

OSBotify commented Feb 2, 2023

github-actions bot commented Feb 2, 2023

Performance Comparison Report 📊

Significant Changes To Duration

Meaningless Changes To Duration

OSBotify commented Feb 2, 2023

yuwenmemon commented Feb 3, 2023

yuwenmemon commented Feb 3, 2023

yuwenmemon commented Feb 3, 2023

OSBotify commented Feb 4, 2023

OSBotify commented Feb 4, 2023

OSBotify commented Feb 4, 2023

yuwenmemon commented Jan 26, 2023 •

edited by techievivek

Loading

techievivek commented Feb 1, 2023 •

edited

Loading

techievivek commented Feb 1, 2023 •

edited

Loading

techievivek commented Feb 1, 2023 •

edited

Loading