Skip to content

Make sure that the user can't select a page multiple times in the manual page selection dropdowns#22419

Merged
leonidasmi merged 6 commits intofeature/llms-txt-phase-2from
655-make-sure-that-the-user-cant-select-a-page-multiple-times-in-the-manual-page-selection-dropdowns
Jul 11, 2025
Merged

Make sure that the user can't select a page multiple times in the manual page selection dropdowns#22419
leonidasmi merged 6 commits intofeature/llms-txt-phase-2from
655-make-sure-that-the-user-cant-select-a-page-multiple-times-in-the-manual-page-selection-dropdowns

Conversation

@igorschoester
Copy link
Copy Markdown
Contributor

@igorschoester igorschoester commented Jul 9, 2025

Context

Summary

This PR can be summarized in the following changelog entry:

  • Makes sure that the user can't select a page multiple times in the manual page selection dropdowns of the LLMs.txt settings page.

Relevant technical choices:

Test instructions

Test instructions for the acceptance test before the PR gets merged

This PR can be acceptance tested by following these steps:

  • First of all, go to src\llms-txt\application\available-posts\available-posts-repository.php and edit the $available_posts = $this->automatic_post_collection->get_recent_posts( $parameters->get_post_type(), 100, $parameters->get_search_filter(), true ); line to $available_posts = $this->automatic_post_collection->get_recent_posts( $parameters->get_post_type(), 12, $parameters->get_search_filter(), true );
    • This will make our testing easier. While we normally fetch 100 pages to populate the dropdowns, now we'll fetch 12 so that we see what happens when we're ran out of pages
  • Go to LLMs.txt settings page, enable it and choose manual page selection
  • Select a page for all the options
    • Everytime you select a page for an option, that page is not shown anymore in the other dropdowns - this is the whole point of the PR
  • Click on add a new page
  • Select the new field, it should now suggest 6 pages (if your site has 12+ pages)
  • All the other fields should suggest those 6, plus the one you have already selected there
  • Add 6 more fields and select a page for each
  • Add yet one more field and see there are no more suggestions
  • Verify you got to see the "No pages found" message
    • This is not a big deal, because we actually have a limit of 100 instead of 12, so this state is super hard for the user to reach
  • Search for something that gets results (but not select it yet)
  • If you click on the other select fields, you should see the selected page plus the ones you just found as options -- verifying that the "selectable" always takes from the global list
  • If you clear a selected page, verify you get to see that one as selectable in the other fields too
  • Now revert your changes in the available-posts-repository.php file
  • You can empty your selections to start fresh
  • Start adding pages in the dropdowns and you can see that even after putting like 15-20 pages there, every time you add a new dropdown, you still have pages displayed as option to be selected there (assuming of course you have more than 20+ pages in your site).

Regression test: #22406
But in particular, the initial load should show loading indication (instead of saying no results)

Relevant test scenarios

  • Changes should be tested with the browser console open
  • Changes should be tested on different posts/pages/taxonomies/custom post types/custom taxonomies
  • Changes should be tested on different editors (Default Block/Gutenberg/Classic/Elementor/other)
  • Changes should be tested on different browsers
  • Changes should be tested on multisite

Test instructions for QA when the code is in the RC

  • QA should use the same steps as above.

QA can test this PR by following these steps:

Impact check

This PR affects the following parts of the plugin, which may require extra testing:

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], added test instructions for Shopify and attached the Shopify label to this PR.

Documentation

  • I have written documentation for this change. For example, comments in the Relevant technical choices, comments in the code, documentation on Confluence / shared Google Drive / Yoast developer portal, or other.

Quality assurance

  • I have tested this code to the best of my abilities.
  • During testing, I had activated all plugins that Yoast SEO provides integrations for.
  • I have added unit tests 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.
  • I have written this PR in accordance with my team's definition of done.
  • I have checked that the base branch is correctly set.

Innovation

  • No innovation project is applicable for this PR.
  • This PR falls under an innovation project. I have attached the innovation label.
  • I have added my hours to the WBSO document.

Fixes https://github.com/Yoast/reserved-tasks/issues/655

