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

New Page Layout Picker: refresh page pattern cache when site lang changes #51087

Merged

Conversation

p-jackson
Copy link
Member

@p-jackson p-jackson commented Mar 16, 2021

Changes proposed in this Pull Request

  • Change pattern cache key to use site locale instead of account/user locale
  • Update execution order to ensure the get_iso_639_locale() function is available when __construct() is called

After changing the site language the layout names and categories in the page layout picker still show in the previous language. And when the pattern is inserted it inserts the pervious language. You'd have to wait a day after updating the site language before the picker would fix itself.

The problem is the pattern data is cached using a key that includes the account language, not the site language. This change makes sure the cache key is using the exact same locale slug that's being used in the query to get pattern data.

Testing instructions

  • install-plugin.sh etk update/refresh-page-pattern-cache-on-site-lang-change on sandbox
  • On sandbox site create a new page (layout picker will open)
  • In another tab update the site language
  • Refresh the editor tab, the modal updates to show pattern thumbnails, names and categories using the new language
  • Insert a layout, and the content added to the page should be in the new site language.

@p-jackson p-jackson added [Type] Bug When a feature is broken and / or not performing as intended [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. i18n Page Layouts labels Mar 16, 2021
@p-jackson p-jackson requested review from a team March 16, 2021 02:57
@p-jackson p-jackson self-assigned this Mar 16, 2021
@matticbot
Copy link
Contributor

@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.

Copy link
Contributor

@roo2 roo2 left a comment

Choose a reason for hiding this comment

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

Very nice fix!

Personally I think it could use a comment, I find the add_action priorities starting from 10 not intuitive. What do you think?

@ramonjd
Copy link
Member

ramonjd commented Mar 16, 2021

Works well for me 👍

There are a few e2e test fails which might be related. Are they referring to the former page picker?

NoSuchElementError: no such element: Unable to locate element: {"method":"css selector","selector":".page-template-modal__buttons .components-button.is-primary"}

@p-jackson
Copy link
Member Author

There are a few e2e test fails which might be related. Are they referring to the former page picker?

Yeah I broke tests on trunk 🤦
A rebase should fix these up.

The `get_verticals_locale()` method in `Starter_Page_Templates` depends
on the `get_ios_639_locale()` helper function, so the module with that
helper function needs to be executed first before we set up starter page
templates.
The page pattern cache was keyed on the account language. However the
account language isn't used when making the request to get the page
pattern data. Switching to the site's language (i.e. the same locale
slug used when fetching pattern data) means the cache will be
invalidated correctly.
@p-jackson p-jackson force-pushed the update/refresh-page-pattern-cache-on-site-lang-change branch from 9ec100d to f548912 Compare March 16, 2021 20:51
@p-jackson
Copy link
Member Author

e2e tests are green 🦖

The php unit tests are failing because of this issue in wp-env WordPress/gutenberg#29800 p1615778999014600-slack-C02DQP0FP

@p-jackson p-jackson merged commit dc7c845 into trunk Mar 16, 2021
@p-jackson p-jackson deleted the update/refresh-page-pattern-cache-on-site-lang-change branch March 16, 2021 21:54
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Mar 16, 2021
p-jackson added a commit that referenced this pull request Mar 16, 2021
p-jackson added a commit that referenced this pull request Mar 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
i18n Page Layouts [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants