UI for room invitations/removals#27966
Conversation
|
@Li357 finished it off for them |
jasperhuangg
changed the title
|
I'll complete the author checklist once Expensify/Web-Expensify/pull/38879 is deployed, need it to use the adhoc builds. |
|
🧪🧪 Use the links below to test this build in android and iOS. Happy testing! 🧪🧪 |
jasperhuangg
changed the title
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
…Removals UI for room invitations/removals (cherry picked from commit 409da51)
|
🚀 Cherry-picked to staging by https://github.com/francoisl in version: 1.3.87-1 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
1 similar comment
|
🚀 Cherry-picked to staging by https://github.com/francoisl in version: 1.3.87-1 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
|
🚀 Cherry-picked to staging by https://github.com/francoisl in version: 1.3.87-1 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
|
🚀 Deployed to production by https://github.com/francoisl in version: 1.3.87-12 🚀
|
| * @param {Object} policies | ||
| * @returns {Boolean} | ||
| */ | ||
| const isPolicyMember = (policyID, policies) => _.some(policies, (policy) => policy.id === policyID); |
There was a problem hiding this comment.
This caused a crash #30078 when deleting a workspace, the policy will be null. We should have included safety checks for the policy.
|
@fedirjh @jasperhuangg @Li357 can we please get a regression test for the regression it caused here? |
| prevReport.statusNum === CONST.REPORT.STATUS.OPEN && | ||
| (report.statusNum === CONST.REPORT.STATUS.CLOSED || !report.statusNum)) |
There was a problem hiding this comment.
This was being applied to all types of reports which caused #30062
| _.each(props.report.participantAccountIDs, (accountID) => { | ||
| const details = props.personalDetails[accountID]; | ||
|
|
||
| if (!details) { |
There was a problem hiding this comment.
We also need to filter out the optimistic personal detail here to prevent this bug.
More detail in this comment
| invitedEmailsToAccountIDs[login] = Number(accountID); | ||
| }); | ||
| Report.inviteToRoom(props.report.reportID, invitedEmailsToAccountIDs); | ||
| Navigation.navigate(backRoute); |
There was a problem hiding this comment.
Not sure if it correct place since a lot of things changes since, but we should've used goBack to correctly handle navigation when Room invite page is opened from different pages. #57102
| <FullPageNotFoundView | ||
| shouldShow={_.isEmpty(props.report)} | ||
| subtitleKey={_.isEmpty(props.report) ? undefined : 'roomMembersPage.notAuthorized'} | ||
| onBackButtonPress={() => Navigation.goBack(backRoute)} |
There was a problem hiding this comment.
Not sure this is the right place, but when room is archived it invite page is still accessible through direct url. More: #67005
|
@jasperhuangg Should this RoomMemberspage also be shown for Announcement room? |
11 participants
Details
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/316180
PROPOSAL:
Tests/QA
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodWaiting for Copylabel for a copy review on the original GH to get the correct copy.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Web
web.mov
Mobile Web - Chrome
mweb_chrome.mp4
Mobile Web - Safari
mweb_safari.MP4
Desktop
desktop.mov
iOS
ios.mov
Android
android.mp4