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-1697] update StateInformationService to be locale aware #5440

Merged
merged 4 commits into from
Jan 28, 2025

Conversation

jnf
Copy link
Contributor

@jnf jnf commented Jan 24, 2025

Link to pivotal/JIRA issue

FYST-1697

Is PM acceptance required? (delete one)

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

What was done?

instead of the StateInformationService knowing a state's name and taxation department, it now knows where to find the name and taxation department in our locale files. this lets us easily localize those proper nouns (if/when we want to) the same way we manage all other localized content. this is similar to the 'sin' value in the StateInformationService; it's not the software identifier itself, rather it's where to find it.

Slack threads with more context

How to test?

  • Ensure page contents are properly localized when switching locales. Look for content strings that bring in a state_name parameter.

Screenshots (for visual changes)

Before

image

After

image

…ation department, it now knows where to _find_ the name and taxation department in our locale files. this lets us easily localize those proper nouns (if/when we want to) the same way we manage all other localized content. this is similar to the 'sin' value in the StateInformationService; it's not the software identifier itself, rather it's where to find it.
@jnf jnf added the wip denotes a work in progress that isn't ready for formal review label Jan 24, 2025
Copy link

Heroku app: https://gyr-review-app-5440-b91f6a470f50.herokuapp.com/
View logs: heroku logs --app gyr-review-app-5440 (optionally add --tail)

@jnf jnf changed the title [wip] update StateInformationService to be locale aware [FYST-1697] update StateInformationService to be locale aware Jan 24, 2025
@jnf jnf marked this pull request as ready for review January 24, 2025 21:55
Copy link
Contributor

@mrotondo mrotondo left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for doing this!

@jnf jnf added in-acceptance and removed wip denotes a work in progress that isn't ready for formal review labels Jan 24, 2025
@jnf jnf merged commit c9b8fa8 into main Jan 28, 2025
7 checks passed
@jnf jnf deleted the jnf/spike/localize_proper_nouns branch January 28, 2025 17:00
Copy link

sentry-io bot commented Jan 29, 2025

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ InvalidStateCodeError: Invalid state code: NC (InvalidStateCodeError) StateFile::LandingPageController#edit View Issue

Did you find this useful? React with a 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants