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.

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

[ReportPreview / Avatars] Fix avatar styles and headline for Ecards, invoices, and group expense reports #49172

Merged
merged 25 commits into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
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/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ const CONST = {
},

// Note: Group and Self-DM excluded as these are not tied to a Workspace
WORKSPACE_ROOM_TYPES: [chatTypes.POLICY_ADMINS, chatTypes.POLICY_ANNOUNCE, chatTypes.DOMAIN_ALL, chatTypes.POLICY_ROOM, chatTypes.POLICY_EXPENSE_CHAT],
WORKSPACE_ROOM_TYPES: [chatTypes.POLICY_ADMINS, chatTypes.POLICY_ANNOUNCE, chatTypes.DOMAIN_ALL, chatTypes.POLICY_ROOM, chatTypes.POLICY_EXPENSE_CHAT, chatTypes.INVOICE],
ANDROID_PACKAGE_NAME,
WORKSPACE_ENABLE_FEATURE_REDIRECT_DELAY: 100,
ANIMATED_HIGHLIGHT_ENTRY_DELAY: 50,
Expand Down
47 changes: 39 additions & 8 deletions src/libs/ReportActionsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,9 @@

function getOriginalMessage<T extends ReportActionName>(reportAction: OnyxInputOrEntry<ReportAction<T>>): OriginalMessage<T> | undefined {
if (!Array.isArray(reportAction?.message)) {
return reportAction?.message ?? reportAction?.originalMessage;

Check failure on line 218 in src/libs/ReportActionsUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

'originalMessage' is deprecated. Used in old report actions before migration. Replaced by using getOriginalMessage function
}
return reportAction.originalMessage;

Check failure on line 220 in src/libs/ReportActionsUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

'originalMessage' is deprecated. Used in old report actions before migration. Replaced by using getOriginalMessage function
}

function isExportIntegrationAction(reportAction: OnyxInputOrEntry<ReportAction>): boolean {
Expand Down Expand Up @@ -593,7 +593,7 @@

// HACK ALERT: We're temporarily filtering out any reportActions keyed by sequenceNumber
// to prevent bugs during the migration from sequenceNumber -> reportActionID
if (String(reportAction.sequenceNumber) === key) {

Check failure on line 596 in src/libs/ReportActionsUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

'sequenceNumber' is deprecated. Used in old report actions before migration. Replaced by reportActionID
Log.info('Front-end filtered out reportAction keyed by sequenceNumber!', false, reportAction);
return true;
}
Expand Down Expand Up @@ -994,20 +994,20 @@
CONST.IOU.REPORT_ACTION_TYPE.TRACK,
]);

