Problem/Motivation

When deleting a node(or other entity) from the edit form clicking the Delete button triggers validation on the entity. This doesn't make sense because a entity that is going to be deleted should not have to validated. It also causes users who don't have access the content overview screen to be unable to delete entities that can't be validate for a number or reasons.

Proposed resolution

Change the delete button to a delete link(similar to cancel links) on the edit form for Entities that provide a URI(unlink entities link Vocabularies).
This link will not delete directly but go to the conformation form like the current delete button.

Remaining tasks

Only need 1 more review to be RTBC.

User interface changes

Delete button changes to link

API changes

This will happen in the EntityFormController class.

Original report by naquah

(for legacy issues whose initial post was not the issue summary)
Say I am editing a node and have cleared the Title field. But then I decide to remove the node altogether. Boom. Title field is required.

It appears that clicking the Delete button on a node form triggers node validation. This is unexpected behavior and was not happening in 5.x.

I'm thinking the '#submit' => array('node_form_delete_submit') syntax for the button is misused here, since we don't actually want the form to be submitted, we just want to go to another page.

Files: 
CommentFileSizeAuthor
#158 interdiff.txt8.24 KBXano
#158 drupal_216064_158.patch27.21 KBXano
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 68,149 pass(es).
[ View ]
#146 drupal_216064_146.patch18.63 KBXano
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch drupal_216064_146.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#85 delete_button_to_link-216064-85.patch6.47 KBtedbow
PASSED: [[SimpleTest]]: [MySQL] 47,988 pass(es).
[ View ]
#82 delete_button_to_link-216064-82.patch4.2 KBtedbow
FAILED: [[SimpleTest]]: [MySQL] 47,548 pass(es), 21 fail(s), and 2 exception(s).
[ View ]
#76 216064-76-test.patch5.76 KBdrifter
FAILED: [[SimpleTest]]: [MySQL] 42,160 pass(es), 8 fail(s), and 1 exception(s).
[ View ]
#76 216064-76-combined.patch9.11 KBdrifter
PASSED: [[SimpleTest]]: [MySQL] 42,171 pass(es).
[ View ]
#73 216064-73-test.patch5.79 KBdrifter
FAILED: [[SimpleTest]]: [MySQL] 42,350 pass(es), 7 fail(s), and 1 exception(s).
[ View ]
#73 216064-73-combined.patch9.13 KBdrifter
PASSED: [[SimpleTest]]: [MySQL] 42,342 pass(es).
[ View ]
#71 216064-68-test.patch5.79 KBdrifter
FAILED: [[SimpleTest]]: [MySQL] 42,172 pass(es), 7 fail(s), and 1 exception(s).
[ View ]
#71 216064-68-combined.patch9.08 KBdrifter
FAILED: [[SimpleTest]]: [MySQL] 42,177 pass(es), 6 fail(s), and 49 exception(s).
[ View ]
#68 216064-68-test.patch5.79 KBdrifter
FAILED: [[SimpleTest]]: [MySQL] 42,167 pass(es), 7 fail(s), and 1 exception(s).
[ View ]
#68 216064-68-fix.patch3.34 KBdrifter
FAILED: [[SimpleTest]]: [MySQL] 42,081 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#66 216064-66-test.patch5.76 KBdrifter
FAILED: [[SimpleTest]]: [MySQL] 42,150 pass(es), 6 fail(s), and 0 exception(s).
[ View ]
#66 216064-66-fix.patch3.29 KBdrifter
FAILED: [[SimpleTest]]: [MySQL] 42,084 pass(es), 10 fail(s), and 51 exception(s).
[ View ]
#64 216064-form_validation-c64.patch10.02 KBArtusamak
PASSED: [[SimpleTest]]: [MySQL] 35,994 pass(es).
[ View ]
#62 216064-form_validation-c62.patch9.8 KBArtusamak
PASSED: [[SimpleTest]]: [MySQL] 35,996 pass(es).
[ View ]
#53 delete_validation-216064-45.patch9.59 KBArtusamak
PASSED: [[SimpleTest]]: [MySQL] 36,022 pass(es).
[ View ]
#52 delete_validation-216064-45.patch9.59 KBArtusamak
PASSED: [[SimpleTest]]: [MySQL] 32,996 pass(es).
[ View ]
#45 delete_validation-216064-45.patch9.59 KBmarcingy
PASSED: [[SimpleTest]]: [MySQL] 33,620 pass(es).
[ View ]
#39 delete_validation-216064-39.patch9.59 KBidflood
FAILED: [[SimpleTest]]: [MySQL] 29,480 pass(es), 0 fail(s), and 1 exception(es).
[ View ]
#38 delete_validation-216064-38.patch9.59 KBidflood
FAILED: [[SimpleTest]]: [MySQL] 29,470 pass(es), 2 fail(s), and 3 exception(es).
[ View ]
#35 delete_validation_path-216064-35.patch2.11 KBidflood
PASSED: [[SimpleTest]]: [MySQL] 29,437 pass(es).
[ View ]
#35 delete_validation_user-216064-35.patch1.5 KBidflood
FAILED: [[SimpleTest]]: [MySQL] 29,417 pass(es), 2 fail(s), and 2 exception(es).
[ View ]
#35 delete_validation_node-216064-35.patch2.4 KBidflood
FAILED: [[SimpleTest]]: [MySQL] 29,423 pass(es), 1 fail(s), and 0 exception(es).
[ View ]
#35 delete_validation_menu-216064-35.patch3.65 KBidflood
FAILED: [[SimpleTest]]: [MySQL] 29,429 pass(es), 0 fail(s), and 1 exception(es).
[ View ]
#34 delete_validation_menu-216064-34.patch3.65 KBidflood
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch delete_validation_menu-216064-34_0.patch.
[ View ]
#34 delete_validation_node-216064-34.patch2.4 KBidflood
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch delete_validation_node-216064-34_0.patch.
[ View ]
#34 delete_validation_path-216064-34.patch2.11 KBidflood
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch delete_validation_path-216064-34_0.patch.
[ View ]
#34 delete_validation_user-216064-34.patch1.5 KBidflood
FAILED: [[SimpleTest]]: [MySQL] 29,407 pass(es), 2 fail(s), and 2 exception(es).
[ View ]
#32 drupal-delete-validation-216064-32.patch9.6 KBidflood
PASSED: [[SimpleTest]]: [MySQL] 29,059 pass(es).
[ View ]
#29 delete_path.patch2.41 KBygerasimov
PASSED: [[SimpleTest]]: [MySQL] 26,630 pass(es).
[ View ]
#29 delete_menu.patch2.6 KBygerasimov
PASSED: [[SimpleTest]]: [MySQL] 26,801 pass(es).
[ View ]
#29 delete_user.patch1.82 KBygerasimov
PASSED: [[SimpleTest]]: [MySQL] 26,569 pass(es).
[ View ]
#29 delete_node.patch2.72 KBygerasimov
PASSED: [[SimpleTest]]: [MySQL] 26,587 pass(es).
[ View ]
#24 forms_delete_buttons.patch2.41 KBygerasimov
PASSED: [[SimpleTest]]: [MySQL] 26,425 pass(es).
[ View ]
#2 delete.patch846 bytescatch
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch delete_0.patch.
[ View ]

