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

migrate pendingChatMembers to reportMetadata #54247

Merged
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
741fdd2
Merge branch 'main' into chore/migrate-pendingChatMembers
TMisiukiewicz Dec 16, 2024
7a94a23
move pendingChatMembers to metadata
TMisiukiewicz Dec 16, 2024
b11c74a
save report metadata as key value pairs
TMisiukiewicz Dec 16, 2024
d0497c8
Merge remote-tracking branch 'upstream/main' into chore/migrate-pendi…
TMisiukiewicz Dec 16, 2024
b53c8b1
add migration for pending members
TMisiukiewicz Dec 17, 2024
2f34725
fix creating pending members in metadata
TMisiukiewicz Dec 17, 2024
f3d686b
Merge branch 'main' into chore/migrate-pendingChatMembers
TMisiukiewicz Dec 17, 2024
0420fce
Merge remote-tracking branch 'upstream/main' into chore/migrate-pendi…
TMisiukiewicz Dec 18, 2024
778451e
fix lint changed
TMisiukiewicz Dec 18, 2024
54b28dd
fix lint changed part 2
TMisiukiewicz Dec 18, 2024
6378f73
fix lint changed errors part 3
TMisiukiewicz Dec 18, 2024
bccb31b
fix typecheck
TMisiukiewicz Dec 18, 2024
446c9ec
fix tests part 1
TMisiukiewicz Dec 18, 2024
348d10d
fix PolicyUtils tests
TMisiukiewicz Dec 18, 2024
c6c4009
ignore checking ReportScreen
TMisiukiewicz Dec 18, 2024
b62eb17
fix ts
TMisiukiewicz Dec 18, 2024
e272a3c
Merge remote-tracking branch 'upstream/main' into chore/migrate-pendi…
TMisiukiewicz Dec 19, 2024
d87bd22
update report utils
TMisiukiewicz Dec 19, 2024
ff39076
Merge remote-tracking branch 'origin/main' into chore/migrate-pending…
TMisiukiewicz Dec 20, 2024
b032b30
update pendingChatMembers in policy members
TMisiukiewicz Dec 20, 2024
1adec5a
fix linter
TMisiukiewicz Dec 20, 2024
8058099
Merge remote-tracking branch 'upstream/main' into chore/migrate-pendi…
TMisiukiewicz Dec 23, 2024
be6c47b
Merge remote-tracking branch 'upstream/main' into chore/migrate-pendi…
TMisiukiewicz Dec 24, 2024
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
9 changes: 1 addition & 8 deletions src/libs/DebugUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -530,12 +530,6 @@
},
'number',
);
case 'pendingChatMembers':
return validateArray<ArrayElement<Report, 'pendingChatMembers'>>(value, {
accountID: 'string',
pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION,
errors: 'object',
});
case 'fieldList':
return validateObject<ObjectElement<Report, 'fieldList'>>(
value,
Expand Down Expand Up @@ -618,7 +612,6 @@
iouReportID: CONST.RED_BRICK_ROAD_PENDING_ACTION,
preexistingReportID: CONST.RED_BRICK_ROAD_PENDING_ACTION,
nonReimbursableTotal: CONST.RED_BRICK_ROAD_PENDING_ACTION,
pendingChatMembers: CONST.RED_BRICK_ROAD_PENDING_ACTION,
fieldList: CONST.RED_BRICK_ROAD_PENDING_ACTION,
permissions: CONST.RED_BRICK_ROAD_PENDING_ACTION,
tripData: CONST.RED_BRICK_ROAD_PENDING_ACTION,
Expand Down Expand Up @@ -1369,7 +1362,7 @@
}

