-
Notifications
You must be signed in to change notification settings - Fork 920
Add warning for when the LLMs.txt cannot be generated in our dashboard. #22329
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
base: trunk
Are you sure you want to change the base?
Add warning for when the LLMs.txt cannot be generated in our dashboard. #22329
Conversation
40a60ac
to
59228b9
Compare
59228b9
to
273f280
Compare
Pull Request Test Coverage Report for Build a7ad6847bd1adff7d83c03c70d60e2ae4883676cWarning: 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
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CR: ❌
I think this implementation is blocked by this issue, so let's talk to see what we should do.
\esc_html__( 'Your llms.txt file couldn\'t be auto-generated', 'wordpress-seo' ), | ||
\sprintf( | ||
/* translators: 1: Link start tag to the WordPress Reading Settings page, 2: Link closing tag. */ | ||
\esc_html__( 'It looks like there is an llms.txt file already that wasn\'t created by Yoast, or the llms.txt file created by Yoast has been edited manually. We don\'t want to overwrite this file\'s content, so if you want to let Yoast keep auto-generating the llms.txt file, you can %1$smanually delete the existing one%2$s. Otherwise, consider disabling the Yoast feature.', 'wordpress-seo' ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like we have a single message that describes only the not_managed_by_yoast_seo
case. We need to be able to show the different message when on the filesystem_permissions
case.
* @return bool | ||
*/ | ||
private function should_show_file_failure_notification(): bool { | ||
return \get_option( Populate_File_Command_Handler::GENERATION_FAILURE_OPTION, false ) !== false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Populate_File_Command_Handler::GENERATION_FAILURE_OPTION
is not an autoloaded option (nor it should be).
Which means that with this PR, we add a DB call to every admin page load, which is something we shouldn't do.
Unfortunately, the way the notification center is built, that way is the only way we ever added notices, so I'm not sure what's the solution here, but let's go back to the drawing board for this.
Context
Summary
This PR can be summarized in the following changelog entry:
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:
alert center
and make sure there are noproblems
relating to the llms.txt feature.alert center
and make sure there are noproblems
relating to the llms.txt feature.control
the file.alert center
and make sure there is nowproblem
relating to the llms.txt feature. Make sure it looks like what is in the issue.alert center
is empty again.Relevant test scenarios
Test instructions for QA when the code is in the RC
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
Other environments
[shopify-seo]
, added test instructions for Shopify and attached theShopify
label to this PR.Documentation
Quality assurance
Innovation
innovation
label.Fixes #