This was spun out of #1503146: Aliased paths cannot be set as front page.
On the site information form, it is possible to set the front page and 4xx pages to system paths (and now aliases) that don't actually exist. And you won't have a way to know about it unless you visit those pages and find that they're broken.
On the status report, flag a warning (or maybe an error) if any of the system.site:page.[front|403|404] configuration options are unroutable.
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | test_failure.txt | 2.48 KB | sivaji_ganesh_jojodae |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
sourav_paulI would like to work on this.
Comment #4
sourav_paulComment #5
smustgrave commentedThere's open threads that appear valid. Also probably needs test coverage for the change.
Comment #6
sourav_paulComment #7
dcam commentedA major refactoring of the
SystemRequirementsclass to move the update and runtime phases to a separate class was committed last week. This has caused a merge conflict and the MR must be rebased. See #3554134: Begin cleaning up system install requirements.The other code was moved to a new Hook class with the intention that the giant hook function will be broken up in follow-up issues - work that has already started. So don't add this new code to the big all-in-one hook. It would just make more work for people later. Try adding it to a new Hook function instead since we can have multiple instances of OOP hooks.
Comment #8
sourav_paulComment #9
sivaji_ganesh_jojodae commentedThe error appears unrelated, but the pipeline is consistently failing.
Comment #10
sivaji_ganesh_jojodae commentedThe following test methods are failing,
* testSettingsExist
* testSitesDirectoryHardening
* testSitesDirectoryHardeningConfig
See the error log attached.
Comment #11
sourav_paulComment #12
smustgrave commentedSome of this is kinda reading like AI written without disclosure so personally I'm deprioritizing this review and will pick up at the needs-review-queue deadline if no one picks up.
Comment #13
sourav_paulThanks for your feedback. I added this explanation to make things clearer.
This started as a warning for system page paths in the status report. These paths include the page, 403 page and 404 page.
I changed the code recently. At first I put the logic for this warning in a class, That caused problems as the system module was doing the thing twice which caused an error in some tests.
Now I removed the class & fixed it so the system module only does it once. The warning about system page paths is now in helper methods. These methods are part of the existing SystemRequirementsHooks class. I added them from the existing runtimeRequirements method.
I've chose this approach for two reasons:
1. It works with how the module's set up now
2. It keeps the logic separate, from other code
I also ran tests again to make sure everything works. The tests that were failing before pass. The new status report coverage also works.
Comment #14
smustgrave commentedSummary should include screenshots for the UX team to review the text proposals. Also may need a test that someone who may access the status report can see the message without the link if they don't have the correct permission for site configuration.