Skip to content

Add tests to check for matching HTML tags across locales #8847

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.

Sign up for GitHub

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

Merged
aduth merged 1 commit into from
Jul 25, 2023
Merged

Add tests to check for matching HTML tags across locales #8847

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions config/locales/idv/en.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ en:
zipcode: Enter a 5 or 9 digit ZIP Code
failure:
attempts_html:
one: For security reasons, you have <strong>one attempt<strong> remaining to add
your ID online.
other: For security reasons, you have <strong>%{count} attempts<strong>
one: For security reasons, you have <strong>one attempt</strong> remaining to
add your ID online.
other: For security reasons, you have <strong>%{count} attempts</strong>
remaining to add your ID online.
button:
try_online: Try again online
Expand Down Expand Up @@ -280,7 +280,7 @@ en:
state_id:
cancel_button: Exit %{app_name}
explanation: |
Unfortunately, we’re experiencing technical difficulties
Unfortunately, we’re experiencing technical difficulties
with IDs from your state and are currently unable to verify your information.
heading: We are working to resolve an error
next_steps:
Expand Down
2 changes: 1 addition & 1 deletion config/locales/idv/fr.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ fr:
how_long_it_takes: Cette procédure prend typiquement 3 à 7 jours ouvrables.
heading: Nous n’avons pas pu vous associer à ce numéro
learn_more_link: 'Apprenez-en plus sur quel numéro de téléphone utiliser'
next_steps_html: 'Essayez <strtong>un autre<strong> numéro que vous utilisez
next_steps_html: 'Essayez <strong>un autre</strong> numéro que vous utilisez
Copy link
Copy Markdown
Contributor

@zachmargolis zachmargolis Jul 24, 2023

Choose a reason for hiding this comment

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

wow yikes glad we're making this linter

souvent et depuis longtemps. Il peut s’agir d’un numéro
professionnel ou personnel.'
try_again_button: 'Essayez un autre numéro'
Expand Down
12 changes: 6 additions & 6 deletions config/locales/instructions/fr.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,12 @@ fr:
votre mot de passe. Ajoutez ensuite votre PIV/CAC à votre compte.</p>
<p><strong>Vous n’avez pas de compte %{app_name}?</strong>
%{create_account}</p>'
add_from_sign_in_html: '<strong> Instructions: </ strong> insérez votre PIV ou
votre CAC dans <strong> “AJOUTER PIV / CAC” </ strong>. Vous devez
<strong> choisir un certificat </ strong> (le bon en a probablement
votre nom) et <strong> saisir votre code confidentiel </ strong>
(votre code confidentiel a été créé lors de la configuration de votre
PIV / CAC). ).'
add_from_sign_in_html: '<strong> Instructions: </strong> insérez votre PIV ou
votre CAC dans <strong> “AJOUTER PIV / CAC” </strong>. Vous devez
<strong> choisir un certificat </strong> (le bon en a probablement
votre nom) et <strong> saisir votre code confidentiel </strong> (votre
code confidentiel a été créé lors de la configuration de votre PIV /
CAC). ).'
already_associated_html: Veuillez choisir un certificat associé à une autre
carte PIV/CAC ou contactez votre administrateur afin de vérifier que
votre carte PIV/CAC est bien à jour. Si vous pensez que c’est une
Expand Down
2 changes: 1 addition & 1 deletion config/locales/user_mailer/en.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ en:
help, please visit the %{app_name_html} %{help_link_html} or
%{contact_link_html}.
info_html: '<p>Your %{app_name} account was just used to sign in on a new
device.</p><br/> <p><strong>%{date}<br/>%{location}</strong></p>'
device.</p><br/><p><strong>%{date}<br/>%{location}</strong></p>'
subject: New sign-in with your %{app_name} account
password_changed:
disavowal_link: reset your password
Expand Down
14 changes: 7 additions & 7 deletions config/locales/user_mailer/es.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,7 @@ es:
help_html: Si no realizó este cambio, %{disavowal_link_html}. Para más ayuda,
visite el %{app_name_html} %{help_link_html} o el %{contact_link_html}.
info_html: '<p>Su cuenta %{app_name} acaba de iniciar sesión en un nuevo
dispositivo.</p><br/><br/>
<p><strong>%{date}<br/>%{location}</strong></p>'
dispositivo.</p><br/><p><strong>%{date}<br/>%{location}</strong></p>'
subject: Nuevo initio de sesion con su %{app_name} cuenta
password_changed:
disavowal_link: restablecer su contraseña
Expand Down Expand Up @@ -256,11 +255,12 @@ es:
%{app_name}</a></strong> y asegúrese de reconocer toda la información en
la página de su cuenta, incluidos los métodos que utiliza para la
autenticación de dos factores, como el número de teléfono, la aplicación
de autenticación, o la clave de seguridad.</li><li><strong>En la página
de su cuenta %{app_name}, solicite una nueva clave personal.</strong>
Recuerde, nunca lo comparta a menos que lo esté utilizando para iniciar
sesión en un sitio web de confianza que utiliza
%{app_name}.</li></ol></p><br>Gracias,<br>El equipo de %{app_name}
de autenticación, o la clave de seguridad.</li><li><strong>En <a
href="%{account_url}">la página de su cuenta %{app_name}</a>, solicite
Comment on lines +258 to +259
Copy link
Copy Markdown
Contributor Author