Comments

Since the only thing the Delete button does is redirect to node/%/delete, it would be better if it were a simple link. Note that the Delete button on vocabulary forms still works by checking $_POST['op'] before the form is rendered, which is quite ugly.

Maybe there should be a way to keep (i.e. not replace them by links) the Delete buttons, not have form validation occur when clicking them, and not have the ugly $_POST['op'] == t('Delete') check. I'm thinking something like:

<?php
$form
['buttons']['delete'] = array(
 
'#type' => 'button', // This is a button, since we're not talking submission here
 
'#value' => t('Delete'),
 
'#weight' => 15,
 
'#redirect' => array('node/'. $node->nid .'/delete', $destination), // Similar to #validate and #submit for submit buttons
);
?>

So my proposal is that we let _form_builder_handle_input_element check for the presence of #redirect when a button is clicked, save this fact in the $form_state, then let drupal_process_form call drupal_redirect_form before even drupal_validate_form is called.

Title:Delete button triggers node validationRegression: Delete button triggers node validation
Version:6.0-rc2» 6.x-dev
Status:Active» Needs review
StatusFileSize
new846 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch delete_0.patch.
[ View ]

I can see how this'd be really, really annoying.

Rolled the above into a patch, untested.

Ran the patch.

Before:
1. Created a node with content in both the title and body fields.
2. Click through to Administer > Manage > Content > Edit Page
3. Cleared text form the Title field.
4. Clicked "Delete"
5. Received message...

Title field is required.

After:
1. Patch ran clean, without zero errors.
2. Click through to Administer > Manage > Content > Edit Page
3. Immediately page displays error above "Title" field.

notice: Undefined variable: destination in C:\server\pub\test\6p00dev\modules\node\node.pages.inc on line 248.

Confirmation:
1. Reversed patch, above error disappeared.

» Respectfully, GreenLED
» Stable Files . net

Changing status.

» Respectfully, GreenLED
» Stable Files . net

Status:Needs review» Needs work

Quick note: the patch will need to implement the #redirect functionality for buttons too before this would work... and setting $destination to drupal_get_destination() would fix the notice.

Hi.

Is there any progress on this issue? In Drupal 6.3 it's obviously not fixed.