function getTransactionID(report: OnyxEntry<Report>, reportActions: OnyxEntry<ReportActions>) {
const transactionID = TransactionUtils.getTransactionID(report?.reportID ?? '-1');
const transactionID = TransactionUtils.getTransactionID(report?.reportID ?? '');

Check failure on line 1365 in src/libs/DebugUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check


return Number(transactionID) > 0
? transactionID
Expand Down
12 changes: 11 additions & 1 deletion src/libs/PolicyUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,13 @@ const isPolicyUser = (policy: OnyxInputOrEntry<Policy>, currentUserLogin?: strin
const isPolicyAuditor = (policy: OnyxInputOrEntry<Policy>, currentUserLogin?: string): boolean =>
(policy?.role ?? (currentUserLogin && policy?.employeeList?.[currentUserLogin]?.role)) === CONST.POLICY.ROLE.AUDITOR;

const isPolicyEmployee = (policyID: string, policies: OnyxCollection<Policy>): boolean => Object.values(policies ?? {}).some((policy) => policy?.id === policyID);
const isPolicyEmployee = (policyID: string | undefined, policies: OnyxCollection<Policy>): boolean => {
if (!policyID) {
return false;
}

return Object.values(policies ?? {}).some((policy) => policy?.id === policyID);
};

/**
* Checks if the current user is an owner (creator) of the policy.
Expand Down Expand Up @@ -1108,6 +1114,10 @@ function hasVBBA(policyID: string) {
}

function getTagApproverRule(policyOrID: string | SearchPolicy | OnyxEntry<Policy>, tagName: string) {
if (!policyOrID) {
return;
}

const policy = typeof policyOrID === 'string' ? getPolicy(policyOrID) : policyOrID;

const approvalRules = policy?.rules?.approvalRules ?? [];
Expand Down
48 changes: 40 additions & 8 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,9 @@ import type {ErrorFields, Errors, Icon, PendingAction} from '@src/types/onyx/Ony
import type {OriginalMessageChangeLog, PaymentMethodType} from '@src/types/onyx/OriginalMessage';
import type {Status} from '@src/types/onyx/PersonalDetails';
import type {ConnectionName} from '@src/types/onyx/Policy';
import type {NotificationPreference, Participants, PendingChatMember, Participant as ReportParticipant} from '@src/types/onyx/Report';
import type {NotificationPreference, Participants, Participant as ReportParticipant} from '@src/types/onyx/Report';
import type {Message, OldDotReportAction, ReportActions} from '@src/types/onyx/ReportAction';
import type {PendingChatMember} from '@src/types/onyx/ReportMetadata';
import type {SearchPolicy, SearchReport, SearchTransaction} from '@src/types/onyx/SearchResults';
import type {Comment, TransactionChanges, WaypointCollection} from '@src/types/onyx/Transaction';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
Expand Down Expand Up @@ -385,6 +386,7 @@ type OptimisticWorkspaceChats = {
expenseChatData: OptimisticChatReport;
expenseReportActionData: Record<string, OptimisticCreatedReportAction>;
expenseCreatedReportActionID: string;
pendingChatMembers: PendingChatMember[];
};

type OptimisticModifiedExpenseReportAction = Pick<
Expand Down Expand Up @@ -702,6 +704,7 @@ Onyx.connect({
});

let allReportMetadata: OnyxCollection<ReportMetadata>;
const allReportMetadataKeyValue: Record<string, ReportMetadata> = {};
Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT_METADATA,
waitForCollectionCallback: true,
Expand All @@ -710,6 +713,15 @@ Onyx.connect({
return;
}
allReportMetadata = value;

Object.entries(value).forEach(([reportID, reportMetadata]) => {
if (!reportMetadata) {
return;
}

const [, id] = reportID.split('_');
allReportMetadataKeyValue[id] = reportMetadata;
});
Comment on lines +716 to +723
Copy link
Contributor

Choose a reason for hiding this comment

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

@TMisiukiewicz Is this really needed? I am not sure about this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shubham1206agra I decided to normalize the state here to make faster and easier calls to reportMetadata based on reportID. I was doing performance measurements and it this solution does not affect performance when making thousands of calls to reportMetadata when e.g. calculating option list

},
});

Expand Down Expand Up @@ -1760,7 +1772,11 @@ function isPayAtEndExpenseReport(reportID: string, transactions: Transaction[] |
/**
* Checks if a report is a transaction thread associated with a report that has only one transaction
*/
function isOneTransactionThread(reportID: string, parentReportID: string, threadParentReportAction: OnyxEntry<ReportAction>): boolean {
function isOneTransactionThread(reportID: string | undefined, parentReportID: string | undefined, threadParentReportAction: OnyxEntry<ReportAction>): boolean {
if (!reportID || !parentReportID) {
return false;
}

const parentReportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${parentReportID}`] ?? ([] as ReportAction[]);
const transactionThreadReportID = ReportActionsUtils.getOneTransactionThreadReportID(parentReportID, parentReportActions);
return reportID === transactionThreadReportID && !ReportActionsUtils.isSentMoneyReportAction(threadParentReportAction);
Expand Down Expand Up @@ -2214,6 +2230,7 @@ function getDisplayNameForParticipant(

function getParticipantsAccountIDsForDisplay(report: OnyxEntry<Report>, shouldExcludeHidden = false, shouldExcludeDeleted = false, shouldForceExcludeCurrentUser = false): number[] {
const reportParticipants = report?.participants ?? {};
const reportMetadata = getReportMetadata(report?.reportID);
let participantsEntries = Object.entries(reportParticipants);

// We should not show participants that have an optimistic entry with the same login in the personal details
Expand Down Expand Up @@ -2253,7 +2270,7 @@ function getParticipantsAccountIDsForDisplay(report: OnyxEntry<Report>, shouldEx

if (
shouldExcludeDeleted &&
report?.pendingChatMembers?.findLast((member) => Number(member.accountID) === accountID)?.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE
reportMetadata?.pendingChatMembers?.findLast((member) => Number(member.accountID) === accountID)?.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE
) {
return false;
}
Expand Down Expand Up @@ -2314,8 +2331,10 @@ function getGroupChatName(participants?: SelectedParticipant[], shouldApplyLimit
return report.reportName;
}

const reportMetadata = getReportMetadata(report?.reportID);

const pendingMemberAccountIDs = new Set(
report?.pendingChatMembers?.filter((member) => member.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE).map((member) => member.accountID),
reportMetadata?.pendingChatMembers?.filter((member) => member.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE).map((member) => member.accountID),
);
let participantAccountIDs =
participants?.map((participant) => participant.accountID) ??
Expand Down Expand Up @@ -3018,7 +3037,11 @@ function getTitleReportField(reportFields: Record<string, PolicyReportField>) {
/**
* Get the key for a report field
*/
function getReportFieldKey(reportFieldId: string) {
function getReportFieldKey(reportFieldId: string | undefined) {
if (!reportFieldId) {
return '';
}

// We don't need to add `expensify_` prefix to the title field key, because backend stored title under a unique key `text_title`,
// and all the other report field keys are stored under `expensify_FIELD_ID`.
if (reportFieldId === CONST.REPORT_FIELD_TITLE_FIELD_ID) {
Expand Down Expand Up @@ -6073,7 +6096,6 @@ function buildOptimisticWorkspaceChats(policyID: string, policyName: string, exp
false,
policyName,
),
pendingChatMembers,
};
const adminsChatReportID = adminsChatData.reportID;
const adminsCreatedAction = buildOptimisticCreatedReportAction(CONST.POLICY.OWNER_EMAIL_FAKE);
Expand Down Expand Up @@ -6113,6 +6135,7 @@ function buildOptimisticWorkspaceChats(policyID: string, policyName: string, exp
expenseChatData,
expenseReportActionData,
expenseCreatedReportActionID: expenseReportCreatedAction.reportActionID,
pendingChatMembers,
};
}

Expand Down Expand Up @@ -8227,7 +8250,16 @@ function createDraftWorkspaceAndNavigateToConfirmationScreen(transactionID: stri
}
}

function createDraftTransactionAndNavigateToParticipantSelector(transactionID: string, reportID: string, actionName: IOUAction, reportActionID: string): void {
function createDraftTransactionAndNavigateToParticipantSelector(
transactionID: string | undefined,
reportID: string | undefined,
actionName: IOUAction,
reportActionID: string | undefined,
): void {
if (!transactionID || !reportID || !reportActionID) {
return;
}

const transaction = allTransactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`] ?? ({} as Transaction);
const reportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`] ?? ([] as ReportAction[]);

Expand Down Expand Up @@ -8542,7 +8574,7 @@ function hasInvoiceReports() {
}

function getReportMetadata(reportID?: string) {
return allReportMetadata?.[`${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`];
return allReportMetadataKeyValue[reportID ?? '-1'];
}

export {
Expand Down
29 changes: 25 additions & 4 deletions src/libs/actions/IOU.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5750,7 +5750,11 @@ function prepareToCleanUpMoneyRequest(transactionID: string, reportAction: OnyxT
* @param isSingleTransactionView - whether we are in the transaction thread report
* @returns The URL to navigate to
*/
function getNavigationUrlOnMoneyRequestDelete(transactionID: string, reportAction: OnyxTypes.ReportAction, isSingleTransactionView = false): Route | undefined {
function getNavigationUrlOnMoneyRequestDelete(transactionID: string | undefined, reportAction: OnyxTypes.ReportAction, isSingleTransactionView = false): Route | undefined {
if (!transactionID) {
return undefined;
}

const {shouldDeleteTransactionThread, shouldDeleteIOUReport, iouReport} = prepareToCleanUpMoneyRequest(transactionID, reportAction);

// Determine which report to navigate back to
Expand All @@ -5773,7 +5777,16 @@ function getNavigationUrlOnMoneyRequestDelete(transactionID: string, reportActio
* @param isSingleTransactionView - Whether we're in single transaction view
* @returns The URL to navigate to
*/
function getNavigationUrlAfterTrackExpenseDelete(chatReportID: string, transactionID: string, reportAction: OnyxTypes.ReportAction, isSingleTransactionView = false): Route | undefined {
function getNavigationUrlAfterTrackExpenseDelete(
chatReportID: string | undefined,
transactionID: string | undefined,
reportAction: OnyxTypes.ReportAction,
isSingleTransactionView = false,
): Route | undefined {
if (!chatReportID || !transactionID) {
return undefined;
}

const chatReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${chatReportID}`] ?? null;

// If not a self DM, handle it as a regular money request
Expand Down Expand Up @@ -5948,7 +5961,11 @@ function cleanUpMoneyRequest(transactionID: string, reportAction: OnyxTypes.Repo
* @param isSingleTransactionView - whether we are in the transaction thread report
* @return the url to navigate back once the money request is deleted
*/
function deleteMoneyRequest(transactionID: string, reportAction: OnyxTypes.ReportAction, isSingleTransactionView = false) {
function deleteMoneyRequest(transactionID: string | undefined, reportAction: OnyxTypes.ReportAction, isSingleTransactionView = false) {
if (!transactionID) {
return;
}

// STEP 1: Calculate and prepare the data
const {
shouldDeleteTransactionThread,
Expand Down Expand Up @@ -6209,7 +6226,11 @@ function deleteMoneyRequest(transactionID: string, reportAction: OnyxTypes.Repor
return urlToNavigateBack;
}

function deleteTrackExpense(chatReportID: string, transactionID: string, reportAction: OnyxTypes.ReportAction, isSingleTransactionView = false) {
function deleteTrackExpense(chatReportID: string | undefined, transactionID: string | undefined, reportAction: OnyxTypes.ReportAction, isSingleTransactionView = false) {
if (!chatReportID || !transactionID) {
return;
}

const urlToNavigateBack = getNavigationUrlAfterTrackExpenseDelete(chatReportID, transactionID, reportAction, isSingleTransactionView);

// STEP 1: Get all collections we're updating
Expand Down
Loading
Loading