Problem/Motivation

This module aims to solve sort of the same problems as prevent_homepage_deletion, since their last update to also protect 403 and 404 page.

Proposed resolution

Make one of the modules obsolete. Probably by always keeping nodes that are configured in system.site.yml. This way you always keep the most important pages, so the site will not break, without the need to configure these pages manually. We might add a setting to always protect them or leave it to individual settings.

Issue fork node_keep-3480116

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

jeroen dost created an issue. See original summary.

divyansh.gupta’s picture

Assigned: Unassigned » divyansh.gupta

divyansh.gupta’s picture

Assigned: divyansh.gupta » Unassigned
Status: Active » Needs review

Added the feature where it always keeps homepage, 403 and 404 and prevents them from being deleted as the site may crash if these important nodes are deleted.

anybody’s picture

This is really a nice idea! But implementation looks a bit hacky to me and I think this should be optional, so it would need a settings page to enable this functionality.

But just my 2 cents ;)

joshahubbers’s picture

I agree that a setting should be preferrable to prevent unexpected functionality.

What I was wondering: the normal working of this module is a checkbox at the node-edit page (I did not live test the module, but that's what I got from the documentation, correct me if I'm wrong). If I got this correct, the checkbox should be checked and grayed out on the page that is set as homepage, 404 and 403 by default to keep consistency with the normal working way of this module.

A last point: is unpublishing of the page also prevented for these modules?

When these points are fixed, I think we can deprecate the prevent_homepage_deletion module in favour of this one.

divyansh.gupta’s picture

Assigned: Unassigned » divyansh.gupta
Status: Needs review » Active

Hello @joshahubbers, @anybody
I proposed my solution as per the problem mentioned in the summary that if a page mentioned in system.site.yml is deleted than it may cause the site to break.
So as per your suggestions i will work and create a option to enable/disable this feature in settings form.
Also @joshahubbers i have not created any checks while unpublishing the pages mentioned in system.site.yml.

divyansh.gupta’s picture

Assigned: divyansh.gupta » Unassigned
joshahubbers’s picture

Thank you! That's nice.

Maybe the check for unpublished should be a seperate ticket because it is new global functionality, don't you think? If so than I can create a new issue for that?

divyansh.gupta’s picture

Hello @joshahubbers,
I have applied the changes according to requirements and it would be a great idea to create a seperate issue for implementing a button in settings to enable and disable this.
Also if you think my work is correct and requires no changes according to this issue, can you please move my ticket to needs review.

divyansh.gupta’s picture

Status: Active » Needs review
joshahubbers’s picture

Hi @divyansh.gupta,

you said in #7:

So as per your suggestions i will work and create a option to enable/disable this feature in settings form.

I am missing that in this branch?

joshahubbers’s picture

Status: Needs review » Needs work
divyansh.gupta’s picture

Assigned: Unassigned » divyansh.gupta

Sorry forgot about that will make the changes.

divyansh.gupta’s picture

Assigned: divyansh.gupta » Unassigned
Status: Needs work » Needs review

Created a option to enable and disable this functionality in settings form also tested it and it is working fine from my side.

joshahubbers’s picture

Sorry, I know I am a knit-wit... but "enabled" suggests the variable enables the entire functionality. But it enables/disables the protection of 404, 403 and front pages. Maybe rename that to "protect_404_403_front"? Than it is obvious what the setting does...

divyansh.gupta’s picture

@joshahubbers,
I have made the changes please check.

joshahubbers’s picture

Yes, great. Now only test-coverage is missing. ;-)

joshahubbers’s picture

Status: Needs review » Needs work
dhruv.mittal’s picture

Assigned: Unassigned » dhruv.mittal
dhruv.mittal’s picture

Assigned: dhruv.mittal » Unassigned
Status: Needs work » Needs review