[HOLD for payment 2025-01-24] [$250] Report fields -System message shows undefined when list value is changed back to initial value #47067

IuliiaHerets · 2024-08-08T12:38:31Z

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: v9.0.18-1
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team

Action Performed:

Go to staging.new.expensify.com
Go to workspace chat.
Submit two expenses.
Go to workspace settings > Report fields (enable in More features).
Add a list type report field, with some list values and an initial value.
Return to the workspace chat.
Go to expense report.
Click on the list report field.
Select a different list value.
Revisit the report.
Click on the list report field.
Click on the already selected value (so that it will change to the initial value).
Revisit the report.

Expected Result:

The system message will not contain "undefined".

Actual Result:

The system message contains "undefined" when the list value is changed back to the initial value.

Workaround:

Unknown

Platforms:

Screenshots/Videos

Bug6565437_1723109214472.20240808_172103.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~014a67065c97e4bdba
Upwork Job ID: 1822942704704377446
Last Price Increase: 2024-08-19
Automatic offers:

dukenv0307 | Reviewer | 103641179
nkdengineer | Contributor | 103641182
etCoderDysto | Contributor | 105612296

Issue Owner

Current Issue Owner: @dukenv0307

melvin-bot · 2024-08-08T12:38:42Z

Triggered auto assignment to @bfitzexpensify (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.

IuliiaHerets · 2024-08-08T12:38:53Z

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

IuliiaHerets · 2024-08-08T12:39:12Z

@bfitzexpensify 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

etCoderDysto · 2024-08-08T13:00:18Z

Edited by proposal-police: This proposal was edited at 2024-08-08 13:17:27 UTC.

Proposal

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

System message shows undefined when list value is changed back to initial value

What is the root cause of that problem?

When selecting the same value (fieldValue === option.text), we assign empty string to fieldKey and pass it to the submit function

App/src/pages/EditReportFieldDropdown.tsx

Line 111 in 9c7c4a1

    
           onSelectRow={(option) => onSubmit({[fieldKey]: !option?.text || fieldValue === option.text ? '' : option.text})}

Then when value is an empty string we assign null to value prop, update the BE with with a new reportField that has null as a value

App/src/pages/EditReportFieldPage.tsx

Lines 76 to 82 in 9c7c4a1

    
           const handleReportFieldChange = (form: FormOnyxValues<typeof ONYXKEYS.FORMS.REPORT_FIELDS_EDIT_FORM>) => { 
        
               const value = form[fieldKey]; 
        
               if (isReportFieldTitle) { 
        
                   ReportActions.updateReportName(report.reportID, value, report.reportName ?? ''); 
        
                   Navigation.goBack(); 
        
               } else { 
        
                   ReportActions.updateReportField(report.reportID, {...reportField, value: value === '' ? null : value}, reportField);

Then BE returns a report action object that have null on originalMessage.newValue prop

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

We should remove

|| fieldValue === option.text

from

App/src/pages/EditReportFieldDropdown.tsx

Line 111 in 9c7c4a1

    
           onSelectRow={(option) => onSubmit({[fieldKey]: !option?.text || fieldValue === option.text ? '' : option.text})}

Or we should change it to

 onSelectRow={(option) => onSubmit({[fieldKey]: option?.text  ??  '' })}

What alternative solutions did you explore? (Optional)

We can dismiss the modal when value is an empty string

if (value) {
ReportActions.updateReportField(report.reportID, {...reportField, value}, reportField);
            }
Navigation.dismissModal(report?.reportID);

nkdengineer · 2024-08-08T13:04:06Z

Edited by proposal-police: This proposal was edited at 2024-08-08 13:11:00 UTC.

Proposal

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

The system message contains "undefined" when the list value is changed back to the initial value.

What is the root cause of that problem?

If the selected value is the current value of the report field, we pass it as empty string here

App/src/pages/EditReportFieldDropdown.tsx

Line 111 in 4f45424

    
           onSelectRow={(option) => onSubmit({[fieldKey]: !option?.text || fieldValue === option.text ? '' : option.text})}

Then here we pass the new value of the report field as null

App/src/pages/EditReportFieldPage.tsx

Line 82 in 4f45424

    
           ReportActions.updateReportField(report.reportID, {...reportField, value: value === '' ? null : value}, reportField);

So BE returns the new value in report action as null. Then when merging data to Onyx, the newValue field is deleted since it has null value.

It leads newValue being undefined when we get the message here

App/src/libs/ReportActionsUtils.ts

Line 1223 in 4f45424

    
           return Localize.translateLocal('report.actions.type.changeField', {oldValue, newValue, fieldName});

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

We should add another case when the new value doesn't exist to display a message like remove the ${fieldName} (previously "${oldValue}"). We also need to create a new translation key for this message

if (!newValue) {
    return // translated message;
}

App/src/libs/ReportActionsUtils.ts

Line 1220 in 4f45424

if (!oldValue) {

What alternative solutions did you explore? (Optional)

We can do nothing if the user select the selected value

onSelectRow={(option) => {
    if (fieldValue === option.text || !option?.text) {
        return;
    }
    onSubmit({[fieldKey]: option.text})
}}

App/src/pages/EditReportFieldDropdown.tsx

Lines 111 to 112 in 4f45424

    
           onSelectRow={(option) => onSubmit({[fieldKey]: !option?.text || fieldValue === option.text ? '' : option.text})} 
        
           initiallyFocusedOptionKey={selectedOptionKey ?? undefined}

nkdengineer · 2024-08-08T13:11:12Z

Updated proposal

Detail the RCA

etCoderDysto · 2024-08-08T13:17:59Z

~~Updated proposal~~

~~Added an alternative solution~~

melvin-bot · 2024-08-12T10:26:26Z

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

melvin-bot · 2024-08-12T10:26:30Z

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

melvin-bot · 2024-08-12T10:52:42Z

Triggered auto assignment to @grgia, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

etCoderDysto · 2024-08-12T11:16:16Z

@dukenv0307 Can we please ask someone from internal to confirm the expected behaviour? From UX perspective, It doesn't make sense to select the default value when the user is selecting another report field twice. Especially when the user can select the default value if they want.

Report field values are 1, 2, 3. And the default value is 1 in the video. When selecting '2' twice, the value reverts to '1' which is the default value. If the user want to change the value to '1', they can select it form the list.

Screen.Recording.2024-08-12.at.2.12.40.in.the.afternoon.mp4

cc: @grgia @mountiny

mountiny · 2024-08-12T13:42:49Z

@etCoderDysto can you please write this bug in numbered steps? That will be easier to understand

etCoderDysto · 2024-08-12T13:53:22Z

@mountiny If we implement The selected solution, the result will be following

Steps

Go to staging.new.expensify.com
Go to workspace chat.
Submit two expenses.
Go to workspace settings > Report fields (enable in More features).
Add a list type report field, with some list values and an initial value. (in my video the initial value is '1')
Return to the workspace chat.
Go to expense report.
Click on the list report field.
Select a different list value. (in my video the value is 2)
Click on the list report field.
Click on the already selected value (in my video the value is 2)

Expected Result: The selected report field shouldn't fallback to default report field, and it should be the selected one which is ('2')
Actual result: The selected report field falls back to the initial report field('1') when user selects a report field that is already selected

Attachment:

Screen.Recording.2024-08-12.at.2.12.40.in.the.afternoon.mp4

mountiny · 2024-08-12T13:55:29Z

@etCoderDysto got it, I agree with that, we should not change to the default.

Can we just close the panel and do nothing when user clicks on the already selected report field?

nkdengineer · 2024-08-12T14:01:45Z

@mountiny

If we want to do nothing, I believe the proposal from @etCoderDysto doesn't get this expected behavior. This proposal makes the API called then a report action is created with the message displayed like change report field from value1 to value1

I updated my proposal with alternative solution to do that

etCoderDysto · 2024-08-12T14:04:15Z

Can we just close the panel and do nothing when user clicks on the already selected report field?

@mountiny When the user selects existing report field, handleReportFieldChange is called with value prop assigned to an empty string. We can check if 'value' is empty string and dismiss the modal when it is. But I have to check if this change won't introduce a regression since handleReportFieldChange is called when editing Date and Text report fields

if (value) {
  ReportActions.updateReportField(report.reportID, {...reportField, value}, reportField);reportField);
            }
Navigation.dismissModal(report?.reportID);

App/src/pages/EditReportFieldPage.tsx

Lines 76 to 82 in 9c7c4a1

    
           const handleReportFieldChange = (form: FormOnyxValues<typeof ONYXKEYS.FORMS.REPORT_FIELDS_EDIT_FORM>) => { 
        
               const value = form[fieldKey]; 
        
               if (isReportFieldTitle) { 
        
                   ReportActions.updateReportName(report.reportID, value, report.reportName ?? ''); 
        
                   Navigation.goBack(); 
        
               } else { 
        
                   ReportActions.updateReportField(report.reportID, {...reportField, value: value === '' ? null : value}, reportField);

Alternatively, my proposal states that we should call updateReportField, with the existing report field value when user selects existing report field.

nkdengineer · 2024-08-12T14:05:25Z

@mountiny Additional I think we can do this because for tag or category, we can de-select the select tag/category when we click on the selected option. So I think can do the same for report field.

etCoderDysto · 2024-08-12T14:14:04Z

@mountiny

If we want to do nothing, I believe the proposal from @etCoderDysto doesn't get this expected behavior. This proposal makes the API called then a report action is created with the message displayed like change report field from value1 to value1

I updated my proposal with alternative solution to do that

It will make the API call, but there will be no action messages. The action message is only rendered when there is a change in value. And the alternative solution you just proposed won't close the modal, since dismmisModal is called only in handleReportFieldChange

melvin-bot · 2025-01-08T16:57:22Z

📣 @etCoderDysto 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

etCoderDysto · 2025-01-11T12:49:20Z

@dukenv0307 PR is ready for review

melvin-bot · 2025-01-17T13:32:28Z

Reviewing label has been removed, please complete the "BugZero Checklist".

melvin-bot · 2025-01-17T13:32:36Z

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.86-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

fix: System message shows undefined when selecting an already selected list value #55118

If no regressions arise, payment will be issued on 2025-01-24. 🎊

For reference, here are some details about the assignees on this issue:

@dukenv0307 requires payment automatic offer (Reviewer)
@nkdengineer requires payment automatic offer (Contributor)
@etCoderDysto requires payment automatic offer (Contributor)

melvin-bot · 2025-01-17T13:32:38Z

@dukenv0307 @bfitzexpensify @dukenv0307 The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

melvin-bot · 2025-01-27T09:05:23Z

@cead22, @grgia, @bfitzexpensify, @dukenv0307, @dubielzyk-expensify, @nkdengineer, @etCoderDysto Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

bfitzexpensify · 2025-01-27T15:28:30Z

@dukenv0307 bump on the BZ checklist

bfitzexpensify · 2025-01-27T15:29:52Z

Payments complete to @nkdengineer @etCoderDysto

dukenv0307 · 2025-01-29T13:53:58Z

BugZero Checklist:

[Contributor] Classify the bug:

Bug classification

Source of bug:

1a. Result of the original design (eg. a case wasn't considered)
1b. Mistake during implementation
1c. Backend bug
1z. Other:

Where bug was reported:

2a. Reported on production
2b. Reported on staging (deploy blocker)
2c. Reported on a PR
2z. Other:

Who reported the bug:

[Contributor] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake.

Link to comment: https://github.com/Expensify/App/pull/36553/files#r1933927246
[Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in https://app.slack.com/client/E047TPA624F/C01GTK53T8Q has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner.

Link to discussion: N/A
[Contributor] If it was decided to create a regression test for the bug, please propose the https://github.com/Expensify/App/blob/main/contributingGuides/REGRESSION_TEST_BEST_PRACTICES.md steps using the template below to ensure the same bug will not reach production again. Yes

Regression Test Proposal Template

[BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

Link to issue:

Regression Test Proposal

Test:

Set up before executing test 1, and 2

Go to staging.new.expensify.com
Create a workspace
Go to workspace settings > Report fields (enable in More features).
Add a list type report field, with some list values and an initial value.
Go to the workspace chat
Create an expense

Test 1

Go to the workspace chat
Click on the iou preview
Click on List fields
Click on the already selected value
Verify that there is NO Report_SetFields API call in Dev tools > Networks tab
Navigate to another chat
Navigate back to the expense report details page
Verify that there is no system message for selecting the same report field value

Test 2

Go to the workspace chat
Click on the iou preview
Click on List fields
Select another list field value
Verify that there is Report_SetFields API call in Dev tools > Networks tab
Navigate to another chat
Navigate back to the expense report details page
Verify that there is a system message with a format changed list fields from A to B or changed list fields to C

Do we agree 👍 or 👎

dukenv0307 · 2025-01-29T13:56:11Z

@bfitzexpensify I completed the BZ. Thank for bumping me

bfitzexpensify · 2025-01-29T14:58:17Z

Thanks. Proposed the regression test steps, and finalised payment.

IuliiaHerets added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Aug 8, 2024

melvin-bot bot assigned bfitzexpensify Aug 8, 2024

melvin-bot bot added the Overdue label Aug 12, 2024

bfitzexpensify added the External Added to denote the issue can be worked on by a contributor label Aug 12, 2024

melvin-bot bot changed the title ~~Report fields -System message shows undefined when list value is changed back to initial value~~ [$250] Report fields -System message shows undefined when list value is changed back to initial value Aug 12, 2024

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 12, 2024

melvin-bot bot assigned dukenv0307 Aug 12, 2024

melvin-bot bot removed the Overdue label Aug 12, 2024

This comment was marked as outdated.

Sign in to view

melvin-bot bot assigned grgia Aug 12, 2024

bfitzexpensify added this to [#whatsnext] #expense Aug 12, 2024

melvin-bot bot removed the Overdue label Jan 8, 2025

grgia assigned etCoderDysto Jan 8, 2025

etCoderDysto mentioned this issue Jan 11, 2025

fix: System message shows undefined when selecting an already selected list value #55118

Merged

50 tasks

melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Weekly KSv2 labels Jan 11, 2025

melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Jan 17, 2025

melvin-bot bot changed the title ~~[$250] Report fields -System message shows undefined when list value is changed back to initial value~~ [HOLD for payment 2025-01-24] [$250] Report fields -System message shows undefined when list value is changed back to initial value Jan 17, 2025

melvin-bot bot removed the Reviewing Has a PR in review label Jan 17, 2025

garrettmknight moved this from Bugs and Follow Up Issues to Hold for Payment in [#whatsnext] #expense Jan 21, 2025

melvin-bot bot added Daily KSv2 Overdue and removed Weekly KSv2 labels Jan 24, 2025

melvin-bot bot removed the Overdue label Jan 27, 2025

dukenv0307 mentioned this issue Jan 29, 2025

feat: add policyReportFields to the policy object directly #36553

Merged

50 tasks

bfitzexpensify closed this as completed Jan 29, 2025

github-project-automation bot moved this from Hold for Payment to Done in [#whatsnext] #expense Jan 29, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD for payment 2025-01-24] [$250] Report fields -System message shows undefined when list value is changed back to initial value #47067

[HOLD for payment 2025-01-24] [$250] Report fields -System message shows undefined when list value is changed back to initial value #47067

IuliiaHerets commented Aug 8, 2024 •

edited by bfitzexpensify

Loading

melvin-bot bot commented Aug 8, 2024

IuliiaHerets commented Aug 8, 2024

IuliiaHerets commented Aug 8, 2024

etCoderDysto commented Aug 8, 2024 •

edited

Loading

nkdengineer commented Aug 8, 2024 •

edited

Loading

nkdengineer commented Aug 8, 2024

etCoderDysto commented Aug 8, 2024 •

edited

Loading

melvin-bot bot commented Aug 12, 2024

melvin-bot bot commented Aug 12, 2024

This comment was marked as outdated.

melvin-bot bot commented Aug 12, 2024

etCoderDysto commented Aug 12, 2024 •

edited

Loading

mountiny commented Aug 12, 2024

etCoderDysto commented Aug 12, 2024

mountiny commented Aug 12, 2024

nkdengineer commented Aug 12, 2024

etCoderDysto commented Aug 12, 2024 •

edited

Loading

nkdengineer commented Aug 12, 2024

etCoderDysto commented Aug 12, 2024 •

edited

Loading

melvin-bot bot commented Jan 8, 2025

etCoderDysto commented Jan 11, 2025

melvin-bot bot commented Jan 17, 2025

melvin-bot bot commented Jan 17, 2025

melvin-bot bot commented Jan 17, 2025

melvin-bot bot commented Jan 27, 2025

bfitzexpensify commented Jan 27, 2025

bfitzexpensify commented Jan 27, 2025

dukenv0307 commented Jan 29, 2025 •

edited

Loading

dukenv0307 commented Jan 29, 2025

bfitzexpensify commented Jan 29, 2025

[HOLD for payment 2025-01-24] [$250] Report fields -System message shows undefined when list value is changed back to initial value #47067

[HOLD for payment 2025-01-24] [$250] Report fields -System message shows undefined when list value is changed back to initial value #47067

Comments

IuliiaHerets commented Aug 8, 2024 • edited by bfitzexpensify Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Aug 8, 2024

IuliiaHerets commented Aug 8, 2024

IuliiaHerets commented Aug 8, 2024

etCoderDysto commented Aug 8, 2024 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

nkdengineer commented Aug 8, 2024 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

nkdengineer commented Aug 8, 2024

etCoderDysto commented Aug 8, 2024 • edited Loading

melvin-bot bot commented Aug 12, 2024

melvin-bot bot commented Aug 12, 2024

This comment was marked as outdated.

melvin-bot bot commented Aug 12, 2024

etCoderDysto commented Aug 12, 2024 • edited Loading

mountiny commented Aug 12, 2024

etCoderDysto commented Aug 12, 2024

mountiny commented Aug 12, 2024

nkdengineer commented Aug 12, 2024

etCoderDysto commented Aug 12, 2024 • edited Loading

nkdengineer commented Aug 12, 2024

etCoderDysto commented Aug 12, 2024 • edited Loading

melvin-bot bot commented Jan 8, 2025

etCoderDysto commented Jan 11, 2025

melvin-bot bot commented Jan 17, 2025

melvin-bot bot commented Jan 17, 2025

melvin-bot bot commented Jan 17, 2025

melvin-bot bot commented Jan 27, 2025

bfitzexpensify commented Jan 27, 2025

bfitzexpensify commented Jan 27, 2025

dukenv0307 commented Jan 29, 2025 • edited Loading

BugZero Checklist:

Regression Test Proposal

Test:

dukenv0307 commented Jan 29, 2025

bfitzexpensify commented Jan 29, 2025

IuliiaHerets commented Aug 8, 2024 •

edited by bfitzexpensify

Loading

etCoderDysto commented Aug 8, 2024 •

edited

Loading

nkdengineer commented Aug 8, 2024 •

edited

Loading

etCoderDysto commented Aug 8, 2024 •

edited

Loading

etCoderDysto commented Aug 12, 2024 •

edited

Loading

etCoderDysto commented Aug 12, 2024 •

edited

Loading

etCoderDysto commented Aug 12, 2024 •

edited

Loading

dukenv0307 commented Jan 29, 2025 •

edited

Loading