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

Fixes #16436. Allow filtering of indexable creation. #18222

Open
wants to merge 4 commits into
base: trunk
Choose a base branch
from

Conversation

NielsdeBlaauw
Copy link
Contributor

Summary

This changeset standardises the order in which the type of the indexable page is determined. It also adds two filters, so developers can manipulate the order and logic with which the indexables are created far easier than is currently the case.

This PR can be summarized in the following changelog entry: changelog:bugfix, changelog: enhancement

  • Fixes a bug where no SEO title would be found when a user visits a post type archive filtered by a taxonomy term.
  • Adds two filters: wpseo_frontend_page_type_mapping and wpseo_frontend_indexable_mapping. These filters allow developers to change the order and logic of the creation of indexables.

Relevant technical choices:

  • I've limited the changes to the methods that already exist with this logic. It seems overkill to seperate these checks out into different files.
  • I've normalised the order of the checks of both functions. This possibly fixes a similair issue when a custom search result page is used.
  • I've prioritised the term_archive line over the post_type_archive line, as I think in most cases the term archive title will be more specific and valuable than the post archive title.
  • static_posts_page seems to be missing from the Indexable_Repository::for_current_page method. It is unclear if this is intentional.

Test instructions

Test instructions for the acceptance test before the PR gets merged

This PR can be acceptance tested by following these steps:

Test instructions for QA when the code is in the RC

  • QA should use the same steps as above.

UI changes

  • This PR changes the UI in the plugin. I have added the 'UI change' label to this PR.

Other environments

  • This PR also affects Shopify. I have added a changelog entry starting with [shopify-seo] and I have added test instructions for Shopify.

Documentation

  • I have written documentation for this change.

Quality assurance

  • I have tested this code to the best of my abilities
  • I have added unittests to verify the code works as intended
  • If any part of the code is behind a feature flag, my test instructions also cover cases where the feature flag is switched off.

Fixes #16436

Also standardises order of checks to match `Current_Page_Helper::get_page_type`.

Note: `static_posts_page` seems to be missing from the `Indexable_Repository::for_current_page` method. It is unclear if this is intentional.
@nlemoine
Copy link
Contributor

nlemoine commented Mar 18, 2022

Thanks @NielsdeBlaauw! I would love to see this kind of filters happen. It's now quite complicated to make WP SEO behave right when not fitting in one of the contexts already defined.

@enricobattocchi enricobattocchi self-assigned this Sep 26, 2022
@enricobattocchi enricobattocchi removed their assignment Nov 18, 2022
@enricobattocchi
Copy link
Member

@NielsdeBlaauw sorry for the delay! We'll try to have a look in the upcoming weeks, we're working on a major improvements of our indexables creation and we need to check carefully how this could interact. I hope we can move it further soon.

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.

Yoast Title in custom post archive-posttype.php is blank when add more query string
4 participants