I just wanted to add, that this problem is not unique to deleting nodes. It also occurs when deleting users (see http://drupal.org/node/274547 ). Perhaps this issue should be marked as a forms API issue and not just specific to the node system.

I haven't tested but I imagine this might also occur in other areas (like deleting taxonomy terms perhaps?).

Version:6.x-dev» 6.9

This issue is still happening in 6.9. Is anyone still working on this?

Work is happening on this to an effect for 7 at #370823: Allow Nodes to Be Previewed Without Required Fields.

Got the same problem.

Title:Regression: Delete button triggers node validationDelete button triggers form validation
Version:6.9» 7.x-dev
Component:node system» forms system

it applies at least for the node and for the user form..
this issue is dependent on #370537: Allow suppress of form errors (hack to make "more" buttons work properly)

Status:Needs work» Active

#370537: Allow suppress of form errors (hack to make "more" buttons work properly) has landed and is now at "needs work" status for the addition of docs. Should therefore be possible to fix this issue now. :D

Marking as "active" (no patch) since #2 is not relevant to 7.x.

This bit one of about 4 serious users on a site I run - we've made the node "Log message" field required like on d.o, and she couldn't work out how to delete nodes. Having to put something in the Log message box is pretty bizarre given that it will be nuked in just a moment!!!

#370537: Allow suppress of form errors (hack to make "more" buttons work properly) has landed...Should therefore be possible to fix this issue now.

Yes. Just set '#limit_validation_errors' => array() on the "delete" button. Would someone like to roll that patch for all the relevant "delete" buttons in core, so it can be discussed? Not sure if it can make it into D7, but a concrete patch could help spur discussion.

subscribing

This has often annoyed me too.

This behavior is also apparent when deleting menus. A module created a menu with a name longer than 27 characters, but now I can't delete it because it tells me that the menu name is too long. Destructive actions should skip the validation step.

subscribing

Is 27 still the max for a menu? I am trying to recreate this and I am using menu title's with more than 27 characters and they are being saved.
EDIT: Nevermind it appears to be 128 now. And this is still an issue.

Version:7.x-dev» 6.19

This is realy looking strange: you can't delete the node without validation proccess.
I've tried the above solutions and they didn't help...
Subscribing.

Version:6.19» 7.x-dev

Fixes need to be done first on the version in development (currently, D7), then only after that is complete, possibly backported to an already released version (e.g., D6).

StatusFileSize
new2.41 KB
PASSED: [[SimpleTest]]: [MySQL] 26,425 pass(es).
[ View ]

I have tired to fix the forms with '#limit_validation_errors'.

Forms that are fixed:
1. Menu edit form. admin/structure/menu/item/%/edit
2. Role edit form. admin/people/permissions/roles/edit/%
3. Node edit form. node/%/edit
4. URL alias edit form. admin/config/search/path/edit/%

Forms that cannot be fixed in this way (not fixed with this patch):
1. Content type edit form admin/structure/types/manage/%
2. Taxonomy vocabulary edit form admin/structure/taxonomy/%/edit
3. Taxonomy term edit form taxonomy/term/%/edit

Please check if I missed any other forms and whether this patch fixes above mentioned forms.

Status:Active» Needs review

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

This is one correct patch, although it needs comments ( why it's not empty? Why nid?) i do not have the shortest idea how it will be backported. Also, needs tests.

Assigned:Unassigned» ygerasimov

Alright. Lets concentrate on first four forms from #24 first. I will write tests and put comments.

Oh, by the way why we can't fix those three? if there is no #type value to hang your hat on, well, add one.

Status:Needs work» Needs review
StatusFileSize
new2.72 KB
PASSED: [[SimpleTest]]: [MySQL] 26,587 pass(es).
[ View ]
new1.82 KB
PASSED: [[SimpleTest]]: [MySQL] 26,569 pass(es).
[ View ]
new2.6 KB
PASSED: [[SimpleTest]]: [MySQL] 26,801 pass(es).
[ View ]
new2.41 KB
PASSED: [[SimpleTest]]: [MySQL] 26,630 pass(es).
[ View ]

I have splitted the patch to four pieces for each form separately.

Regarding the last three forms, they cannot be fixed with #limit_validation_errors as Delete buttons don't have #submit property. So they should be fixed in different way.

Please review in the meantime patches for fixable forms.

#29: delete_menu.patch queued for re-testing.

User and node apply with offset but the patch looks good to me. I tested the 4 scenarios without patches to reproduce the issue and then applied them. Everything works as expected but i noticed one other form where this validation issue occurs, admin/structure/menu/manage/%/edit ( to delete a custom menu ).

Maybe it's better to open another issue for the 3 "special" forms about taxonomy and content type.

Status:Needs review» Needs work
StatusFileSize
new9.6 KB
PASSED: [[SimpleTest]]: [MySQL] 29,059 pass(es).
[ View ]

I worked on the "delete menu" form. I'm not sure about the new test, and if it should be moved in a new function, maybe something like $this->deleteCustomMenuValidation().

This include the 4 patches from #29 + fix for the "delete menu" form.

Status:Needs work» Needs review

Version:7.x-dev» 8.x-dev
StatusFileSize
new1.5 KB
FAILED: [[SimpleTest]]: [MySQL] 29,407 pass(es), 2 fail(s), and 2 exception(es).
[ View ]
new2.11 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch delete_validation_path-216064-34_0.patch.
[ View ]
new2.4 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch delete_validation_node-216064-34_0.patch.
[ View ]
new3.65 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch delete_validation_menu-216064-34_0.patch.
[ View ]

Let's do it for d8 first. Patches should be identical as the previous one but I wasn't able to apply it so I made the change manually.

StatusFileSize
new3.65 KB
FAILED: [[SimpleTest]]: [MySQL] 29,429 pass(es), 0 fail(s), and 1 exception(es).
[ View ]
new2.4 KB
FAILED: [[SimpleTest]]: [MySQL] 29,423 pass(es), 1 fail(s), and 0 exception(es).
[ View ]
new1.5 KB
FAILED: [[SimpleTest]]: [MySQL] 29,417 pass(es), 2 fail(s), and 2 exception(es).
[ View ]
new2.11 KB
PASSED: [[SimpleTest]]: [MySQL] 29,437 pass(es).
[ View ]

same as before but without broken last line in patch file...

Status:Needs review» Needs work

The last submitted patch, delete_validation_menu-216064-35.patch, failed testing.

@idflood: Please merge those patches into a single one. (just like the one in #32)

Status:Needs work» Needs review
StatusFileSize
new9.59 KB
FAILED: [[SimpleTest]]: [MySQL] 29,470 pass(es), 2 fail(s), and 3 exception(es).
[ View ]

@sun: here it is. I didn't looked at those failed tests yet.

StatusFileSize
new9.59 KB
FAILED: [[SimpleTest]]: [MySQL] 29,480 pass(es), 0 fail(s), and 1 exception(es).
[ View ]

The fix was not in the right place for user.admin.inc. This sould fix at least 1 test.

Status:Needs review» Needs work

The last submitted patch, delete_validation-216064-39.patch, failed testing.

Just ran into this.

Assigned:ygerasimov» marcingy

Grabing I had a more exacting patch for nodes will work on this over the next couple of days

Status:Needs work» Reviewed & tested by the community

Looks good

Status:Reviewed & tested by the community» Needs work

Looks like a mis-post.

Status:Needs work» Needs review
StatusFileSize
new9.59 KB
PASSED: [[SimpleTest]]: [MySQL] 33,620 pass(es).
[ View ]

There was a typo in the patch - reroll.

Status:Needs review» Reviewed & tested by the community

Yay, i tested all of this and it's working great, this patch is just what we need.
Curious that it hasn't been fixed for D7, it could easily be backported.

Issue tags:+needs backport to D7

Adding backport tag

Issue tags:+needs backport to D6

I propose this also be backported to D6 as well, so I am tagging it as such. If for no other reason, than just because this was originally a 6.x bug!

Drupal 6 has no concept of validation limiting so it would be an API change and as result it can't really be backported to drupal 6 is the assumption I'm making.

Issue tags:-Needs tests

It has tests now so removing that tag.

StatusFileSize
new9.59 KB
PASSED: [[SimpleTest]]: [MySQL] 32,996 pass(es).
[ View ]

The patch in #45 is also appliable to D7. Let's attach the patch for review.

Version:8.x-dev» 7.x-dev
StatusFileSize
new9.59 KB
PASSED: [[SimpleTest]]: [MySQL] 36,022 pass(es).
[ View ]

Sorry, rerolling with 7.x

Version:7.x-dev» 8.x-dev

Please leave at d8 as it needs to be fixed there first

The 7.x-dev version modification was to tell the bot to roll the patch for D7.

Subscribe

Status:Reviewed & tested by the community» Needs work

+++ b/modules/menu/menu.test
@@ -60,6 +60,24 @@ class MenuTestCase extends DrupalWebTestCase {
+    $this->assertNoText(t('Title field is required.'), t('Delete menu on edit form with empty title.'));
@@ -72,6 +90,35 @@ class MenuTestCase extends DrupalWebTestCase {
+    $this->assertNoText(t('Menu link title field is required.'), t('Delete link on edit form with empt    y title.'));
...
+    $this->assertNoText(t('Path field is required.'), t('Delete link on edit form with empty path.'));
+++ b/modules/node/node.test
@@ -394,6 +394,54 @@ class PageEditTestCase extends DrupalWebTestCase {
+    $this->assertNoText(t('Title field is required.'), t('Delete button works with empty title on edit form.'));
+++ b/modules/path/path.test
@@ -107,6 +107,41 @@ class PathTestCase extends DrupalWebTestCase {
+    $this->assertNoText(t('Existing system path field is required.'), t('Delete alias on edit form with empty source field.'));
...
+    $this->assertNoText(t('Path alias field is required.'), t('Delete alias on edit form with empty alias field.'));
+++ b/modules/user/user.test
@@ -1841,6 +1841,11 @@ class UserRoleAdminTestCase extends DrupalWebTestCase {
+    $this->assertNoText(t('Role name field is required.'), t('Role can be deleted with empty name submitted to the form.'));

1) Either use the proper t() source string here ("!name field is required."), or don't use t() at all.

2) Odd/stray spacing in one of the assertion messages.

+++ b/modules/node/node.test
@@ -394,6 +394,54 @@ class PageEditTestCase extends DrupalWebTestCase {
+class NodeDeleteTestCase extends DrupalWebTestCase {

Missing phpDoc.

20 days to next Drupal core point release.

This is also a problem on #ajax, I can't seem to stop the button from validating the form on the ajax callback.

Running fresh D7 :
When I try to delete taxonomy vocabulary (including taxonomy menu module)... The window-name changes to "are you sure you want to delete...blabla", but the admin overlay does not change aspect... and there is no way to go out from this loop. If I go to the homepage and back to admin, I simplu see that the vocabulary has not been deleted...
I suppose this is just a problem due to the fact that I the delete confirmation window is not well shown...
HELP !!! It's a big bug !

Hi, @xgougeon, welcome to drupal.org. It seems that your problem isn't related to this thread. You would probably be better off looking in the Taxonomy Menu issue queue (perhaps you could create a new issue there). More information on Drupal issues can be found in the issue queue handbook.

Hi pjcdawkins. Thanks for your help, and sorry about that I was dealing with several problems at once and got confused...

Assigned:marcingy» Unassigned
Status:Needs work» Needs review
StatusFileSize
new9.8 KB
PASSED: [[SimpleTest]]: [MySQL] 35,996 pass(es).
[ View ]

Let's try that again!

Status:Needs review» Needs work

Now there's a mix of using t()

+++ b/core/modules/menu/menu.testundefined
@@ -62,6 +62,21 @@ class MenuTestCase extends DrupalWebTestCase {
+    $this->assertNoText('Title field is required.', t('Delete menu on edit form with empty title.'));

one string with and the other without t()

+++ b/core/modules/menu/menu.testundefined
@@ -74,6 +89,35 @@ class MenuTestCase extends DrupalWebTestCase {
+    $this->assertNoText('Menu link title field is required.', t('Delete link on edit form with empty title.'));

dito

+++ b/core/modules/menu/menu.testundefined
@@ -74,6 +89,35 @@ class MenuTestCase extends DrupalWebTestCase {
+    $this->assertNoText('Path field is required.', t('Delete link on edit form with empty path.'));

dito

+++ b/core/modules/node/node.testundefined
@@ -413,6 +413,59 @@ class PageEditTestCase extends NodeWebTestCase {
+    $this->assertNoText('Title field is required.', t('Delete button works with empty title on edit form.'));

dito

+++ b/core/modules/path/path.testundefined
@@ -131,6 +131,41 @@ class PathAliasTestCase extends PathTestCase {
+    $this->assertNoText('Existing system path field is required.', t('Delete alias on edit form with empty source field.'));
+

dito

+++ b/core/modules/path/path.testundefined
@@ -131,6 +131,41 @@ class PathAliasTestCase extends PathTestCase {
+    $this->assertNoText('Path alias field is required.', t('Delete alias on edit form with empty alias field.'));

dito

+++ b/core/modules/user/user.testundefined
@@ -1923,6 +1923,11 @@ class UserRoleAdminTestCase extends DrupalWebTestCase {
+    $this->assertNoText('Role name field is required.', t('Role can be deleted with empty name submitted to the form.'));

dito

Status:Needs work» Needs review
StatusFileSize
new10.02 KB
PASSED: [[SimpleTest]]: [MySQL] 35,994 pass(es).
[ View ]

Reposting with the appropriate fix.

Assigned:Unassigned» Artusamak
Status:Needs review» Needs work

Feedback from XJM:

[17:54] Artusamak: couple thoughts on that patch. one, it is helpful to include an interdiff when you update patches
[17:54] interdiff?
[17:54] For instructions on creating an interdiff, see http://drupal.org/node/1488712 Or, if you're into microbranching like I am, see xjm's piece http://xjm.drupalgardens.com/blog/interdiffs-how-make-them-and-why-they-...
[17:54] Artusamak: also, you had the translations for assertions backwards
[17:54] assert t?
[17:54] Do not use t() on assertion message texts. See http://drupal.org/simpletest-tutorial-drupal7#t and http://drupal.org/node/500866.
[17:54] Artusamak: ^ review that for when it should be used and not in tests
[17:55] Artusamak: basically, it is definitely never needed on the assertion message argument, but should be used when testing *FOR* strings already translated
[17:56] Artusamak: another comment without having looked too closely is that the inline comments are wrapping very early. they should go as close to 80 chars as possible without going over. Also, it's good to use full sentences, including words like "the"; "menu link item ID" instead of mlid; etc.
[17:57] Artusamak: also, this is a really pedantic thing and I'm sorry--but all the one-line summaries in the docblocks should begin with a third-person verb
[17:58] Artusamak: for example, I would suggest changing "Delete alias through alias edit form" to "Tests deletion of a path alias through the alias edit form."
[17:59] Artusamak: and apply that sort of change to all the comments throughout the patch so that they are more complete English sentences (and less terse)
[18:01] Artusamak: oh and finally, it is also good to upload a test-only patch (as the first attachment) and the complete patch (as the second) to expose the failures of the test on the testbot. using that order prevents the bot from marking the issue needs work, also. :)

Assigned:Artusamak» drifter
Status:Needs work» Needs review
StatusFileSize
new3.29 KB
FAILED: [[SimpleTest]]: [MySQL] 42,084 pass(es), 10 fail(s), and 51 exception(s).
[ View ]
new5.76 KB
FAILED: [[SimpleTest]]: [MySQL] 42,150 pass(es), 6 fail(s), and 0 exception(s).
[ View ]

Rerolling, updated tests for new locations, and the node form to EntityFormController. As a bonus, we'll get this fix for all entities using EntityFormController.

Status:Needs review» Needs work

The last submitted patch, 216064-66-fix.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new3.34 KB
FAILED: [[SimpleTest]]: [MySQL] 42,081 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
new5.79 KB
FAILED: [[SimpleTest]]: [MySQL] 42,167 pass(es), 7 fail(s), and 1 exception(s).
[ View ]

A better way to determine the entity type in the EntityFormController, plus fixes for role deletion.

Status:Needs review» Needs work

The last submitted patch, 216064-68-fix.patch, failed testing.

@drifter - Thanks for the reroll and subsequent changes! Can you please upload the test-only patch along with a patch containing both the test AND the fix? See #13 in http://drupal.org/node/1468170 for more explanation.

Status:Needs work» Needs review
StatusFileSize
new9.08 KB
FAILED: [[SimpleTest]]: [MySQL] 42,177 pass(es), 6 fail(s), and 49 exception(s).
[ View ]
new5.79 KB
FAILED: [[SimpleTest]]: [MySQL] 42,172 pass(es), 7 fail(s), and 1 exception(s).
[ View ]

OK, let's try that... the failing tests passed for me locally.

Status:Needs review» Needs work

The last submitted patch, 216064-68-combined.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new9.13 KB
PASSED: [[SimpleTest]]: [MySQL] 42,342 pass(es).
[ View ]
new5.79 KB
FAILED: [[SimpleTest]]: [MySQL] 42,350 pass(es), 7 fail(s), and 1 exception(s).
[ View ]

Hmm, didn't do my merges correctly. Sorry about that.

Thanks @drifter! To move this even further along, there are a few minor code conventions that can be cleaned up:

+++ b/core/lib/Drupal/Core/Entity/EntityFormController.phpundefined
@@ -132,6 +135,8 @@ protected function actions(array $form, array &$form_state) {
+        '#limit_validation_errors' => array(array($info['entity keys']['id'])),        ¶

Trailing whitespace here.

+++ b/core/modules/node/lib/Drupal/node/Tests/NodeDeletionTest.phpundefined
@@ -0,0 +1,61 @@
\ No newline at end of file

Add a newline (blank line) at the end of this new test file.

The inline comments should also be wrapped at 80 characters (some are over 80 characters, some wrap early) and be complete sentences as per http://drupal.org/node/1354.

#1342444: Do not validate fields if entity is in the process of being deleted was closed as a duplicate of this issue, looks like it came to a similar conclusion anyway (modify EntityFormController).

StatusFileSize
new9.11 KB
PASSED: [[SimpleTest]]: [MySQL] 42,171 pass(es).
[ View ]
new5.76 KB
FAILED: [[SimpleTest]]: [MySQL] 42,160 pass(es), 8 fail(s), and 1 exception(s).
[ View ]

Thanks for the review. Fixing up the comments and whitespace.

Status:Needs review» Needs work

@drifter, went to check the patch but it not longer applies. Can you reroll?

Also I think you should be using $entity->id() to get the id of the entity in EntityFormController->actions() instead of entity_get_info. It's cleaner and simplier.

Here is the example

  protected function actions(array $form, array &$form_state) {
    $entity = $this->getEntity($form_state);
    return array(
     .....
      'delete' => array(
        '#value' => t('Delete'),
        // No need to validate the form when deleting the entity.
        // Keep the entity id key for passing an id to delete confirmation form
        '#limit_validation_errors' => array(array($entity->id())),
  }

Also I think we are going to need to deal with the required tags on client-side. Basically currently the browser will not let me submit the form because the browser(newer ones at least. I checked Mac Chrome) is not going to let you submit the form because of "required" tag on the input element. This happens on the client side it stops it before it actually submits it. So it is independent of the form_api.

I am not a front-end so I don't know the preferred way to deal with this(or if we should) . My guess would be through javascript but there may be a better way through an attribute.

It might be a larger issue for the form api to remove the required tags on input if the element has the '#limit_validation_errors'

Easier solution, of course, is to move "delete" to a separate callback / page tab. Maybe turn the button into a link.

A link makes sense to.

If we did a separate callback wouldn't still have the client side required validation issue?
Or would it be a separate element?

Delete should really be a separate callback. The form contents have nothing to do with the delete operation, so sending it on delete does not make much sense.

But it should NOT be a link. According to the HTTP spec, potential destructive operations should never be sent through a GET request (simplified explanation), it should be used for information retrieval only. Some more info here: http://stackoverflow.com/questions/786070/why-should-you-delete-using-an...

@fuzzy76 it will be a link to the confirmation page (which is a form with only the $entity->id() element), so no problem on GET.

Status:Needs work» Needs review
StatusFileSize
new4.2 KB
FAILED: [[SimpleTest]]: [MySQL] 47,548 pass(es), 21 fail(s), and 2 exception(s).
[ View ]

Ok here is patch that changes the Delete button to a link in certain entity types.

The link is used for entity types that have URI.

For entity types like Vocabulary that don't provide a URI then the button is still used. In the case of the VocabularyFormController the delete action is actually a different output of the add/edit form(i.e there is not separate delete confirm form).

This will still need work and will need tests but setting needs review to get for eyes on it.

I have manually tested it with Vocabularies, Terms, Nodes

Status:Needs review» Needs work

The last submitted patch, delete_button_to_link-216064-82.patch, failed testing.

Assigned:drifter» tedbow

@drifter I am assigning this to myself for now.

I am going to work on changing tests reflect this new change.

Still waiting on hearing back from people about this approach.

Title:Delete button triggers form validationDelete button triggers form validation - change to Delete link for entities with URI
Status:Needs work» Needs review
StatusFileSize
new6.47 KB
PASSED: [[SimpleTest]]: [MySQL] 47,988 pass(es).
[ View ]

Ok, I have added some changes to tests. Hopefully this will pass all.

Changing title to reflect the fix

Ok so it passed all the tests. Any reviews would be good.

I think it should be ok as a link because:

  • It is just a redirection to the delete confirm form which actually does the deleting.
  • Adding in a link type under the "actions" is done other places in drupal core in the same manner - confirm_form for instance.
  • It avoids all client and server side validation of the entity being deleted(should not have to validate on delete)

Hopefully we can get this in before feature freeze.

I like it. It is a minor, minor UI change (but smaller than turning DELETE into a tab).

If we want to use CSS to clean this up to look like a button, we can do it later.

I'd rather not delay a long-delayed older issue in favor of a newer problem.

I'd rather not delay a long-delayed older issue in favor of a newer problem.

I agree. The user perspective, if css is used to make it look like a button, there is not much difference to the user.
But does fix a very basic problem.

Agree, this doesn't get in the way of #1834002: Configuration delete operations are all over the place which is rather different sort of UX bug. Fixing this for 8.x might also allow the problem to be considered further for 7.x. I had to implement a custom workaround on a 6.x site a couple of years back as users couldn't work out how to delete a node with a required field on the node edit form. I see there is now http://drupal.org/project/skip_validation for 6.x and http://drupal.org/project/soft_delete for 7.x but I'm not sure that either of those quite fits my basic requirement.

I had to implement a custom workaround on a 6.x site a couple of years back as users couldn't work out how to delete a node with a required field on the node edit form.

I think the problem that this patch solves is more serious than this.

There are some cases where without this patch a user is not able delete an entity they should be able to delete.
Sometimes an existing entity is not able to be validated at all.
A simple example of this is a node that has a required term reference field using a select list with no terms in the vocabulary.

When adding contributed modules this can happen in a number of ways.
If you have an Entity with a required Entity Reference field where the available options are controlled by a View. If the View doesn't return any results(this may be true for only some entities b/c of contextual filters) then you will not be able to delete the Entity.

If the current user doesn't have access to the Content Overview screen then there will literally be no way for the user to delete the entity though according to how the permissions are set up on the site they should be able to.

This is a problem with the core Entity system not any core or contributed fields.

I agree that #1834002: Configuration delete operations are all over the place is important but as far as I can tell it does not stop a user from performing actions that they should be able to. I think the current issue should be resolved first because it fixes a bug in Drupal that affects functionality.

Can we get one more review on the merits of the code in this patch and then RTBC, please.

+++ b/core/lib/Drupal/Core/Entity/EntityFormController.phpundefined
@@ -132,16 +130,32 @@ protected function actions(array $form, array &$form_state) {
+        // @todo Is this the proper why to get the #href???
+        $actions['delete'] = array(
+            '#type' => 'link',
+            '#title' => t('Delete'),
+            '#href' => $entity_uri['path'] . '/delete',

is this always going to be '/delete'?

is this always going to be '/delete'?

I think this true for all core entities that provide a URI(Vocabs don't) but of course not necessarily for contrib entities that use the EntityFormController class. Of course they can override the "actions" or "actionsElement" functions to change this.

There would also be a problem if left the delete button in place. Not all entities would handle the delete action in the same callback. So basically either way contrib modules that are using EntityFormController would have to know the default behaviour and either program to match or override 1 of these functions.

It should always be 'delete'. The issue that @sun links to in #88 should be a follow-up to enforce that standard.

StatusFileSize
new3.46 KB
new6.5 KB
PASSED: [[SimpleTest]]: [MySQL] 48,241 pass(es).
[ View ]

Minor docs cleanup and a couple indentation and whitespace fixes.

@Cottser
Thanks for that!

Would be nice if this could get RTBC!

Since this is a "bug" and not a new feature can it be worked on after Feature Freeze?

StatusFileSize
new6.44 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch delete_button_to_link-216064-100.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Reroll of patch in #97 but removed the @todo since #96 seems to confirm it is correct.

// @todo Is this the proper way to get the #href?

Status:Needs review» Needs work
Issue tags:-needs backport to D6, -needs backport to D7

The last submitted patch, delete_button_to_link-216064-100.patch, failed testing.

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, delete_button_to_link-216064-100.patch, failed testing.

Status:Needs work» Needs review

Status:Needs review» Needs work
Issue tags:+needs backport to D6, +needs backport to D7

The last submitted patch, delete_button_to_link-216064-100.patch, failed testing.

Issue tags:+Needs reroll

Tagging for reroll.

Status:Needs work» Needs review
Issue tags:-Needs reroll
StatusFileSize
new6.36 KB
PASSED: [[SimpleTest]]: [MySQL] 49,092 pass(es).
[ View ]

subscribe

FYI, @deastlack: Stop subscribing, start following. :)

Status:Needs review» Needs work

@TravisCarden thanks for the re-roll.

This works for me for Nodes and Terms but not for Vocabularies

Here is the problem(thought I don't know about the solution)

$entity_uri = $entity->uri();
    if (!$entity->isNew()) {
      if (empty($entity_uri['path'])) {
        // For entities without a URI we have to use a submit button.
        $actions['delete'] = array(
          '#value' => t('Delete'),
          '#submit' => array(
            array($this, 'delete'),
          ),
        );
      }
      else {
        // For entities with a URI we provide a link to the delete form.
        $actions['delete'] = array(
          '#type' => 'link',
          '#title' => t('Delete'),
          '#href' => $entity_uri['path'] . '/delete',
        );
      }

When I wrote the patch initially "empty($entity_uri['path'])" was true for vocabularies and other entities that didn't have a separate path for deleting the entity. Now "$entity_uri['path']" has a value(which makes sense). So the current code tries to make the link to a delete path. But the Vocabulary entities don't have a separate delete path, it is just callback to the same form. So the link gets a page not found.

The problem is that I am not sure there is way to tell in code which Entity types are going to have a separate path to delete vs just a form callback.

Couple ideas on solutions:

1. We change EntityFormController->actions() to always make "delete" a link then in VocabularyFormController->actions() we could add special logic that change delete to a button. This would have to be done for any subclass of EntityFormController were the delete doesn't have a separate path.

2. We could add a function EntityFormController->deleteMethod() (or a protected variable) that would default to 'link'. Then VocabularyFormController would have to override this to "callback" . This would have to be done for any subclass of EntityFormController were the delete doesn't have a separate path.

Any other ideas?

1 other thing I noticed(but probably should be new issue to resolve)
Drupal not puts a property on the delete button(b/c button without this patch)
$element['delete']['#button_type'] = 'danger';
As a button this puts the classes "button button-danger" on the input.
Which then moves the link over to the right of the screen(I guess because this is a destructive action)

This patch leaves that code but since it is now a link the classes don't get applied. I found that if manually add the class "button-danger" it does move the link to the right. Should this link also somehow be moved to the right?

Status:Needs work» Needs review
StatusFileSize
new7.19 KB
FAILED: [[SimpleTest]]: [MySQL] 50,403 pass(es), 71 fail(s), and 0 exception(s).
[ View ]

Ok, here is patch that implements by solution 2 from #112.
It takes away reliance on $entity->uri() and adds

  /**
   * The method which the form conroller uses for deleting entities
   *
   * Subclasses can change the value to "callback" if deletions are not done
   * via a link to separate form.
   *
   * @var string
   */
  protected $delete_method = 'link';

This then determines in EntityFormController->actions() whether to create a link or a button.

Status:Needs review» Needs work

The last submitted patch, drupal-delete-button-to-link-216064-114.patch, failed testing.

So was expecting the patch in #114 to fail. There are tons of test for "Found Delete" button. Obviously these would need to be change but again want to get some feedback on the solution before we re-write tests.

Also other subclasses of "EntityFormController" would need to set $delete_method

Title:Delete button triggers form validation - change to Delete link for entities with URIDelete button triggers server-side + HTML5 form validation; change Delete button to a link
Issue tags:-needs backport to D6, -needs backport to D7+Entity system, +html5

Clarifying the issue title.

+++ b/core/lib/Drupal/Core/Entity/EntityFormController.php
@@ -23,6 +23,16 @@ class EntityFormController implements EntityFormControllerInterface {
+  protected $delete_method = 'link';

Given that browsers will not allow to execute a button, I do not think that it makes sense to make this switchable. We should simply use Delete links everywhere.

And yes, with the recently refactored CSS styles for buttons and link-buttons, we need to apply the two classes "button" and "button-danger" to all Delete links.

Furthermore, we'd ideally leverage the new dialog facility for links, so that the delete confirmation form appears in a modal dialog.

All of this should happen by default on the EntityFormController.

VocabularyFormController has to be changed to not embed the delete confirmation form into the regular entity form. It should use a Delete link, too.

Lastly, all tests need to be updated accordingly. Instead of drupalPost(), they need to use drupalGet() + clickLink().

Issue summary:View changes

add summary

Title:Delete button triggers server-side + HTML5 form validation; change Delete button to a linkEntity form "Delete" button triggers server-side + HTML5 form validation; change "Delete" button to a link
Component:forms system» entity system
Assigned:tedbow» Xano
Issue summary:View changes
Status:Needs work» Needs review
StatusFileSize
new4.13 KB
FAILED: [[SimpleTest]]: [MySQL] 63,398 pass(es), 174 fail(s), and 8 exception(s).
[ View ]

What about this? It changes the delete button to a link and lets forms provide a route where the actual delete action takes place. It includes the conversion of NodeFormController as an example.

Status:Needs review» Needs work

The last submitted patch, 119: drupal_216064_119.patch, failed testing.

Definitely the right direction, thanks!

  1. +++ b/core/lib/Drupal/Core/Entity/EntityFormController.php
    @@ -230,30 +229,38 @@ protected function actionsElement(array $form, array &$form_state) {
    +      '#type' => 'submit',

    Can we move the #type to the top of the element definition, as usual?

  2. +++ b/core/lib/Drupal/Core/Entity/EntityFormController.php
    @@ -230,30 +229,38 @@ protected function actionsElement(array $form, array &$form_state) {
    +      $actions['delete'] = array(
    ...
    +        '#type' => 'link',

    I thought we had a #type button in D8 already?

    Alternatively, #type link works too, but then we need to manually apply a .button class + possibly also .button--danger class.

    At least I understood the term "link" in the proposal here to just mean the HTML element, not its presentation; i.e., even if the button is a link, it should still appear as a button. For that, we already have generic .button classes in D8 that also work for links.

    That said, given that this use-case most likely appears in many forms, I wonder whether we shouldn't introduce a dedicated #type link_button for that, so you (1) do not have to manually futz with classes and so you can still use the #button_type property that you already know from #type submit/button. That would be good for DX, I think.

Status:Needs work» Needs review
StatusFileSize
new4.24 KB
FAILED: [[SimpleTest]]: [MySQL] 64,216 pass(es), 171 fail(s), and 9 exception(s).
[ View ]

Re-roll, plus addressed points 1 and 2.

Status:Needs review» Needs work

The last submitted patch, 122: drupal_216064_122.patch, failed testing.

I thought we had a #type button in D8 already?

That's still just a generic HTML button, just like in D7 and before.

Status:Needs work» Needs review
StatusFileSize
new4.2 KB
new8.86 KB
FAILED: [[SimpleTest]]: [MySQL] 64,436 pass(es), 105 fail(s), and 6 exception(s).
[ View ]

Status:Needs review» Needs work

The last submitted patch, 125: drupal_216064_125.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new16.97 KB
FAILED: [[SimpleTest]]: [MySQL] 64,516 pass(es), 9 fail(s), and 1 exception(s).
[ View ]
new7.5 KB

Status:Needs review» Needs work

The last submitted patch, 127: drupal_216064_127.patch, failed testing.

The last submitted patch, 127: drupal_216064_127.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new18.39 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
new1.7 KB

This one should pass.

Issue tags:+ux

Adding tag for review to make sure we don't change UX.

Status:Needs review» Needs work

The last submitted patch, 130: drupal_216064_130.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new18.73 KB
FAILED: [[SimpleTest]]: [MySQL] 64,445 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
new1.19 KB

I fixed the link styling and a destination bug. If the tests pass, I expect we're pretty much done.

Status:Needs review» Needs work

The last submitted patch, 133: drupal_216064_133.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new18.69 KB
PASSED: [[SimpleTest]]: [MySQL] 64,496 pass(es).
[ View ]
new756 bytes

Issue tags:-ux+Usability, +html5

Thanks a lot for resurrecting this issue, @Xano! :)

Very minor remark upfront: Can we always declare the #type property in elements first, please? :)

I tested this patch via simplytest.me and it (almost) worked as expected:

  1. The Delete link has the .button--danger class (good), but misses the base .button class, so the button styling is not actually applied :-)
  2. After manually adding the base .button class, the Delete link looks and works correctly in Bartik + Stark.
  3. The Seven theme does not implement styles for .button. :-/
  4. Bartik, on the other hand, does not implement styles for .button--danger. :-/

But nevermind. Fixing the CSS of those themes is a different issue and not to be solved here. As long as the base .button class is output, we're fine.

Second, I assume that removing the local tasks/tabs is left for a separate follow-up issue?

→ Can we just add the base .button class, and if possible, also move the #type properties to the top? :)

With those two adjustments, we should be able to move forward.


Mandatory off by one error: Usage of NestedArray for merging the query destination into the route options looks a bit excessive to me — let's replace that with a simple:

<?php
$route_info
['options'] += array('query' => array());
$route_info['options']['query'] += array('destination' => ...);
?>

That said, the intention of the current code in the patch appears to be to force-replace the destination, so that is even more simple:

<?php
$route_info
['options']['query']['destination'] = ...;
?>

:-)

StatusFileSize
new18.36 KB
PASSED: [[SimpleTest]]: [MySQL] 64,631 pass(es).
[ View ]
new1.77 KB

StatusFileSize
new18.52 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 65,109 pass(es).
[ View ]
new5.39 KB

Helping you out a little bit :-)

  1. Reverted bogus chmod change to default.settings.php.
  2. Check isNew() first (performance).
  3. Specify element #type first.

While we're at it, can we add 'use-ajax' class and 'data-accepts' => 'application/vnd.drupal-modal' attribute to these links so they use a modal?

Status:Needs review» Reviewed & tested by the community

Specify element #type first.

That was really out of scope. I don't care that much in this case, but please stick to the change the issue is about.

While we're at it, can we add 'use-ajax' class and 'data-accepts' => 'application/vnd.drupal-modal' attribute to these links so they use a modal?

Shall we do that in a follow-up so it will be easier to get this committed?

Status:Reviewed & tested by the community» Needs work

Yeah, that's definitely a separate issue.

Wow, this is an oldie but a goodie! Unfortunately, does not seem to apply anymore. :(

Status:Needs work» Reviewed & tested by the community
StatusFileSize
new18.52 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 65,123 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Merged 8.x (cleanly).

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 142: entity.delete.142.patch, failed testing.

Status:Needs work» Needs review

142: entity.delete.142.patch queued for re-testing.

Status:Needs review» Needs work

The last submitted patch, 142: entity.delete.142.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new18.63 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch drupal_216064_146.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Re-roll because picture_mapping entities have been renamed.

Status:Needs review» Reviewed & tested by the community

back to rtbc
#142 was random failures

StatusFileSize
new88.4 KB

+1 to RTBC. Here's a screenshot.

StatusFileSize
new9.03 KB

I'm not a UX designer but it does make me sad in my sad place to see three different "buttons" right next to each other like this:

"That said, I *MUCH* prefer three different buttons to having to deal with the horrible annoyance of validation functions firing when deleting a node...

This issue does *not* change the design and never will. It only changes the inner workings of the button. If you want to change the design, please create a separate issue for that.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 146: drupal_216064_146.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new18.81 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,887 pass(es), 48 fail(s), and 24 exception(s).
[ View ]

The patch from #146 didn't apply. Here's a re-rolled patch.

Status:Needs review» Needs work

The last submitted patch, 152: drupal_216064_152.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new18.71 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 67,852 pass(es).
[ View ]
new2.45 KB

this should be it

Status:Needs review» Reviewed & tested by the community

Great. Back to RTBC then.

This issue does *not* change the design

Correct. I somehow missed that in HEAD it already is styled as a link despite being an HTML submit button, which is why I uploaded the screenshot in #148. Sorry for the confusion.

154: drupal_216064_154.patch queued for re-testing.

Status:Reviewed & tested by the community» Needs review

+++ b/core/modules/system/tests/modules/entity_test/lib/Drupal/entity_test/EntityTestFormController.php
@@ -108,6 +108,21 @@ public function save(array $form, array &$form_state) {
+  protected function actions(array $form, array &$form_state) {
+    $actions = parent::actions($form, $form_state);
+    $actions['delete'] = array(
+      '#type' => 'submit',
+      '#value' => $this->t('Delete'),
+      '#submit' => array(
+        array($this, 'delete'),
+      ),
+    );
+    return $actions;
+  }

I thought we are making delete's links?

StatusFileSize
new27.21 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 68,149 pass(es).
[ View ]
new8.24 KB

Status:Needs review» Reviewed & tested by the community

yes true, the test entity should follow the rest of core:)