Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions src/libs/OptionsListUtils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@
*/
let currentUserLogin: string | undefined;
let currentUserAccountID: number | undefined;
Onyx.connect({

Check warning on line 192 in src/libs/OptionsListUtils/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.SESSION,
callback: (value) => {
currentUserLogin = value?.email;
Expand All @@ -198,19 +198,19 @@
});

let loginList: OnyxEntry<Login>;
Onyx.connect({

Check warning on line 201 in src/libs/OptionsListUtils/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.LOGIN_LIST,
callback: (value) => (loginList = isEmptyObject(value) ? {} : value),
});

let allPersonalDetails: OnyxEntry<PersonalDetailsList>;
Onyx.connect({

Check warning on line 207 in src/libs/OptionsListUtils/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
callback: (value) => (allPersonalDetails = isEmptyObject(value) ? {} : value),
});

const policies: OnyxCollection<Policy> = {};
Onyx.connect({

Check warning on line 213 in src/libs/OptionsListUtils/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.POLICY,
callback: (policy, key) => {
if (!policy || !key || !policy.name) {
Expand All @@ -222,14 +222,14 @@
});

let allPolicies: OnyxCollection<Policy> = {};
Onyx.connect({

Check warning on line 225 in src/libs/OptionsListUtils/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.POLICY,
waitForCollectionCallback: true,
callback: (val) => (allPolicies = val),
});

let allReports: OnyxCollection<Report>;
Onyx.connect({

Check warning on line 232 in src/libs/OptionsListUtils/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.REPORT,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -238,7 +238,7 @@
});

let allReportNameValuePairs: OnyxCollection<ReportNameValuePairs>;
Onyx.connect({

Check warning on line 241 in src/libs/OptionsListUtils/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -250,7 +250,7 @@
const allSortedReportActions: Record<string, ReportAction[]> = {};
let allReportActions: OnyxCollection<ReportActions>;
const lastVisibleReportActions: ReportActions = {};
Onyx.connect({

Check warning on line 253 in src/libs/OptionsListUtils/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.REPORT_ACTIONS,
waitForCollectionCallback: true,
callback: (actions) => {
Expand Down Expand Up @@ -312,7 +312,7 @@
});

let activePolicyID: OnyxEntry<string>;
Onyx.connect({

Check warning on line 315 in src/libs/OptionsListUtils/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.NVP_ACTIVE_POLICY_ID,
callback: (value) => (activePolicyID = value),
});
Expand Down Expand Up @@ -934,8 +934,8 @@
/**
* Get the option for a given report.
*/
function getReportOption(participant: Participant, reportAttributesDerived?: ReportAttributesDerivedValue['reports']): OptionData {
const report = getReportOrDraftReport(participant.reportID);
function getReportOption(participant: Participant, reportAttributesDerived?: ReportAttributesDerivedValue['reports'], reportDrafts?: OnyxCollection<Report>): OptionData {
const report = getReportOrDraftReport(participant.reportID, undefined, undefined, reportDrafts);
const visibleParticipantAccountIDs = getParticipantsAccountIDsForDisplay(report, true);

const option = createOption(
Expand Down
26 changes: 18 additions & 8 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -950,7 +950,7 @@
const parsedReportActionMessageCache: Record<string, string> = {};

let conciergeReportID: OnyxEntry<string>;
Onyx.connect({

Check warning on line 953 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.CONCIERGE_REPORT_ID,
callback: (value) => {
conciergeReportID = value;
Expand Down Expand Up @@ -1001,6 +1001,13 @@
callback: (value) => (allPolicies = value),
});

let allPolicyDrafts: OnyxCollection<Policy>;
Onyx.connectWithoutView({
key: ONYXKEYS.COLLECTION.POLICY_DRAFTS,
waitForCollectionCallback: true,
callback: (value) => (allPolicyDrafts = value),
});

Comment on lines +1004 to +1010

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we still need to avoid using Onyx.connect. Is this the only way to handle getReportOrDraftReport. Can we just rely on the passing params instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's not for getReportOrDraftReport. It's for buildOptimisticExpenseReport and getReportFieldsByPolicyID. Both functions don't affect rendering, so I think it's fine to use connectWithoutView.

let allReports: OnyxCollection<Report>;
let reportsByPolicyID: ReportByPolicyMap;
Onyx.connect({
Expand Down Expand Up @@ -1208,10 +1215,10 @@
/**
* Get the report or draft report given a reportID
*/
function getReportOrDraftReport(reportID: string | undefined, searchReports?: Report[], fallbackReport?: Report): OnyxEntry<Report> {
function getReportOrDraftReport(reportID: string | undefined, searchReports?: Report[], fallbackReport?: Report, reportDrafts?: OnyxCollection<Report>): OnyxEntry<Report> {
const searchReport = searchReports?.find((report) => report.reportID === reportID);
const onyxReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`];
return searchReport ?? onyxReport ?? allReportsDraft?.[`${ONYXKEYS.COLLECTION.REPORT_DRAFT}${reportID}`] ?? fallbackReport;
return searchReport ?? onyxReport ?? (reportDrafts ?? allReportsDraft)?.[`${ONYXKEYS.COLLECTION.REPORT_DRAFT}${reportID}`] ?? fallbackReport;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In case you use eslint-disable above, let's re-enable the rule here too

Suggested change
}
}
/* eslint-enable @typescript-eslint/no-deprecated */

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Which part? I don't see any lint warning around here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah my bad, I was running ./scripts/lintChanged.sh locally, and I must have been an an outdated version of your branch.

There's still errors in tests/unit/ReportUtilsTest.ts though

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done


function reportTransactionsSelector(transactions: OnyxCollection<Transaction>, reportID: string | undefined): Transaction[] {
Expand Down Expand Up @@ -4187,10 +4194,11 @@
return {};
}

const policyReportFields = Object.entries(allPolicies ?? {}).find(([key]) => key.replace(ONYXKEYS.COLLECTION.POLICY, '') === policyID);
const fieldList = policyReportFields?.[1]?.fieldList;
const policy = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${policyID}`];
const policyDraft = allPolicyDrafts?.[`${ONYXKEYS.COLLECTION.POLICY_DRAFTS}${policyID}`];
const fieldList = (policy ?? policyDraft)?.fieldList;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We need to get the report field list on the policy draft too. I simplify this to getting the policy object by directly indexing the object instead of iterating the entires one by one.


if (!policyReportFields || !fieldList) {
if ((!policy && !policyDraft) || !fieldList) {
return {};
}

Expand Down Expand Up @@ -6570,12 +6578,14 @@
// The amount for Expense reports are stored as negative value in the database
const storedTotal = total * -1;
const storedNonReimbursableTotal = nonReimbursableTotal * -1;
const report = chatReportID ? getReport(chatReportID, allReports) : undefined;
const report = chatReportID ? getReportOrDraftReport(chatReportID) : undefined;
const policyName = getPolicyName({report});
const formattedTotal = convertToDisplayString(storedTotal, currency);
// This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850
// eslint-disable-next-line @typescript-eslint/no-deprecated
const policy = getPolicy(policyID);
const policyReal = getPolicy(policyID);
const policyDraft = allPolicyDrafts?.[`${ONYXKEYS.COLLECTION.POLICY_DRAFTS}${policyID}`];
const policy = policyReal ?? policyDraft;

const {stateNum, statusNum} = getExpenseReportStateAndStatus(policy);

Expand Down Expand Up @@ -6611,7 +6621,7 @@
}

const titleReportField = getTitleReportField(getReportFieldsByPolicyID(policyID) ?? {});
if (!!titleReportField && isPaidGroupPolicyExpenseReport(expenseReport)) {
if (!!titleReportField && isGroupPolicy(policy?.type ?? '')) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Even though we get the policy field list from getTitleReportField(getReportFieldsByPolicyID(policyID), we still need to check whether the policy is a group policy or not. We check it by getting the policy from report.policyID.

isPaidGroupPolicyExpenseReport first checks whether the report is an expense report or not, which in this case is always true.

App/src/libs/ReportUtils.ts

Lines 1684 to 1687 in a0f0159

function isPaidGroupPolicyExpenseReport(report: OnyxEntry<Report>): boolean {
return isExpenseReport(report) && isPaidGroupPolicy(report);
}

isPaidGroupPolicy gets the policy from report.policyID and check the type.

App/src/libs/ReportUtils.ts

Lines 1662 to 1665 in a0f0159

function isPaidGroupPolicy(report: OnyxEntry<Report>): boolean {
const policyType = getPolicyType(report, allPolicies);
return policyType === CONST.POLICY.TYPE.CORPORATE || policyType === CONST.POLICY.TYPE.TEAM;
}

So, to simplify this, I just use isGroupPolicy and pass the policy?.type, where the policy can be the real policy or a draft one.

expenseReport.reportName = populateOptimisticReportFormula(titleReportField.defaultValue, expenseReport, policy);
}

Expand Down
11 changes: 11 additions & 0 deletions src/libs/actions/Policy/Policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2571,6 +2571,17 @@ function createDraftWorkspace(
disabledFields: {defaultBillable: true, reimbursable: false},
requiresCategory: true,
defaultReimbursable: true,
fieldList: {
[CONST.POLICY.FIELDS.FIELD_LIST_TITLE]: {
defaultValue: CONST.POLICY.DEFAULT_REPORT_NAME_PATTERN,
pendingFields: {defaultValue: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD, deletable: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD},
type: CONST.POLICY.DEFAULT_FIELD_LIST_TYPE,
target: CONST.POLICY.DEFAULT_FIELD_LIST_TARGET,
name: CONST.POLICY.DEFAULT_FIELD_LIST_NAME,
fieldID: CONST.POLICY.FIELDS.FIELD_LIST_TITLE,
deletable: true,
},
},

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We need to optimistically set the field list.

},
},
{
Expand Down
6 changes: 4 additions & 2 deletions src/pages/iou/request/step/IOURequestStepConfirmation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ function IOURequestStepConfirmation({
const draftPolicyID = getIOURequestPolicyID(initialTransaction, reportDraft);
const [policyDraft] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_DRAFTS}${draftPolicyID}`, {canBeMissing: true});
const [policyReal] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${realPolicyID}`, {canBeMissing: true});
const [reportDrafts] = useOnyx(ONYXKEYS.COLLECTION.REPORT_DRAFT, {canBeMissing: true});

/*
* We want to use a report from the transaction if it exists
* Also if the report was submitted and delayed submission is on, then we should use an initial report
Expand Down Expand Up @@ -260,9 +262,9 @@ function IOURequestStepConfirmation({
if (participant.isSender && iouType === CONST.IOU.TYPE.INVOICE) {
return participant;
}
return participant.accountID ? getParticipantsOption(participant, personalDetails) : getReportOption(participant, reportAttributesDerived);
return participant.accountID ? getParticipantsOption(participant, personalDetails) : getReportOption(participant, reportAttributesDerived, reportDrafts);
}) ?? [],
[transaction?.participants, iouType, personalDetails, reportAttributesDerived],
[transaction?.participants, iouType, personalDetails, reportAttributesDerived, reportDrafts],
);
const isPolicyExpenseChat = useMemo(() => participants?.some((participant) => participant.isPolicyExpenseChat), [participants]);
const shouldGenerateTransactionThreadReport = !isBetaEnabled(CONST.BETAS.NO_OPTIMISTIC_TRANSACTION_THREADS);
Expand Down
22 changes: 22 additions & 0 deletions tests/unit/ReportUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import type {LocaleContextProps} from '@components/LocaleContextProvider';
import useReportIsArchived from '@hooks/useReportIsArchived';
import {putOnHold} from '@libs/actions/IOU';
import type {OnboardingTaskLinks} from '@libs/actions/Welcome/OnboardingFlow';
import {convertToDisplayString} from '@libs/CurrencyUtils';
import DateUtils from '@libs/DateUtils';
import {getEnvironmentURL} from '@libs/Environment/Environment';
import getBase62ReportID from '@libs/getBase62ReportID';
Expand Down Expand Up @@ -8511,6 +8512,27 @@ describe('ReportUtils', () => {
});
});

describe('buildOptimisticExpenseReport', () => {
beforeEach(Onyx.clear);

it('should include the policy name in report name from report draft', async () => {
const chatReportID = '1';
const policyID = '2';
const reportDraft: Report = {
...createRandomReport(Number(chatReportID), CONST.REPORT.CHAT_TYPE.POLICY_EXPENSE_CHAT),
policyID,
};
const fakePolicy: Policy = createRandomPolicy(Number(policyID));
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_DRAFT}${chatReportID}`, reportDraft);
await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, fakePolicy);

const total = 100;
const currency = CONST.CURRENCY.USD;
const expenseReport = buildOptimisticExpenseReport(chatReportID, undefined, 1, total, currency);
expect(expenseReport.reportName).toBe(`${fakePolicy.name} owes ${convertToDisplayString(-total, currency)}`);
});
});

describe('hasEmptyReportsForPolicy', () => {
const policyID = 'workspace-001';
const otherPolicyID = 'workspace-002';
Expand Down
Loading