App icon indicating copy to clipboard operation
App copied to clipboard

[$250] Expense - Report header title changes to Workspace owes X after editing custom name field

Open lanitochka17 opened this issue 1 year ago • 34 comments

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Version Number: 9.0.56-2 Reproducible in staging?: Y Reproducible in production?: Y If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Email or phone of affected tester (no customers): [email protected] Issue reported by: Applause - Internal Team

Action Performed:

  1. Go to staging.new.expensify.com
  2. Create a new workspace
  3. Go to workspace chat and submit an expense
  4. Open expense report
  5. Note that the report header title is Expense Report #number
  6. Go to workspace settings
  7. Enable Report fields and Rules
  8. Go to Rules
  9. Enable Custom report names
  10. Click Custom name, edit the name and save it
  11. Go back to the expense report in Step 4
  12. Click Title field
  13. Rename the title and save it

Expected Result:

The report header title should still display Expense Report #number (Old Dot behavior)

Actual Result:

The report header title changes to Workspace owes X after editing custom name field

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • [x] Android: Standalone
  • [x] Android: HybridApp
  • [x] Android: mWeb Chrome
  • [x] iOS: Standalone
  • [x] iOS: HybridApp
  • [x] iOS: mWeb Safari
  • [x] MacOS: Chrome / Safari
  • [x] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/7a29a435-e880-41f5-930b-59aa5f04e0df

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021853969647585038505
  • Upwork Job ID: 1853969647585038505
  • Last Price Increase: 2024-12-04
Issue OwnerCurrent Issue Owner: @c3024

lanitochka17 avatar Nov 01 '24 15:11 lanitochka17

