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

[READY FOR PAYMENT][$125] Report fields - List type report field shows "List is required" when there is an initial value #46223

Closed
6 tasks done
lanitochka17 opened this issue Jul 25, 2024 · 37 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Monthly KSv2 Reviewing Has a PR in review

Comments

@lanitochka17
Copy link

lanitochka17 commented Jul 25, 2024

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.12-0
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Issue found when executing PR #44139

Action Performed:

Precondition:

  • Workspace is under Control plan
  • Report fields feature is enabled
  • List type report field is added, and the list has an initial value
  1. Go to workspace chat
  2. Split a manual expense
  3. Go to Troubleshoot > Clear cache and restart > Reset and restart
  4. Go to workspace chat
  5. Split another manual expense (because violations do not show up instantly)
  6. Go to expense report

Expected Result:

List type report field will not display "List is required" violation because there is an initial value

Actual Result:

List type report field displays "List is required" violation when there is an initial value

Workaround:

Unknown

Platforms:

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

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6552686_1721913687120.20240725_212026.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01fa66587c84d7c165
  • Upwork Job ID: 1823770004376025525
  • Last Price Increase: 2024-08-14
  • Automatic offers:
    • nkdengineer | Contributor | 103527627
Issue OwnerCurrent Issue Owner: @JmillsExpensify
@lanitochka17 lanitochka17 added DeployBlockerCash This issue or pull request should block deployment DeployBlocker Indicates it should block deploying the API labels Jul 25, 2024
Copy link

melvin-bot bot commented Jul 25, 2024

Triggered auto assignment to @Beamanator (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

Copy link
Contributor

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@francoisl
Copy link
Contributor

@thienlnam is this also something about report fields we can polish with a follow-up PR?

@Beamanator Beamanator assigned thienlnam and unassigned Beamanator Jul 25, 2024
@Beamanator
Copy link
Contributor

Assigning jack since this is part of your Report Field Violations Project 🙏

@Beamanator
Copy link
Contributor

Beamanator commented Jul 25, 2024

Also if this is under beta we can demote, is there a report fields beta? reportFieldsFeature i think?

@thienlnam
Copy link
Contributor

Yeah, we definitely can address - @jnowakow another one to fix

@thienlnam
Copy link
Contributor

@Beamanator That beta is for setting up report fields in NewDot - slightly unrelated to showing them, but I don't think this needs to be a blocker

@thienlnam thienlnam added Daily KSv2 and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 DeployBlocker Indicates it should block deploying the API labels Jul 25, 2024
@nkdengineer
Copy link
Contributor

Proposal

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

List type report field displays "List is required" violation when there is an initial value

What is the root cause of that problem?

After we reset the cache the field list has value is undefined since we use the defaultValue when we create the expense report.

App/src/libs/actions/IOU.ts

Lines 862 to 867 in 04741f4

if (excludedFields.includes(field.fieldID) || !!field.value) {
return;
}
// in case of missing field violation the empty object is indicator.
missingFields[field.fieldID] = {};
});

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

We should return early if the defaultValue is not empty

if (excludedFields.includes(field.fieldID) || !!field.value || !!field.defaultValue) { 
   return; 
}

App/src/libs/actions/IOU.ts

Lines 862 to 867 in 04741f4

if (excludedFields.includes(field.fieldID) || !!field.value) {
return;
}
// in case of missing field violation the empty object is indicator.
missingFields[field.fieldID] = {};
});

Or BE should return the value of fieldList as defaultValue after we create the expense report and don't change any report field.

What alternative solutions did you explore? (Optional)

@thienlnam
Copy link
Contributor

👋 @war-in Could we also address this issue with #46215?

@thienlnam thienlnam added the Bug Something is broken. Auto assigns a BugZero manager. label Jul 26, 2024
Copy link

melvin-bot bot commented Jul 26, 2024

Triggered auto assignment to @JmillsExpensify (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 melvin-bot bot added the Overdue label Jul 29, 2024
@war-in
Copy link
Contributor

war-in commented Jul 29, 2024

Hi 👋 @thienlnam sure! I'll take care of it

@thienlnam
Copy link
Contributor

cc @JmillsExpensify Could we please get a payment to @nkdengineer for $125?

@nkdengineer
Copy link
Contributor

@thienlnam Do we need to assign me here for payment?

Copy link

melvin-bot bot commented Aug 14, 2024

📣 @nkdengineer 🎉 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 📖

Copy link

melvin-bot bot commented Aug 21, 2024

@JmillsExpensify, @thienlnam, @war-in, @nkdengineer Whoops! This issue is 2 days overdue. Let's get this updated quick!

Copy link

melvin-bot bot commented Aug 23, 2024

@JmillsExpensify, @thienlnam, @war-in, @nkdengineer Eep! 4 days overdue now. Issues have feelings too...

Copy link

melvin-bot bot commented Aug 27, 2024

@JmillsExpensify, @thienlnam, @war-in, @nkdengineer 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

@thienlnam thienlnam added the Awaiting Payment Auto-added when associated PR is deployed to production label Aug 27, 2024
@thienlnam thienlnam changed the title [$125] Report fields - List type report field shows "List is required" when there is an initial value [READY FOR PAYMENT][$125] Report fields - List type report field shows "List is required" when there is an initial value Aug 27, 2024
@thienlnam
Copy link
Contributor

Bump payment for this one cc @JmillsExpensify
#46223 (comment)

@melvin-bot melvin-bot bot added Daily KSv2 and removed Daily KSv2 labels Sep 3, 2024
Copy link

melvin-bot bot commented Sep 10, 2024

@JmillsExpensify, @thienlnam, @war-in, @nkdengineer Whoops! This issue is 2 days overdue. Let's get this updated quick!

Copy link

melvin-bot bot commented Sep 12, 2024

@JmillsExpensify, @thienlnam, @war-in, @nkdengineer Eep! 4 days overdue now. Issues have feelings too...

Copy link

melvin-bot bot commented Sep 16, 2024

@JmillsExpensify, @thienlnam, @war-in, @nkdengineer Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

Copy link

melvin-bot bot commented Sep 18, 2024

@JmillsExpensify, @thienlnam, @war-in, @nkdengineer 10 days overdue. I'm getting more depressed than Marvin.

Copy link

melvin-bot bot commented Sep 20, 2024

@JmillsExpensify, @thienlnam, @war-in, @nkdengineer 12 days overdue. Walking. Toward. The. Light...

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Sep 23, 2024
Copy link

melvin-bot bot commented Sep 23, 2024

This issue has not been updated in over 14 days. @JmillsExpensify, @thienlnam, @war-in, @nkdengineer eroding to Weekly issue.

@nkdengineer
Copy link
Contributor

@JmillsExpensify Gentle bump on #46223 (comment)

TIA

@melvin-bot melvin-bot bot added Monthly KSv2 and removed Weekly KSv2 labels Oct 16, 2024
Copy link

melvin-bot bot commented Oct 16, 2024

This issue has not been updated in over 15 days. @JmillsExpensify, @thienlnam, @war-in, @nkdengineer eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

@nkdengineer
Copy link
Contributor

@JmillsExpensify Gentle bump on #46223 (comment)

TIA

@JmillsExpensify
Copy link

Ah sorry I missed this in my filters as the payment has no date.

@JmillsExpensify
Copy link

All paid out. Closing this one.

@github-project-automation github-project-automation bot moved this from Hold for Payment to Done in [#whatsnext] #expense Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Monthly KSv2 Reviewing Has a PR in review
Projects
Status: Done
Development

No branches or pull requests

8 participants