/**
* Gets the reportID for the transaction thread associated with a report by iterating over the reportActions and identifying the IOU report actions.
* Returns a reportID if there is exactly one transaction thread for the report, and null otherwise.
*/
function getOneTransactionThreadReportID(reportID: string, reportActions: OnyxEntry<ReportActions> | ReportAction[], isOffline: boolean | undefined = undefined): string | undefined {
// If the report is not an IOU, Expense report, or Invoice, it shouldn't be treated as one-transaction report.
function getMoneyRequestActions(
reportID: string,
reportActions: OnyxEntry<ReportActions> | ReportAction[],
isOffline: boolean | undefined = undefined,
): Array<ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.IOU>> {
// If the report is not an IOU, Expense report, or Invoice, it shouldn't have money request actions.
const report = ReportConnection.getAllReports()?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`];
if (report?.type !== CONST.REPORT.TYPE.IOU && report?.type !== CONST.REPORT.TYPE.EXPENSE && report?.type !== CONST.REPORT.TYPE.INVOICE) {
return;
return [];
}

const reportActionsArray = Array.isArray(reportActions) ? reportActions : Object.values(reportActions ?? {});
if (!reportActionsArray.length) {
return;
return [];
}

const iouRequestActions = [];
Expand Down Expand Up @@ -1035,6 +1035,15 @@
iouRequestActions.push(action);
}
}
return iouRequestActions;
}

/**
* Gets the reportID for the transaction thread associated with a report by iterating over the reportActions and identifying the IOU report actions.
* Returns a reportID if there is exactly one transaction thread for the report, and null otherwise.
*/
function getOneTransactionThreadReportID(reportID: string, reportActions: OnyxEntry<ReportActions> | ReportAction[], isOffline: boolean | undefined = undefined): string | undefined {
const iouRequestActions = getMoneyRequestActions(reportID, reportActions, isOffline);

// If we don't have any IOU request actions, or we have more than one IOU request actions, this isn't a oneTransaction report
if (!iouRequestActions.length || iouRequestActions.length > 1) {
Expand All @@ -1054,6 +1063,27 @@
return singleAction.childReportID;
}

/**
* Returns true if all transactions on the report have the same ownerID
*/
function hasSameActorForAllTransactions(reportID: string, reportActions: OnyxEntry<ReportActions> | ReportAction[], isOffline: boolean | undefined = undefined): boolean {
Copy link
Contributor

Choose a reason for hiding this comment

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

Okay I'm going to be annoying here but trying to be future proof - for all functions that are going to affect avatars, can we first check if there is a delegateAccountID in the action, and use that instead of actorAccountID if it exists? We want to show the avatar of the copilot not the person they were copiloted into.

Copy link
Contributor

Choose a reason for hiding this comment

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

Although cc @Expensify/design @dylanexpensify since I'm not actually sure if that should apply to report previews too?

const iouRequestActions = getMoneyRequestActions(reportID, reportActions, isOffline);
if (!iouRequestActions.length) {
return true;
}

let actorID: number | undefined;

for (const action of iouRequestActions) {
if (actorID !== undefined && actorID !== action?.actorAccountID) {
return false;
}
actorID = action?.actorAccountID;
}

return true;
}

/**
* When we delete certain reports, we want to check whether there are any visible actions left to display.
* If there are no visible actions left (including system messages), we can hide the report from view entirely
Expand Down Expand Up @@ -1698,7 +1728,7 @@
}

function getCardIssuedMessage(reportAction: OnyxEntry<ReportAction>, shouldRenderHTML = false) {
const assigneeAccountID = (reportAction?.originalMessage as IssueNewCardOriginalMessage)?.assigneeAccountID;

Check failure on line 1731 in src/libs/ReportActionsUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

'originalMessage' is deprecated. Used in old report actions before migration. Replaced by using getOriginalMessage function
const assigneeDetails = PersonalDetailsUtils.getPersonalDetailsByIDs([assigneeAccountID], currentUserAccountID ?? -1)[0];

const assignee = shouldRenderHTML ? `<mention-user accountID="${assigneeAccountID}"/>` : assigneeDetails?.firstName ?? assigneeDetails.login ?? '';
Expand Down Expand Up @@ -1754,7 +1784,7 @@
getNumberOfMoneyRequests,
getOneTransactionThreadReportID,
getOriginalMessage,
getParentReportAction,

Check failure on line 1787 in src/libs/ReportActionsUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

'getParentReportAction' is deprecated. Use Onyx.connect() or withOnyx() instead

Check failure on line 1787 in src/libs/ReportActionsUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

'getParentReportAction' is deprecated. Use Onyx.connect() or withOnyx() instead
getRemovedFromApprovalChainMessage,
getReportAction,
getReportActionHtml,
Expand Down Expand Up @@ -1834,6 +1864,7 @@
getRenamedAction,
isCardIssuedAction,
getCardIssuedMessage,
hasSameActorForAllTransactions,
};

export type {LastVisibleMessage};
27 changes: 18 additions & 9 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1580,12 +1580,20 @@ function hasOnlyNonReimbursableTransactions(iouReportID: string | undefined): bo
return transactions.every((transaction) => !TransactionUtils.getReimbursable(transaction));
}

/**
* Checks if a report has only transactions with same ownerID
*/
function isSingleActorMoneyReport(reportID: string): boolean {
const reportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`] ?? ([] as ReportAction[]);
return !!ReportActionsUtils.hasSameActorForAllTransactions(reportID, reportActions);
}