Triggered auto assignment to @anmurali (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

melvin-bot[bot] avatar Nov 01 '24 15:11 melvin-bot[bot]

We think that this bug might be related to #wave-control

lanitochka17 avatar Nov 01 '24 15:11 lanitochka17

@anmurali FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

lanitochka17 avatar Nov 01 '24 15:11 lanitochka17

@anmurali Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

melvin-bot[bot] avatar Nov 04 '24 18:11 melvin-bot[bot]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Report header title changes to Workspace owes X after editing custom name field.

What is the root cause of that problem?

When a user sets a custom report name, the response from the SetPolicyDefaultReportTitle request changes the attribute type in the text_title report field from "formula" to "text" for the policy fields.

SetPolicyDefaultReportTitle Response

Hence, the getMoneyRequestReportName function doesn't return the value of report.reportName since the titleReportField is undefined.

https://github.com/Expensify/App/blob/657531f56427e51d377b642af6ef65faf30605d9/src/libs/ReportUtils.ts#L2958-L2963

This is because the getFormulaTypeReportField function only finds a report field where the type is "formula"

https://github.com/Expensify/App/blob/657531f56427e51d377b642af6ef65faf30605d9/src/libs/ReportUtils.ts#L2878-L2880

What changes do you think we should make in order to solve the problem?

Let's remove titleReportField from this condition to allow the value of report?.reportName to be returned.

Therefore, the condition should resemble the one below.

if (report?.reportName && isPaidGroupPolicyExpenseReport(report)) {

Tony-MK avatar Nov 04 '24 20:11 Tony-MK

Job added to Upwork: https://www.upwork.com/jobs/~021853969647585038505

melvin-bot[bot] avatar Nov 06 '24 01:11 melvin-bot[bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @c3024 (External)

melvin-bot[bot] avatar Nov 06 '24 01:11 melvin-bot[bot]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Report header title changes to Workspace owes X after editing custom name field

What is the root cause of that problem?

The title is obtained from getReportName. if the report name is available in the cache, it will be retrieved from there. If it is not in the cache, it will be retrieved using getMoneyRequestReportName. https://github.com/Expensify/App/blob/0f03601422dc02a42b5cf6bbc748b14be3cd1695/src/components/AvatarWithDisplayName.tsx#L74 https://github.com/Expensify/App/blob/0f03601422dc02a42b5cf6bbc748b14be3cd1695/src/libs/ReportUtils.ts#L4016-L4018

For the first expense created without setting up Rules or Report fields, fieldList will not be contained in report or policy, and the report isn't set. Therefore, reportFields will be received from getReportFieldsByPolicyID, with reportFields having type='formula'. As a result, const titleReportField = getFormulaTypeReportField(reportFields ?? {}) will return the text report fields object. Consequently, getMoneyRequestReportName will return the report name, and we will see Expense Report #number displayed in the header

https://github.com/Expensify/App/blob/0f03601422dc02a42b5cf6bbc748b14be3cd1695/src/libs/ReportUtils.ts#L3013-L3019

After that, we went on to set Rules or Report fields, enabled Custom report names, and input report , which is a text type (not a formula). At that point, fieldList exists in policy.Therefore, reportFields will be received from report?.fieldList having type='text' because we input report when setting up Custom report names. so const titleReportField = getFormulaTypeReportField(reportFields ?? {}) will return undefined because reportFields.type === 'text'. As a result, we can't retrieve the report name, and getMoneyRequestReportName will return Localize.translateLocal('iou.payerOwesAmount', {payer: payerOrApproverName, amount: formattedAmount});, which is why we see Workspace owes X

This issue happens after editing the custom name field because, before editing the custom name, the cache exists and we retrieve the value from the cache to display. After editing, we update the cache with the new value, so the new title will display after editing.

What changes do you think we should make in order to solve the problem?

To resolve this issue, we must cover the case where report?.fieldList has type = 'text' and still return the report name. Something like this:

  1. Create a function to get the text-type report field
//.src/libs/ReportUtils.ts#L2935

+   function getTextTypeReportField(reportFields: Record<string, PolicyReportField>) {
+      return Object.values(reportFields).find((field) => field?.type === 'text');
+    }
  1. Update the condition in the getMoneyRequestReportName function to return the report name
function getMoneyRequestReportName(report: OnyxEntry<Report>, policy?: OnyxEntry<Policy>, invoiceReceiverPolicy?: OnyxEntry<Policy>): string {
  ...
//.src/libs/ReportUtils.ts#L3020
+   const textTypeReportTitle = getTextTypeReportField(reportFields ?? {});

-   if (titleReportField && report?.reportName && isPaidGroupPolicyExpenseReport(report)) {
+   if ((titleReportField ?? textTypeReportTitle) && report?.reportName && isPaidGroupPolicyExpenseReport(report)) {
        return report.reportName;
    }

Optional: getMoneyRequestReportName is used for both money request reports and invoice reports. So, if we want it to be specific to only the money request report, it would be like this:

function getMoneyRequestReportName(report: OnyxEntry<Report>, policy?: OnyxEntry<Policy>, invoiceReceiverPolicy?: OnyxEntry<Policy>): string {
  ...
//.src/libs/ReportUtils.ts#L3020
+   const textTypeReportTitle = getTextTypeReportField(reportFields ?? {});

-   if (titleReportField && report?.reportName && isPaidGroupPolicyExpenseReport(report)) {
+   if ((invoiceReceiverPolicy ? titleReportField : titleReportField ?? textTypeReportTitle) && report?.reportName && isPaidGroupPolicyExpenseReport(report)) {
        return report.reportName;
    }

Test branch

POC

https://github.com/user-attachments/assets/ffc6ec22-648a-4d3e-96a6-472b141c21a3

huult avatar Nov 06 '24 18:11 huult

Proposal

Please re-state the problem that we are trying to solve in this issue.

Expense - Report header title changes to Workspace owes X after editing custom name field

What is the root cause of that problem?

We display the report.reportName only when the title field exists here https://github.com/Expensify/App/blob/8a83e2baa1c7ce7ee2c31103574937c0484854be/src/libs/ReportUtils.ts#L3015-L3018 but the method we use to get the title report field is by searching for a report field with type formula so whenever the title field is set to non-formula text that condition will be unsatisfied and we will show the owes ... title

What changes do you think we should make in order to solve the problem?

The correct way to check if there it the title field is via the fieldID, so we should change it to

    const titleReportField = Object.values(reportFields ?? {}).find((reportField) => reportField?.fieldID === CONST.REPORT_FIELD_TITLE_FIELD_ID);

What alternative solutions did you explore? (Optional)

FitseTLT avatar Nov 07 '24 22:11 FitseTLT

@anmurali, @c3024 Eep! 4 days overdue now. Issues have feelings too...

melvin-bot[bot] avatar Nov 11 '24 15:11 melvin-bot[bot]

@c3024 - you have several proposals to review above. Can you pls prioritize?

anmurali avatar Nov 12 '24 03:11 anmurali

Triggered auto assignment to @muttmuure (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

melvin-bot[bot] avatar Nov 12 '24 03:11 melvin-bot[bot]

Rotating the label for another BZ member while I am OOO

anmurali avatar Nov 12 '24 03:11 anmurali

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

melvin-bot[bot] avatar Nov 13 '24 16:11 melvin-bot[bot]

Will review and update.

c3024 avatar Nov 13 '24 16:11 c3024

@muttmuure @c3024 this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

melvin-bot[bot] avatar Nov 15 '24 09:11 melvin-bot[bot]

@muttmuure, @c3024 Huh... This is 4 days overdue. Who can take care of this?

melvin-bot[bot] avatar Nov 19 '24 09:11 melvin-bot[bot]

Thank you for your proposals.

I think this requires more clarity regarding the expected behavior.

If we change the custom report name to some text, what should be the name of the new expense reports created by new members? 1. Expense Report #ReportNumber 2. Workspace owes X

Currently, these reports are named “Workspace owes X” because this check here:

https://github.com/Expensify/App/blob/f8819c51ffcdd29a34c69708e8fcbec35e511335/src/libs/ReportUtils.ts#L4723-L4727

fails, and the formulaic name “Expense Report #ReportNumber” is not applied.

If this existing behavior for report name for new reports created by new users of the workspace is correct, then the actual behavior—where previously created reports also show the name “Workspace owes X” after changing the custom report name from a formula to text—seems consistent and correct to me.

@muttmuure

Could you check this and share your opinion? Thanks!

c3024 avatar Nov 19 '24 13:11 c3024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

melvin-bot[bot] avatar Nov 20 '24 16:11 melvin-bot[bot]

@muttmuure, @c3024 Huh... This is 4 days overdue. Who can take care of this?

melvin-bot[bot] avatar Nov 25 '24 09:11 melvin-bot[bot]

Sorry will take a look at this tomorrow

muttmuure avatar Nov 26 '24 18:11 muttmuure

@muttmuure, @c3024 Still overdue 6 days?! Let's take care of this!

melvin-bot[bot] avatar Nov 27 '24 09:11 melvin-bot[bot]

New reports should follow the custom name rule

muttmuure avatar Nov 27 '24 13:11 muttmuure

cc @c3024

muttmuure avatar Nov 27 '24 13:11 muttmuure

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

melvin-bot[bot] avatar Nov 27 '24 16:11 melvin-bot[bot]

@muttmuure @c3024 this issue is now 4 weeks old, please consider:

  • Finding a contributor to fix the bug
  • Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
  • If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

melvin-bot[bot] avatar Nov 29 '24 09:11 melvin-bot[bot]

@muttmuure, @c3024 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

melvin-bot[bot] avatar Nov 29 '24 09:11 melvin-bot[bot]

Thanks @muttmuure.

New reports should follow the custom name rule

We can have a simple text (say "Common Report Name") instead of a formula as custom report name.

Screenshot 2024-11-29 at 5 43 46 PM

In this case, should all new expense reports of the workspace have their report names as the same text (like "Common Report Name")? All reports having the same text as their report name kinda looks odd to me. I think we should have a fallback formula like Expense Report #ReportNumber (or continue existing practice of using "Workspace owes X") when the custom report name is not any formula but a simple string.

c3024 avatar Nov 29 '24 12:11 c3024

All reports having the same text as their report name kinda looks odd to me. I think we should have a fallback formula like Expense Report #ReportNumber

@c3024 When the title is set to non-formula text the BE is already returning the expense report's reportName as Expense Report #ReportNumber the current issue's inconsistency is happening because we are assuming the title field should be a formula and when the title field is non-formula text this condition here fails https://github.com/Expensify/App/blob/8a83e2baa1c7ce7ee2c31103574937c0484854be/src/libs/ReportUtils.ts#L3015-L3018 so we need to check for the title field via checking whether reportField.fieldID is REPORT_FIELD_TITLE_FIELD_ID then we will fallback to Expense Report #ReportNumber when the title in non-formula.

FitseTLT avatar Nov 29 '24 12:11 FitseTLT

@muttmuure, @c3024 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

melvin-bot[bot] avatar Dec 03 '24 09:12 melvin-bot[bot]