Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.In site/hosting_site.module we have
/**
* Hide the delete button on site nodes
*/
function hosting_site_form_alter(&$form, &$form_state, $form_id) {
// Remove delete button from site edit form
if ($form_id == 'site_node_form') {
$form['delete']['#type'] = 'hidden';
}
}
but this doesn't hide the Delete button. The correct code (that works for me) is:
function hosting_site_form_alter(&$form, &$form_state, $form_id) {
// Remove delete button from site edit form
if ($form_id == 'site_node_form') {
$form['buttons']['delete']['#type'] = 'hidden';
}
}
At the moment we are leaving the Site node exposed to being deleted without executing the proper Delete task, which is dangerous (see #563950: Deleting a site does not clean up the /config/vhost.d directory)
Any objections to the above code before I commit?
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | 564038.patch | 914 bytes | mig5 |
| #3 | 564038.patch | 690 bytes | mig5 |
Comments
Comment #1
Anonymous (not verified) commentedI also meant to add, should we be doing a similar thing anywhere else such as Platforms?
I see we have a hosting_platform_delete() in platform that does similar cleanup to deleting a site, however the only hosting_tasks we have for Platforms is 'Verify' (kind of makes sense.. we do not want to disable a platform that has enabled sites running under it.. I'm assuming? Sites should be disabled first before an entire platform is disabled.. this is a design question I guess.)
So maybe a bit of work to do here.
Edit: actually on closer inspection, deleting a Platform node goes through and deletes all sites, tasks, packages etc that are tied to that platform rid. Deleting a site node does the same thing (deletes tasks and site node).
So we should be consistent here and either:
a) Remove the Delete submit option and thus the hook_delete tasks are (I think) not required, because instead the 'Delete' task should be queued and executed, and let the 'provision delete' command do the job (which seems more wholesome in its actions such as removing Apache configs)
At that rate, remove the Site delete option per this ticket and also the hook_delete that removes the site task nodes too
b) Keep the deletion of the site/platform nodes but factor in extra stuff such as deleting Apache configs.
We need to decide whether we can delete a Platform outright if we have currently enabled sites running on it, for either option. And obviously a) seems like the better choice right?
Anarcat noted that deleting a platform node doesn't remove the apache conf in this ticket #529242: deleting a platform should remove the platform apache config - is that the expected practice, deleting sites/platforms by deleting their nodes, because that seems to be in conflict with what we're telling users now (use Disable & then Delete tasks for the site)
In testing I added a 'Delete' task for the platform, and tried to run a delete task in that way, but the 'provision delete' command in Provision is heavily designed to work only with sites (it tries to take a backup first, etc), and in fact the Delete task fails (could not find /default/settings.php etc)
So there is a bit of work to do here either way.
Comment #2
Anonymous (not verified) commentedComment #3
Anonymous (not verified) commentedSo per discussion with Vertice on IRC, 'Delete' button still exists on Platform and may as well stay there for the meantime, as it can be used to 're-import' sites by blowing away the platform. And we won't have proper platform management in the immediate term.
So I've attached a patch that removes the Delete button from site nodes, forcing the practice of using the Disable > Delete tasks. I'll also speak to steveparks who can probably make the documentation more concise on this.
If someone can test this patch and confirm that it hides the Delete button on editing a site node, I'll commit (and if they can confirm the button was there prior to the patch.. apparently I have a habit of patching imaginary issues that no-one else experiences :( hehe)
Comment #4
Anonymous (not verified) commentedUpdated patch per IRC conversation: hide the Delete button unless the site's already been Disabled and Deleted through tasks (I was wrong and assumed the Delete task removed the site node).
So after a site's been deleted the proper way, we offer the choice to the user to delete the site node (some people may wish to keep it for logging, maybe at some point with views integration, deleted sites may wish to be reported on)
Comment #5
Anonymous (not verified) commentederr.. drupal.org bug? comment submission with a missing Version field caused the patch to have a href of 'drupal.org/files/issues'
Patch attached.
Comment #6
acWorks as advertised
Comment #7
Anonymous (not verified) commentedCommitted to HEAD, thanks!