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

Enforce consistency in the /pd/international_workshop country field stored values #41992

Merged
merged 2 commits into from
Aug 24, 2021

Conversation

smusoke
Copy link
Contributor

@smusoke smusoke commented Aug 11, 2021

What
Country values within the PD InternationalOptIn form are titleized during form creation.

Why
To have the values stored in a capitalized format and be consistent with other stored country values across our database (ex. user_geos,form_geos).

Links

Testing story

  • Tested locally
    Completed local form with no changes and one with these changes. The stored country value was successfully capitalized as intended.
Screen.Recording.2021-08-11.at.4.59.18.PM.mov

Follow-up work

As mentioned in this ticket we need to clean up old country values from being in other languages/translated incorrectly.

PR Checklist:

  • Tests provide adequate coverage
  • Privacy and Security impacts have been assessed
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

Copy link
Contributor

@annaxuphoto annaxuphoto left a comment

Choose a reason for hiding this comment

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

Looks great! Can you add a quick comment to explain that we want to capitalize country names?

@smusoke
Copy link
Contributor Author

smusoke commented Aug 12, 2021

Looks great! Can you add a quick comment to explain that we want to capitalize country names?

@annaxuphoto Updated the why section to be clearer on the capitalization.

@annaxuphoto
Copy link
Contributor

Looks great! Can you add a quick comment to explain that we want to capitalize country names?

@annaxuphoto Updated the why section to be clearer on the capitalization.

Thanks! I more so meant, could you put a comment in the code so anyone trying to understand what's going on there knows?

Also, I took another look at the Jira task and am a little confused on what Bryan is asking, so maybe hold off on merging until he's back and can approve himself @dju90

@smusoke
Copy link
Contributor Author

smusoke commented Aug 12, 2021

Looks great! Can you add a quick comment to explain that we want to capitalize country names?

@annaxuphoto Updated the why section to be clearer on the capitalization.

Thanks! I more so meant, could you put a comment in the code so anyone trying to understand what's going on there knows?

Also, I took another look at the Jira task and am a little confused on what Bryan is asking, so maybe hold off on merging until he's back and can approve himself @dju90

Ohh yes that's a good idea. I'll hold off until Monday 👍

@dju90
Copy link
Contributor

dju90 commented Aug 17, 2021

LGTM! @annaxuphoto for clarification, we want the first letter of the country names to be capitalized in the database (which is how user_geos and form_geos store them) (and retain the current behavior where the country names are still stored in English even if the user fills out the form in non-EN)

@smusoke smusoke merged commit 416d5b2 into staging Aug 24, 2021
@smusoke smusoke deleted the fnd-1232-capitalize-int-workshop-country-field branch August 24, 2021 16:19
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.

None yet

4 participants