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

Prioritise results by requested locale, but return Locale and default (en_US) patterns. #307

Merged
merged 4 commits into from
Jul 20, 2021

Conversation

dd32
Copy link
Member

@dd32 dd32 commented Jul 20, 2021

Currently the Pattern directory limits the results to the given locale, this is not ideal in all situations.

Consider:

This PR changes the logic from only returning results in the matching locale, to returning in both the local and in en_US, with the localised results first.

No attempt at deduplication is done here, both the Translated and english results would be returned.
This also has the benefit that searching an English phrase would result in matching an english pattern, and not just returning an empty data set if it's not included in the locale translated patterns. Searching for a German phrase on an English or French site would however not return a matching German pattern.

See #244

Screenshots

How to test the changes in this Pull Request:

  1. Apply patch
  2. Visit URLs mentioned. Perform searches.

@dd32 dd32 added [Component] Pattern Directory API The pattern API on WordPress.org, and/or the CPT endpoint i18n Translations and internationalization of patterns labels Jul 20, 2021
@dd32 dd32 requested review from iandunn and tellyworth July 20, 2021 05:36
@dd32
Copy link
Member Author

dd32 commented Jul 20, 2021

I also note that this introduces a query alteration which will require writing some manual ElasticSearch rules when time to implement that comes along.

@iandunn
Copy link
Member

iandunn commented Jul 20, 2021

it's not uncommon to see API requests with it set to en or es which are equally invalid WP_Locale values

Some recent WPCOM discussion around that is at D64053-code

@iandunn
Copy link
Member

iandunn commented Jul 20, 2021

No attempt at deduplication is done here, both the Translated and english results would be returned.

That's not ideal for en_* locales, since e.g., the en_AU and en_US versions of About me cards will both appear.

https://en-au.wordpress.org/patterns/search/about/

We can probably de-dupe via post_parent, but that can be an iteration in another PR. I'll open an issue.

Copy link
Member

@iandunn iandunn left a comment

Choose a reason for hiding this comment

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

LGTM. I'll go ahead and merge/deploy since it's early in Australia and this'll improve results for folks upgrading to 5.8.

@iandunn iandunn merged commit 9a03a87 into WordPress:trunk Jul 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Component] Pattern Directory API The pattern API on WordPress.org, and/or the CPT endpoint i18n Translations and internationalization of patterns
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants