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.

Sign up for GitHub

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

update report preview message to use workspace name, not admin name #18945

Merged
merged 1 commit into from
May 16, 2023
Merged

update report preview message to use workspace name, not admin name #18945

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/components/ReportActionItem/IOUPreview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ const IOUPreview = (props) => {

// When displaying within a IOUDetailsModal we cannot guarantee that participants are included in the originalMessage data
// Because an IOUPreview of type split can never be rendered within the IOUDetailsModal, manually building the email array is only needed for non-billSplit ious
const participantEmails = props.isBillSplit ? props.action.originalMessage.participants : [managerEmail, ownerEmail];
const participantEmails = props.isBillSplit ? lodashGet(props.action, 'originalMessage.participants', []) : [managerEmail, ownerEmail];
const participantAvatars = OptionsListUtils.getAvatarsForLogins(participantEmails, props.personalDetails);

// Pay button should only be visible to the manager of the report.
Expand Down
15 changes: 14 additions & 1 deletion src/components/ReportActionItem/ReportPreview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,15 @@ const propTypes = {
// eslint-disable-next-line react/no-unused-prop-types
iouReportID: PropTypes.string.isRequired,

/** chatReport associated with iouReport */
chatReport: PropTypes.shape({
/** The participants of this report */
participants: PropTypes.arrayOf(PropTypes.string),

/** Whether the chat report has an outstanding IOU */
hasOutstandingIOU: PropTypes.bool.isRequired,
}),

/** Active IOU Report for current report */
iouReport: PropTypes.shape({
/** Email address of the manager in this iou report */
Expand Down Expand Up @@ -78,6 +87,7 @@ const propTypes = {
const defaultProps = {
contextMenuAnchor: null,
isHovered: false,
chatReport: {},
iouReport: {},
onViewDetailsPressed: () => {},
checkIfContextMenuActive: () => {},
Expand All @@ -89,7 +99,7 @@ const defaultProps = {
const ReportPreview = (props) => {
const reportAmount = CurrencyUtils.convertToDisplayString(ReportUtils.getMoneyRequestTotal(props.iouReport), props.iouReport.currency);
const managerEmail = props.iouReport.managerEmail || '';
const managerName = ReportUtils.getDisplayNameForParticipant(managerEmail, true);
const managerName = ReportUtils.isPolicyExpenseChat(props.chatReport) ? ReportUtils.getPolicyName(props.chatReport) : ReportUtils.getDisplayNameForParticipant(managerEmail, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

We're pulling the chatReport object to simply get the managerName? Is it not possible to get this from the IOUReport?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's ideal but this is current IOUReport data:
Screenshot 2023-05-15 at 4 30 07 PM

Copy link
Contributor

@Julesssss Julesssss May 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Ah, right. What about the reportAction? IOUActions should contain participants under action.originalMessage.participants

Copy link
Contributor Author

Choose a reason for hiding this comment

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

still don't have chat report detail, only IDs

Screenshot 2023-05-15 at 4 38 41 PM

Currently __fake__ is in participants which is bug I mentioned and blocking test.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, that seems like a separate issue to me. But okay, we can follow up with that fix.

const isCurrentUserManager = managerEmail === lodashGet(props.session, 'email', null);
return (
<View style={[styles.chatItemMessage, styles.mt4]}>
Expand Down Expand Up @@ -149,6 +159,9 @@ ReportPreview.displayName = 'ReportPreview';
export default compose(
withLocalize,
withOnyx({
chatReport: {
key: ({chatReportID}) => `${ONYXKEYS.COLLECTION.REPORT}${chatReportID}`,
},
iouReport: {
key: ({iouReportID}) => `${ONYXKEYS.COLLECTION.REPORT}${iouReportID}`,
},
Expand Down
4 changes: 2 additions & 2 deletions src/pages/home/report/ReportActionItemSingle.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,9 @@ const ReportActionItemSingle = (props) => {
{props.shouldShowSubscriptAvatar ? (
<SubscriptAvatar
mainAvatar={{source: avatarSource, type: CONST.ICON_TYPE_AVATAR}}
secondaryAvatar={ReportUtils.getIcons(props.report, {})[0]}
secondaryAvatar={ReportUtils.getIcons(props.report, {})[props.report.isOwnPolicyExpenseChat ? 0 : 1]}
Copy link
Member

@rushatgabhane rushatgabhane May 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@0xmiroslav this code needs a comment explaining why and what we're doing. It seems arbitrary to me

Copy link
Member

@rushatgabhane rushatgabhane May 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I'm guessing index 0 has your own icon, and index 1 has the workspace icon.

App/src/libs/ReportUtils.js

Lines 753 to 755 in 50ed3e6

// If the user is an admin, return avatar source of the other participant of the report
// (their workspace chat) and the avatar source of the workspace
return [adminIcon, workspaceIcon];

This comment was marked as off-topic.

Sign in to view

Copy link
Contributor Author

Choose a reason for hiding this comment

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

shouldShowSubscriptAvatar is only for workspace chat (policyExpenseChat).
If isOwnPolicyExpenseChat, it returns array with only 1 element - workspace icon:

App/src/libs/ReportUtils.js

Line 854 in d28ec8f

return [result];

mainTooltip={actorEmail}
secondaryTooltip={ReportUtils.getReportName(props.report)}
secondaryTooltip={ReportUtils.getPolicyName(props.report)}
noMargin
/>
) : (
Expand Down