/**
* Checks if a report has only one transaction associated with it
*/
function isOneTransactionReport(reportID: string): boolean {
const reportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`] ?? ([] as ReportAction[]);
return ReportActionsUtils.getOneTransactionThreadReportID(reportID, reportActions) !== null;
return !!ReportActionsUtils.getOneTransactionThreadReportID(reportID, reportActions);
}

/*
Expand Down Expand Up @@ -2210,7 +2218,7 @@ function getIcons(
if (isChatThread(report)) {
const parentReportAction = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.parentReportID}`]?.[report.parentReportActionID];

const actorAccountID = getReportActionActorAccountID(parentReportAction, report);
const actorAccountID = getReportActionActorAccountID(parentReportAction);
const actorDisplayName = PersonalDetailsUtils.getDisplayNameOrDefault(allPersonalDetails?.[actorAccountID ?? -1], '', false);
const actorIcon = {
id: actorAccountID,
Expand Down Expand Up @@ -2305,7 +2313,7 @@ function getIcons(
const isManager = currentUserAccountID === report?.managerID;

// For one transaction IOUs, display a simplified report icon
if (isOneTransactionReport(report?.reportID ?? '-1')) {
if (isOneTransactionReport(report?.reportID ?? '-1') || isSingleActorMoneyReport(report?.reportID ?? '-1')) {
return [ownerIcon];
}

Expand Down Expand Up @@ -4819,9 +4827,9 @@ function buildOptimisticReportPreview(
},
],
created,
accountID: iouReport?.managerID ?? -1,
accountID: iouReport?.ownerAccountID ?? -1,
// The preview is initially whispered if created with a receipt, so the actor is the current user as well
actorAccountID: hasReceipt ? currentUserAccountID : iouReport?.managerID ?? -1,
actorAccountID: hasReceipt ? currentUserAccountID : iouReport?.ownerAccountID ?? -1,
childReportID: childReportID ?? iouReport?.reportID,
childMoneyRequestCount: 1,
childLastMoneyRequestComment: comment,
Expand Down Expand Up @@ -6644,7 +6652,7 @@ function shouldReportShowSubscript(report: OnyxEntry<Report>): boolean {
return true;
}

if (isExpenseReport(report) && isOneTransactionReport(report?.reportID ?? '-1')) {
if (isExpenseReport(report)) {
return true;
}

Expand All @@ -6656,7 +6664,7 @@ function shouldReportShowSubscript(report: OnyxEntry<Report>): boolean {
return true;
}

if (isInvoiceRoom(report)) {
if (isInvoiceRoom(report) || isInvoiceReport(report)) {
return true;
}

Expand Down Expand Up @@ -7538,10 +7546,10 @@ function canLeaveChat(report: OnyxEntry<Report>, policy: OnyxEntry<Policy>): boo
return (isChatThread(report) && !!getReportNotificationPreference(report)) || isUserCreatedPolicyRoom(report) || isNonAdminOrOwnerOfPolicyExpenseChat(report, policy);
}

function getReportActionActorAccountID(reportAction: OnyxInputOrEntry<ReportAction>, iouReport: OnyxInputOrEntry<Report> | undefined): number | undefined {
function getReportActionActorAccountID(reportAction: OnyxInputOrEntry<ReportAction>): number | undefined {
switch (reportAction?.actionName) {
case CONST.REPORT.ACTIONS.TYPE.REPORT_PREVIEW:
return !isEmptyObject(iouReport) ? iouReport.managerID : reportAction?.childManagerAccountID;
return reportAction?.childOwnerAccountID ?? reportAction?.actorAccountID;

case CONST.REPORT.ACTIONS.TYPE.SUBMITTED:
return reportAction?.adminAccountID ?? reportAction?.actorAccountID;
Expand Down Expand Up @@ -8122,6 +8130,7 @@ export {
isIndividualInvoiceRoom,
isAuditor,
hasMissingInvoiceBankAccount,
isSingleActorMoneyReport,
};

export type {
Expand Down
Loading
Loading