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

[FYST-1715] NC County: Update error message if no county selected #5704

Merged

Conversation

arinchoi03
Copy link
Contributor

Link to pivotal/JIRA issue

Is PM acceptance required? (delete one)

  • Yes - don't merge until JIRA issue is accepted!

Reminder: merge main into this branch and get green tests before merging to main

What was done?

  • Copied implementation from MD County selection
  • Removed presence check which had its built-in "can't be blank" error message
  • Since nil or "" is not included in the list of counties, it'll be able to use the new message. There is no way that a user could send an invalid county param (besides sending an API request built manually somehow?) but if they did, they would get the same error

How to test?

  • Describe the testing approach taken to verify the changes, including:
    • Unit tests
    • Test data used: Any NC persona should work

Screenshots (for visual changes)

  • Before
    image

  • After

Screenshot 2025-03-07 at 10 38 17 AM Screenshot 2025-03-07 at 10 02 17 AM

Copy link

github-actions bot commented Mar 7, 2025

Heroku app: https://gyr-review-app-5704-1248d3b6004a.herokuapp.com/
View logs: heroku logs --app gyr-review-app-5704 (optionally add --tail)

@arinchoi03 arinchoi03 marked this pull request as ready for review March 7, 2025 19:56
Copy link
Contributor

@anisharamnani anisharamnani left a comment

Choose a reason for hiding this comment

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

@arinchoi03 arinchoi03 merged commit 5fa1662 into main Mar 10, 2025
7 checks passed
@arinchoi03 arinchoi03 deleted the FYST-1715-nc-update-error-message-when-no-county-selected branch March 10, 2025 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants