Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.56k stars 2.9k forks source link

[HOLD for Payment 2024-10-10][$250] NetSuite - "This field is required" message displayed instead of "Please select an option" #49267

Closed IuliiaHerets closed 1 month ago

IuliiaHerets commented 2 months ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: 9.0.35-0 Reproducible in staging?: Y Reproducible in production?: Y Issue reported by: Applause Internal Team

Action Performed:

  1. Navigate to https://staging.new.expensify.com/
  2. Log in with a new Gmail account
  3. Click on FAB - New workspace
  4. Enable "Accounting" in the "More features" page.
  5. Navigate to "Accounting"
  6. Connect to NetSuite and upgrade the workspace to Control when asked
  7. Wait for the sync to finish
  8. Navigate to Accounting - Import - Custom lists
  9. Click on the "Add custom list" button
  10. Click on "Name" and select any of the options
  11. Click on the "Next" button
  12. Type any ID name and click on the "Next" button
  13. Click on the "Next" button without making a selection

Expected Result:

The message should be "Please select an option above.".

Actual Result:

"This field is required." message displayed instead of "Please select an option above." in custom list. Custom segments/records has a similar selection where the message is "Please select an option above.".

Workaround:

Unknown

Platforms:

Screenshots/Videos

https://github.com/user-attachments/assets/c684d014-7085-4c02-9a5e-de12f73740c6

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021838993006619000838
  • Upwork Job ID: 1838993006619000838
  • Last Price Increase: 2024-09-25
Issue OwnerCurrent Issue Owner: @JmillsExpensify
melvin-bot[bot] commented 2 months ago

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.

IuliiaHerets commented 2 months ago

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

IuliiaHerets commented 2 months ago

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

cretadn22 commented 2 months ago

Edited by proposal-police: This proposal was edited at 2024-09-16 14:30:28 UTC.

Proposal

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

NetSuite - "This field is required" message displayed instead of "Please select an option"

What is the root cause of that problem?

The validate function at the Mapping step is not displaying the correct error message https://github.com/Expensify/App/blob/025a63055cf63438f4b01ec1aad4537d619e2330/src/pages/workspace/accounting/netsuite/import/NetSuiteImportCustomFieldNew/NetSuiteImportAddCustomListPage.tsx#L96-L97

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

We need to update the error message to match the one in NetSuiteImportAddCustomSegmentPage

https://github.com/Expensify/App/blob/025a63055cf63438f4b01ec1aad4537d619e2330/src/pages/workspace/accounting/netsuite/import/NetSuiteImportCustomFieldNew/NetSuiteImportAddCustomSegmentPage.tsx#L126-L130

Additionally, add the shouldHideFixErrorsAlert prop to FormProvider to hide the old error message

shouldHideFixErrorsAlert={screenIndex === CONST.NETSUITE_CUSTOM_FIELD_SUBSTEP_INDEXES.CUSTOM_LISTS.MAPPING}

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

melvin-bot[bot] commented 2 months ago

@JmillsExpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

melvin-bot[bot] commented 1 month ago

@JmillsExpensify Still overdue 6 days?! Let's take care of this!

cretadn22 commented 1 month ago

@JmillsExpensify, could you please check out this issue?

melvin-bot[bot] commented 1 month ago

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

JmillsExpensify commented 1 month ago

Opened up to the community. Pending proposal review.

melvin-bot[bot] commented 1 month ago

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

mananjadhav commented 1 month ago

@cretadn22's proposal looks good to me.

🎀 👀 🎀 C+ reviewed.

melvin-bot[bot] commented 1 month ago

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

luacmartins commented 1 month ago

Shouldn't the error message be displayed under the field with the error instead of the bottom of the page as well? IIRC that's our form design

cretadn22 commented 1 month ago

@luacmartins The design is confirmed here

luacmartins commented 1 month ago

Thanks for confirming @cretadn22! In that case the proposal looks good.

melvin-bot[bot] commented 1 month ago

❌ There was an error making the offer to @cretadn22 for the Contributor role. The BZ member will need to manually hire the contributor.

cretadn22 commented 1 month ago

⚠️ Automation failed here -> this should be on [HOLD for Payment 2024-10-10]

JmillsExpensify commented 1 month ago

Payment summary:

cretadn22 commented 1 month ago

@JmillsExpensify The payment is ready

garrettmknight commented 1 month ago

$250 approved for @mananjadhav

mananjadhav commented 1 month ago

@JmillsExpensify Is the payout done for @cretadn22 ?

I think we didn't decide on the error message during the NetSuite integration. Hence, I don't think we have any offending PR. I also think we don't need a regression test for this one. Hence, once the payout is done, we can close this one out?

JmillsExpensify commented 1 month ago

Sounds good, will do.