Problem/Motivation

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.

Proposed resolution

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.

CommentFileSizeAuthor
#10 test_failure.txt2.48 KBsivaji_ganesh_jojodae

Issue fork drupal-3572830

Command icon Show commands

Start 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:

Comments

phenaproxima created an issue. See original summary.

sourav_paul’s picture

I would like to work on this.

sourav_paul’s picture

Status: Active » Needs review
smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

There's open threads that appear valid. Also probably needs test coverage for the change.

sourav_paul’s picture

Status: Needs work » Needs review
dcam’s picture

Status: Needs review » Needs work

A major refactoring of the SystemRequirements class 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.

sourav_paul’s picture

Status: Needs work » Needs review
sivaji_ganesh_jojodae’s picture

Status: Needs review » Needs work

The error appears unrelated, but the pipeline is consistently failing.

sivaji_ganesh_jojodae’s picture

StatusFileSize
new2.48 KB

The following test methods are failing,

* testSettingsExist
* testSitesDirectoryHardening
* testSitesDirectoryHardeningConfig

See the error log attached.

sourav_paul’s picture

Status: Needs work » Needs review
smustgrave’s picture

Some 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.

sourav_paul’s picture

Thanks 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.

smustgrave’s picture

Status: Needs review » Needs work

Summary 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.