Currently they cannot be deleted via the UI.
Let's make that not true!

CommentFileSizeAuthor
#13 vdc-1828414-13.patch2.54 KBtim.plunkett
#6 interdiff.txt1.44 KBtim.plunkett
#6 vdc-1828414-6-FAIL.patch911 bytestim.plunkett
#6 vdc-1828414-6-PASS.patch2.54 KBtim.plunkett
#4 vdc-1828414-4.patch2.51 KBtim.plunkett
#3 vdc-1828414-3-FAIL.patch776 bytestim.plunkett
#3 vdc-1828414-3-PASS.patch1.32 KBtim.plunkett
#2 vdc-1828414-2.patch578 bytestim.plunkett
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Issue tags: +VDC

Tagging.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Active » Needs review
FileSize
vdc-1828414-2.patch578 bytes

This was too easy.

And, it's too easy to delete a view too :)

We need a confirm_form().

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
FileSize
vdc-1828414-3-PASS.patch1.32 KB
vdc-1828414-3-FAIL.patch776 bytes

Here's a test. Confirmation form later.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Assigned: tim.plunkett » Unassigned
FileSize
vdc-1828414-4.patch2.51 KB

Okay, here's a confirmation form.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Assigned: Unassigned » tim.plunkett
Status: Needs review » Needs work

I forgot to update the test for the confirm form, and the submit handler doesn't get the view passed to it.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Assigned: tim.plunkett » Unassigned
Status: Needs work » Needs review
FileSize
vdc-1828414-6-PASS.patch2.54 KB
vdc-1828414-6-FAIL.patch911 bytes
interdiff.txt1.44 KB

Fixed the test.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented 
+++ b/core/modules/views/views_ui/views_ui.moduleundefined
@@ -752,6 +759,33 @@ function views_ui_ajax_callback(ViewExecutable $view, $op) {
+ *   The config entity being deleted.

I'm wondering whether it makes sense to describe the wrapper itself as "the config entity"

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

The rest is looking great so far!

Status: Needs review » Needs work

The last submitted patch, vdc-1828414-6-PASS.patch, failed testing.

yoroy’s picture

yoroy CreditAttribution: yoroy commented

Help me understand: is this where there actually *is* a delete link in the dropbutton but it doesn't work yet? Because that's what I'm seeing on a fresh d8. Thanks.

*EDIT*: seems like that's the situation indeed. No UI impact then it seems. Would be good to make it work yeah :)

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs work » Needs review
Issue tags: -VDC

#6: vdc-1828414-6-FAIL.patch queued for re-testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Issue tags: +VDC

#6: vdc-1828414-6-PASS.patch queued for re-testing.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
FileSize
vdc-1828414-13.patch2.54 KB 
+++ b/core/modules/views/views_ui/views_ui.moduleundefined
@@ -752,6 +759,33 @@ function views_ui_ajax_callback(ViewExecutable $view, $op) {
+ *   The config entity being deleted.
...
+  drupal_goto('admin/structure/views');

I've fixed this comment, and switched the drupal_goto to $form_state['redirect']

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

That's looking perfect now!

Status: Reviewed & tested by the community » Needs work
Issue tags: -VDC

The last submitted patch, vdc-1828414-13.patch, failed testing.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Needs work » Needs review
Issue tags: +VDC

#13: vdc-1828414-13.patch queued for re-testing.

aspilicious’s picture

aspilicious CreditAttribution: aspilicious commented
Status: Needs review » Reviewed & tested by the community

Back to rtbc. Nice and easy.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented

Yep, CRUD storage tests are already there for deleting, this adds UI tests aswell. Looks fine!

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Priority: Normal » Major
Status: Reviewed & tested by the community » Fixed

Committed to 8.x, thanks!I am going to push once I'm done with my current pile of patches.

No, I'm going to REBASE and THEN push, so I don't cause a 'merge bubble' ;) Stupid Git. ;)

Also, this is very much stretching the definition of a normal bug. ;)

Automatically closed -- issue fixed for 2 weeks with no activity.