Set up Avatars for Workspace Chats #7852

marcochavezf · 2022-02-22T00:52:08Z

Details

This PR includes the avatars for workspace chats (for both admins and workspace members) and shows the workspace chats when the user has the policyExpenseChat beta. I also included fixes for some minor issues I found (like duplicate rows on the Search Page and the user name not included in the second line of the chat header).

Additionally, I created a Web-E PR to fix a few issues I found while I was implementing the workspace chats: https://github.com/Expensify/Web-Expensify/pull/33166

Fixed Issues

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

Tests / QA

Pre-requisites:

Create an account which will be the admin
Create 2 or more accounts which will be the workspace members
Add the created accounts to policyExpenseChat beta

Steps:

With the admin account create a Workspace and invite the workspace members.
Check the workspace chats were created for each workspace member in the Search Page and send them a message.
Login with the workspace members and verify there's only one workspace chat and check the message of the admin was received.

(Test) Verify that no errors appear in the JS console
(QA) Verify that no errors appear in the JS console

Tested On

Screenshots

Web

User:

Admin:

Mobile Web

User (with search value):

Admin (with search value):

Desktop

iOS

Android

TomatoToaster

Great start on this, excited for you to finish it. I just had one suggestion ahead of you finishing this.

src/libs/OptionsListUtils.js

marcaaron

This LGTM just one had more comment to leave.

Very nice work overall with this and I appreciate the clean up that is happening here. I would maybe have liked to see a PR with fewer changes (though not sure how possible it would have been in this case). Did find it a bit hard to focus on all the changes at once.

marcaaron · 2022-03-03T03:04:05Z

src/libs/OptionsListUtils.js

+ * @returns {Array<*>}
+ */
+function getAvatarSources(report) {
+    return _.map(lodashGet(report, 'icons', ['']), (source) => {


If can, please add some kind of comment here about why we are defaulting to an array with an empty string.

Ah I'm planning to merge this function with getReportIcons in another PR (GH issue), in theory if we use only one function (get the Icon components just when the option row is created) we won't need to use empty strings to represent a possible icon.

Also I think this solution is important in order to show the up-to-date avatar when the workspace avatar is changed, because right now we can only display the up-to-date workspace image when we reload the app (yesterday I tried to addressed this by updating the icons from Onyx storage, but then I realized is better to merge these functions and get the icons after the reports are retrieved from Onyx, and not before like currently happens).

I'm going to be OOO for a few days but I will address this issue when I'm come back.

marcaaron · 2022-03-03T03:20:31Z

This is some good advice here (at least I realized how important it is as time goes on).

TomatoToaster

You can add the comment that Marc mentioned but otherwise this LGTM.

TomatoToaster · 2022-03-03T20:39:44Z

src/pages/ReportDetailsPage.js

                                containerStyles={[styles.singleAvatarLarge, styles.mb4]}
                                imageStyles={[styles.singleAvatarLarge]}
-                                source={this.props.report.icons[0]}
+                                source={_.first(OptionsListUtils.getAvatarSources(this.props.report))}


I wanna note that this breaks the icons here, but I've already fixed that in my PR with this commit (I used your updates to RoomHeaderAvatars too!): 5030078

It's not really relevant in this PR, so no need to add it here, I just noticed it when I added reportDetailsView for the policyExpenseChats (which is something I have to do for my PR). The only reason I bring it up is that if anyone notices this is off, it's already fixed!

roryabraham · 2022-03-04T00:30:12Z

src/components/SubscriptAvatar.js

+    secondaryTooltip: PropTypes.string,
+
+    /** Set the size of avatars */
+    mode: PropTypes.oneOf(_.values(CONST.OPTION_MODE)),


NAB but in your follow-up I would recommend changing this prop to size to match the other avatar components

Sure thing, also I will add a Storybook stories of SubscriptAvatar

I addressed this change here #8144

MelvinBot · 2022-03-04T00:33:54Z

Triggered auto assignment to @ctkochan22 (InternalQA), see https://stackoverflow.com/c/expensify/questions/5042 for more details.

botify · 2022-03-04T00:33:58Z

@roryabraham looks like this was merged without passing tests. Please add a note explaining why this was done and remove the Emergency label if this is not an emergency.

roryabraham · 2022-03-04T00:35:10Z

I'm like 90% sure tests were already passing here. If not, that was a mistake on my part 😬

Unless I'm going crazy I've seen this Emergency label get mis-applied a few times recently

OSBotify · 2022-03-04T00:43:26Z

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

thesahindia · 2022-03-04T08:22:43Z

Looks like we have got a regression

cc: @roryabraham @marcochavezf

TomatoToaster · 2022-03-04T15:59:30Z

@thesahindia I've got a fix for this, let me pull it out of my PR so it can be merged sooner.

OSBotify · 2022-03-11T01:00:32Z

🚀 Deployed to staging by @roryabraham in version: 1.1.42-0 🚀

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

TomatoToaster · 2022-03-15T17:54:57Z

Verified this worked on web:
user view:

From admin view:

Admin's own chat:

iOS images in next comment.

TomatoToaster · 2022-03-15T17:56:43Z

iOS images:
admin chat view:

admin viewing user's chat:

OSBotify · 2022-03-15T20:53:15Z

🚀 Deployed to production by @chiragsalian in version: 1.1.42-6 🚀

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

marcochavezf added 5 commits February 21, 2022 16:59

check if the given report is a Workspace Chat

08ffa3d

get the linked policy of the workspace chat

86a1bbc

return avatar of the workspace for non-admin users

ef23069

return participan and workspace avatar url for admins

a2dc66d

use report.reportName for workspace chats

57f4e6c

marcochavezf self-assigned this Feb 22, 2022

marcochavezf added 13 commits February 22, 2022 12:48

Merge branch 'main' into marco-avatarsWorkspaceChats

be4c679

display workspace chat in search page

a61cafb

get reportName as option.text

425e3b7

expose option.isPolicyExpenseChat to use fullTitle

d5a963b

dislay alternateText for non-admins for workspace chats

6dd30c9

change name param name in getSearchText

efe6be5

show workspace chat in search page for admins

0dab98c

get the correct report icons when user is admin

7195cd2

create styles for SubscriptAvatar

3817498

create SubscriptAvatar component

39cc286

add showSubscript MultipleAvatars to render SubscriptAvatar

3763739

add showSubscript to option

4ef49bc

update subscript avatar styles

6543229

TomatoToaster self-requested a review February 23, 2022 20:26

TomatoToaster suggested changes Feb 23, 2022

View reviewed changes

src/libs/OptionsListUtils.js Outdated Show resolved Hide resolved

marcochavezf added 9 commits February 23, 2022 14:39

show SubscriptAvatar in HeaderView

0690dee

fix blank name in HeaderView for workspace chat

5cc113e

save ReportUtils function results in variables

d83c3c2

fix duplicate workspace chates for workspace members

463e928

fix a few jsdoc errors

5981680

fix array of strings jsdoc

0a0509a

fix duplicate admin rooms in Search Page

533f851

display policy name as alternateText for admins

25fd44d

fix displayed name in alternateText for workspace chats

3218eb3

marcochavezf requested a review from roryabraham March 2, 2022 21:59

marcaaron approved these changes Mar 3, 2022

View reviewed changes

TomatoToaster mentioned this pull request Mar 3, 2022

Add Rest of UI for Workspace Chats #7957

Merged

35 tasks

TomatoToaster approved these changes Mar 3, 2022

View reviewed changes

roryabraham approved these changes Mar 4, 2022

View reviewed changes

roryabraham merged commit 7d6ed27 into main Mar 4, 2022

roryabraham deleted the marco-avatarsWorkspaceChats branch March 4, 2022 00:33

MelvinBot assigned ctkochan22 Mar 4, 2022

botify added the Emergency label Mar 4, 2022

roryabraham removed the Emergency label Mar 4, 2022

TomatoToaster mentioned this pull request Mar 4, 2022

Fix reportDetails icons for chatrooms #8011

Merged

1 task

francoisl mentioned this pull request Mar 8, 2022

🍒 Cherry pick PR #8011 to staging 🍒 #8013

Merged

Julesssss mentioned this pull request Mar 9, 2022

Room - App crashed after tap on room name and unable log in app again after that. #8040

Closed

This was referenced Mar 9, 2022

Reapply RoomHeaderAvatars Logic to fix regression correctly this time #8058

Merged

Deploy Checklist: New Expensify 2022-03-01 #7954

Closed

OSBotify mentioned this pull request Mar 10, 2022

Deploy Checklist: New Expensify 2022-03-10 #8074

Closed

francoisl mentioned this pull request Mar 11, 2022

Workspace - Manage members - All the avatar icons are looking same in search screen to invite new members #8092

Closed

marcochavezf mentioned this pull request Mar 14, 2022

change SubscriptAvatar mode prop to size #8144

Merged

50 tasks

marcochavezf mentioned this pull request Apr 5, 2022

merge getReportIcons and getAvatarSources functions #8499

Merged

89 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set up Avatars for Workspace Chats #7852

Set up Avatars for Workspace Chats #7852

marcochavezf commented Feb 22, 2022 •

edited

Loading

TomatoToaster left a comment

marcaaron left a comment

marcaaron Mar 3, 2022

marcochavezf Mar 3, 2022

marcaaron commented Mar 3, 2022

TomatoToaster left a comment

TomatoToaster Mar 3, 2022

roryabraham Mar 4, 2022

marcochavezf Mar 4, 2022

marcochavezf Mar 14, 2022

MelvinBot commented Mar 4, 2022

botify commented Mar 4, 2022

roryabraham commented Mar 4, 2022 •

edited

Loading

OSBotify commented Mar 4, 2022

thesahindia commented Mar 4, 2022

TomatoToaster commented Mar 4, 2022 •

edited

Loading

OSBotify commented Mar 11, 2022

TomatoToaster commented Mar 15, 2022

TomatoToaster commented Mar 15, 2022

OSBotify commented Mar 15, 2022

Set up Avatars for Workspace Chats #7852

Set up Avatars for Workspace Chats #7852

Conversation

marcochavezf commented Feb 22, 2022 • edited Loading

Details

Fixed Issues

Tests / QA

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

TomatoToaster left a comment

Choose a reason for hiding this comment

marcaaron left a comment

Choose a reason for hiding this comment

marcaaron Mar 3, 2022

Choose a reason for hiding this comment

marcochavezf Mar 3, 2022

Choose a reason for hiding this comment

marcaaron commented Mar 3, 2022

TomatoToaster left a comment

Choose a reason for hiding this comment

TomatoToaster Mar 3, 2022

Choose a reason for hiding this comment

roryabraham Mar 4, 2022

Choose a reason for hiding this comment

marcochavezf Mar 4, 2022

Choose a reason for hiding this comment

marcochavezf Mar 14, 2022

Choose a reason for hiding this comment

MelvinBot commented Mar 4, 2022

botify commented Mar 4, 2022

roryabraham commented Mar 4, 2022 • edited Loading

OSBotify commented Mar 4, 2022

thesahindia commented Mar 4, 2022

TomatoToaster commented Mar 4, 2022 • edited Loading

OSBotify commented Mar 11, 2022

TomatoToaster commented Mar 15, 2022

TomatoToaster commented Mar 15, 2022

OSBotify commented Mar 15, 2022

marcochavezf commented Feb 22, 2022 •

edited

Loading

roryabraham commented Mar 4, 2022 •

edited

Loading

TomatoToaster commented Mar 4, 2022 •

edited

Loading