@aduth aduth Jul 24, 2023

Choose a reason for hiding this comment

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

English version has a third link "your Login.gov account page", which was not reflected in the other locales:

identity-idp/config/locales/user_mailer/en.yml

Lines 242 to 243 in 0f21a2a

security key.</li><li><strong>On your <a
href="%{account_url}">%{app_name} account page</a>, request a new

Copy link
Copy Markdown
Contributor

@zachmargolis zachmargolis Jul 24, 2023

Choose a reason for hiding this comment

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

this is a good candidate for one translation string that should be broken into 2-3 and we should move the markup into the template IMO

Copy link
Copy Markdown
Contributor Author

@aduth aduth Jul 25, 2023

Choose a reason for hiding this comment

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

Yeah, the email templates in general could use a lot of love 😬 Not going to try to tackle that here.

una nueva clave personal.</strong> Recuerde, nunca lo comparta a menos
que lo esté utilizando para iniciar sesión en un sitio web de confianza
que utiliza %{app_name}.</li></ol></p><br>Gracias,<br>El equipo de
%{app_name}
intro: Clave personal utilizada para iniciar sesión
subject: Alerta de seguridad de cuenta
phone_added:
Expand Down
10 changes: 5 additions & 5 deletions config/locales/user_mailer/fr.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ fr:
Pour plus d’aide, veuillez visiter le %{help_link_html} de
%{app_name_html} ou %{contact_link_html}.
info_html: '<p>Votre compte %{app_name} a été connecté sur un nouvel
appareil.</p><br/><br/> <p><strong>%{date}<br/>%{location}</strong></p>'
appareil.</p><br/><p><strong>%{date}<br/>%{location}</strong></p>'
subject: Nouvelle connexion avec votre compte %{app_name}
password_changed:
disavowal_link: réinitialiser votre mot de passe
Expand Down Expand Up @@ -265,10 +265,10 @@ fr:
informations de la page de votre compte, y compris les méthodes que vous
utilisez pour l’authentification à deux facteurs, telles que le numéro
de téléphone, l’application d’authentification ou la clé de
sécurité.</li><li><strong>Sur la page de votre compte %{app_name},
demandez une nouvelle clé personnelle.</strong> N’oubliez pas de ne
jamais le partager à moins que vous ne l’utilisiez pour vous connecter à
un site Web de confiance utilisant
sécurité.</li><li><strong>Sur <a href="%{account_url}">la page de votre
compte %{app_name}</a>, demandez une nouvelle clé personnelle.</strong>
N’oubliez pas de ne jamais le partager à moins que vous ne l’utilisiez
pour vous connecter à un site Web de confiance utilisant
%{app_name}.</li></ol></p><br>Merci,<br>L’équipe %{app_name}
intro: La clé personnelle utilisée pour vous connecter
subject: Alerte de sécurité du compte
Expand Down
12 changes: 12 additions & 0 deletions spec/i18n_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,18 @@ def allowed_untranslated_key?(locale, key)
expect(missing_interpolation_argument_keys).to be_empty
end

it 'has matching HTML tags' do
i18n.data[i18n.base_locale].select_keys do |key, _node|
if key.start_with?('i18n.transliterate.rule.') || i18n.t(key).is_a?(Array) || i18n.t(key).nil?
next
end

html_unique_tags = i18n.locales.map { |locale| i18n.t(key, locale)&.scan(/<.+?>/) }.uniq

expect(html_unique_tags.size).to eq(1), "HTML tag mismatch for key #{key}"
Comment on lines +116 to +118
Copy link
Copy Markdown
Contributor

@zachmargolis zachmargolis Jul 24, 2023

Choose a reason for hiding this comment

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

maybe we can change the structure of this or the message to indicate what the tags are?

Suggested change
html_unique_tags = i18n.locales.map { |locale| i18n.t(key, locale)&.scan(/<.+?>/) }.uniq
expect(html_unique_tags.size).to eq(1), "HTML tag mismatch for key #{key}"
html_unique_tags = i18n.locales.map { |locale| i18n.t(key, locale)&.scan(/<.+?>/) }.uniq
expect(html_unique_tags.size).to eq(1), "HTML tag mismatch for key: #{key}, tags: #{html_unique_tags}"

Copy link
Copy Markdown
Contributor Author

@aduth aduth Jul 24, 2023

Choose a reason for hiding this comment

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

Yeah, I actually added that locally to help troubleshoot the existing issues, so probably good to include something similar, though hopefully moving forward it should be obvious enough to the person adding the new locale strings that there might be a problem with their tags.

As proposed, it wasn't especially helpful to debugging some of the more gnarly strings like the one you mentioned, since there's so many tags that it's hard to parse and make use of the output.

Copy link
Copy Markdown
Contributor Author

@aduth aduth Jul 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Since I happen to have the history in my terminal, here's what it ended up looking like for that one 🙈 (this was before I uniq'd it, but not a whole lot better even after)

  1) I18n has matching HTML tags
     Failure/Error: expect(html_tags.size).to eq(1), "HTML tag mismatch for key #{key}: #{html_tags}"
       HTML tag mismatch for key user_mailer.personal_key_sign_in.help_html: [["<p>", "</p>", "<p>", "</p>", "<p>", "<ol>", "<li>", "<strong>", "<a href=\"%{reset_password_url}\">", "</a>", "</strong>", "</li>", "<li>", "<strong>", "<a href=\"%{account_url}\">", "</a>", "</strong>", "</li>", "<li>", "<strong>", "<a href=\"%{account_url}\">", "</a>", "</strong>", "</li>", "</ol>", "</p>", "<br>", "<br>"], ["<p>", "</p>", "<p>", "</p>", "<p>", "<ol>", "<li>", "<strong>", "<a href=\"%{reset_password_url}\">", "</a>", "</strong>", "</li>", "<li>", "<strong>", "<a href=\"%{account_url}\">", "</a>", "</strong>", "</li>", "<li>", "<strong>", "</strong>", "</li>", "</ol>", "</p>", "<br>", "<br>"]]

Copy link
Copy Markdown
Contributor

@zachmargolis zachmargolis Jul 24, 2023

Choose a reason for hiding this comment

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

ah I see your point. I guess it's fine as-is and having the key name is enough to debug

end
end

root_dir = File.expand_path(File.join(File.dirname(__FILE__), '../'))

Dir[File.join(root_dir, '/config/locales/**')].sort.each do |group_path|
Expand Down