@igorschoester igorschoester added this to the feature/llms-txt-phase-2 milestone Jul 9, 2025
@igorschoester igorschoester added the changelog: non-user-facing Needs to be included in the 'Non-userfacing' category in the changelog label Jul 9, 2025
@coveralls
Copy link
Copy Markdown

coveralls commented Jul 9, 2025

Pull Request Test Coverage Report for Build ad2448b82495dcb34eaba09413580e9f24a3579f

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 35 (0.0%) changed or added relevant lines in 6 files are covered.
  • 16 unchanged lines in 6 files lost coverage.
  • Overall coverage decreased (-0.02%) to 53.541%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/llms-txt/application/available-posts/available-posts-repository.php 0 1 0.0%
src/integrations/settings-integration.php 0 4 0.0%
packages/js/src/settings/initialize.js 0 6 0.0%
packages/js/src/settings/routes/llms-txt.js 0 7 0.0%
packages/js/src/settings/store/indexable-pages.js 0 7 0.0%
packages/js/src/settings/components/formik-indexable-page-select-field.js 0 10 0.0%
Files with Coverage Reduction New Missed Lines %
packages/js/src/settings/store/indexable-pages.js 1 0.0%
src/integrations/settings-integration.php 2 24.76%
packages/js/src/settings/routes/llms-txt.js 2 0.0%
packages/js/src/settings/components/formik-indexable-page-select-field.js 2 0.0%
packages/js/src/introductions/components/google-docs-addon-upsell.js 4 0.0%
packages/js/src/introductions/components/content.js 5 0.0%
Totals Coverage Status
Change from base Build 60a0f7ad3ff3b9f0c14528b1218ba95847177b5e: -0.02%
Covered Lines: 30497
Relevant Lines: 57921

💛 - Coveralls

@igorschoester igorschoester marked this pull request as ready for review July 10, 2025 06:42
@igorschoester igorschoester force-pushed the 655-make-sure-that-the-user-cant-select-a-page-multiple-times-in-the-manual-page-selection-dropdowns branch from f2e31b1 to 7315ef4 Compare July 10, 2025 06:57
@github-actions
Copy link
Copy Markdown

A merge conflict has been detected for the proposed code changes in this PR. Please resolve the conflict by either rebasing the PR or merging in changes from the base branch.

# Conflicts:
#	packages/js/src/settings/routes/llms-txt.js

# Conflicts:
#	packages/js/src/settings/routes/llms-txt.js
* preload the store with initial data from the currently selected llms.txt pages -- this guarantees the UI has the correct info from the start as the initial fetch only returns the top 10 pages of your site
* fix the fallback to all ids if the search query is empty
* filter out the other selected ids (except our own)
* limit the maximum amount of selected pages to 10
When the parent/global request was still fetching it would not check this and just fallback to all the ids, which are still empty at that moment
@igorschoester igorschoester force-pushed the 655-make-sure-that-the-user-cant-select-a-page-multiple-times-in-the-manual-page-selection-dropdowns branch from 7315ef4 to a951321 Compare July 10, 2025 10:43
'name' => ( $post->get_title() ) ? $post->get_title() : $post->get_slug(),
'id' => $page_id,
'title' => ( $post->get_title() ) ? $post->get_title() : $post->get_slug(),
'slug' => $post->get_slug(),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we can remove this? (and cascade the slug removal in the store as well)

I dont think we use it anywhere meaningful currently.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not a big deal actually and certainly not a blocker, so I'm moving on

Comment thread packages/js/src/settings/components/formik-indexable-page-select-field.js Outdated
@leonidasmi
Copy link
Copy Markdown
Contributor

leonidasmi commented Jul 11, 2025

CR + Acceptance is ✅

(@igorschoester I pushed the change for increasing the pool of the endpoint to 100 like we said and also a minor comment correction, so just a heads up)

@leonidasmi leonidasmi merged commit 6686939 into feature/llms-txt-phase-2 Jul 11, 2025
36 checks passed
@leonidasmi leonidasmi deleted the 655-make-sure-that-the-user-cant-select-a-page-multiple-times-in-the-manual-page-selection-dropdowns branch July 11, 2025 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog: non-user-facing Needs to be included in the 'Non-userfacing' category in the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants