Skip to content

LG-214 Update remember device translations#2130

Merged
davemcorwin merged 3 commits intomasterfrom
LG-214-remember-device-translations
May 4, 2018
Merged

LG-214 Update remember device translations#2130
davemcorwin merged 3 commits intomasterfrom
LG-214-remember-device-translations

Conversation

@davemcorwin
Copy link
Copy Markdown
Contributor

WHY

  • To be more accurate by specifying browser instead of device
  • To be more helpful to our spanish and french speaking friends

Hi! Before submitting your PR for review, and/or before merging it, please
go through the following checklist:

  • For DB changes, check for missing indexes, check to see if the changes
    affect other apps (such as the dashboard), make sure the DB columns in the
    various environments are properly populated, coordinate with devops, plan
    migrations in separate steps.

  • For route changes, make sure GET requests don't change state or result in
    destructive behavior. GET requests should only result in information being
    read, not written.

  • For encryption changes, make sure it is compatible with data that was
    encrypted with the old code.

  • For secrets changes, make sure to update the S3 secrets bucket with the
    new configs in all environments.

  • Do not disable Rubocop or Reek offenses unless you are absolutely sure
    they are false positives. If you're not sure how to fix the offense, please
    ask a teammate.

  • When reading data, write tests for nil values, empty strings,
    and invalid formats.

  • When calling redirect_to in a controller, use _url, not _path.

  • When adding user data to the session, use the user_session helper
    instead of the session helper so the data does not persist beyond the user's
    session.

  • When adding a new controller that requires the user to be fully
    authenticated, make sure to add before_action :confirm_two_factor_authenticated.

**WHY**
- To be more accurate by specifying `browser` instead of `device`
- To be more helpful to our spanish and french speaking friends
@davemcorwin davemcorwin changed the title Update remember device translations LG-214 Update remember device translations May 1, 2018
messages:
current_address: You should be be able to receive mail at this address.
remember_device: Remember this device for %{duration} days
remember_device: Remember this browser for %{duration} days
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does it make sense to day "Remember this browser"? That makes it sound like we are not requiring their username / password next time. Maybe something along the lines of "Remember this phone on this browser" would make more sense?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I agree, it does sound like that, we would probably have to add more of an explanation for what we are remembering.

However, I just implemented what was in the Jira ticket https://cm-jira.usa.gov/browse/LG-214, so I'll defer to @andrewhughey .

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, let's keep the discussion in that ticket.

### Managing translation files

To help us handle extra newlines and make sure we wrap lines consistently, we have a script called `./script/normalize-yaml` that helps format YAML consistently. After importing translations (or making changes to the *.yml files with strings, run this for the IDP app:
To help us handle extra newlines and make sure we wrap lines consistently, we have a script called `./scripts/normalize-yaml` that helps format YAML consistently. After importing translations (or making changes to the *.yml files with strings, run this for the IDP app:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we replace the reference to scripts/normalize-yaml with make normalize_yaml since that is the recommended command to run?

Copy link
Copy Markdown
Contributor Author

@davemcorwin davemcorwin May 1, 2018

Choose a reason for hiding this comment

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

the diff doesn't tell the whole story :) immediately below that paragraph in the README it says

$ make normalize_yaml

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I know. I added that to the README a while back because people didn't know which command to run or whether or not it needed arguments. Looking back, I should have updated the paragraph above as well, so I thought since you're in there now, might as well do it.

current_address: Vous devriez être en mesure de recevoir du courrier à cette
adresse.
remember_device: NOT TRANSLATED YET
remember_device: Rappelez-vous ce navigateur pendant %{duration} jours
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Enregistrer ce navigateur pendant %{duration} jours

@davemcorwin
Copy link
Copy Markdown
Contributor Author

@monfresh PTAL

Copy link
Copy Markdown
Contributor

@monfresh monfresh left a comment

Choose a reason for hiding this comment

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

LGTM

@davemcorwin davemcorwin merged commit 201a5ad into master May 4, 2018
@davemcorwin davemcorwin deleted the LG-214-remember-device-translations branch May 4, 2018 23:56
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.

4 participants