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

Credential Form: Show error message on the correct language + cleanup #95688

Merged
merged 3 commits into from
Oct 31, 2024

Conversation

gabrielcaires
Copy link
Contributor

@gabrielcaires gabrielcaires commented Oct 24, 2024

Fix #95625 and continue the work started on Remove English-based verifications #95552

Proposed Changes

  • Remove all locale checking from the credential step
  • Add _locale flag to the ticket creation request

Context

This comment highlighted we are not showing an error message on the user language. I initially thought the reason was the missing _locale flag, but actually, I found another English checking that was making the UI show errors directly from the backend.

Now, the UI is showing correct error messages.

Testing Instructions

  • Select any locale (excluding en) in your WordPress account
  • Access any of the migration flows
  • Go to the Do it for me
  • Follow all steps until you reach the credential form
  • Set a valid WordPress site
  • Set invalids credentials
  • Check if the error messages are using the correct language.

Pre-merge Checklist

  • Has the general commit checklist been followed? (PCYsg-hS-p2)
  • Have you written new tests for your changes?
  • Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
  • Have you checked for TypeScript, React or other console errors?
  • Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
  • Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
  • For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-aUh-p2)?

@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

@gabrielcaires gabrielcaires marked this pull request as ready for review October 24, 2024 19:27
@gabrielcaires gabrielcaires self-assigned this Oct 24, 2024
@gabrielcaires gabrielcaires requested a review from a team October 24, 2024 19:27
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Oct 24, 2024
@matticbot
Copy link
Contributor

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

  • notifications
  • wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug update/remove-english-checkings on your sandbox.

@gabrielcaires gabrielcaires changed the title Site Migration: Remove all locale checking from the credential form step Credential Form: Show error message on the correct language + cleanup Oct 24, 2024
isEnglishLocale &&
code === 'automated_migration_tools_login_and_get_cookies_test_failed'
) {
if ( code === 'automated_migration_tools_login_and_get_cookies_test_failed' ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Super nitpicky, but we should reverse this conditional so the constant is on the left-hand side ("Yoda" conditions as per WP coding standards)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @sixhours.
I just double-checked if we are adopting the WP pattern, and I saw we just have
164 instances of the Yoda style
1494 instances of the regular style.

I run a search using the following patterns:
Regular style

=== '

Yoda style

' === 

The javascript WordPress code standards also contain examples of "regular" style usage.
https://developer.wordpress.org/coding-standards/wordpress-coding-standards/javascript/

So, I think the Yoda style is more applicable to PHP

@sixhours
Copy link
Contributor

Screenshot 2024-10-28 at 12 41 21 PM

Confirmed this fixes the error message translations!

@gabrielcaires gabrielcaires merged commit 785c0cb into trunk Oct 31, 2024
16 checks passed
@gabrielcaires gabrielcaires deleted the update/remove-english-checkings branch October 31, 2024 15:54
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Oct 31, 2024
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.

Site Migration: Credential Step is not showing localized error menssages from backend
3 participants