Steps to reproduce:

  1. Create a new term
  2. Click 'edit' from the term list page
  3. Click the 'Delete' link in the bottom right hand corner
  4. Expected to be taken to the confirm delete page
  5. Was taken back to the term listing page and the term was not deleted
CommentFileSizeAuthor
#18 Screenshot (6).png90.47 KBshreya shetty
#18 Screenshot (5).png82.26 KBshreya shetty
#18 Screenshot (2).png86.98 KBshreya shetty
#9 2069411-term-delete-test-only-9-FAIL.patch868 byteskim.pepper
#9 2069411-term-delete-9-PASS.patch1.68 KBkim.pepper
#9 interdiff.txt1.53 KBkim.pepper
#6 patch-2069411-5.patch850 bytesTimVS
#5 patch-2069411-5.patch850 bytesTimVS

Comments

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented

Could be related to #2066671: Delete button on configure block form does nothing

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

This is actually different from the other issue, but it needs similar test coverage.

TermFormController::delete() sets the redirect, but the destination seems to override it. Not 100% sure what's going on there.

-enzo-’s picture

-enzo- commented
Status: Active » Closed (works as designed)
kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented
Status: Closed (works as designed) » Active

This does not work as designed. A term still cannot be deleted from the term edit form.

TimVS’s picture

TimVS commented
Status: Active » Needs review
StatusFileSize
new patch-2069411-5.patch850 bytes

I wrote a patch.
The problem was that the destination wasn't removed.

TimVS’s picture

TimVS commented
StatusFileSize
new patch-2069411-5.patch850 bytes

Double post due to the network

swentel’s picture

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

Patch works, but we need tests for this.

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented
Assigned: Unassigned » kim.pepper
kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt1.53 KB
new 2069411-term-delete-9-PASS.patch1.68 KB
new 2069411-term-delete-test-only-9-FAIL.patch868 bytes

The tests are currently passing because there is no 'destination' query string param being passed. Once you pass one in, it should fail.

The last submitted patch, 2069411-term-delete-9-PASS.patch, failed testing.

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented
Status: Needs work » Needs review
Issue tags: +Needs tests

#9: 2069411-term-delete-9-PASS.patch queued for re-testing.

swentel’s picture

swentel commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

Cool, thanks!

Bojhan’s picture

Bojhan commented

Just an FYI. As far I know, this button shouldn't even be there.

dries’s picture

dries commented

But why does the destination overwrite things? Not sure I understand why it needs to be unset.

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented

9: 2069411-term-delete-9-PASS.patch queued for re-testing.

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented

9: 2069411-term-delete-9-PASS.patch queued for re-testing.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 9: 2069411-term-delete-9-PASS.patch, failed testing.

shreya shetty’s picture

shreya shetty commented
Issue summary: View changes
StatusFileSize
new Screenshot (2).png86.98 KB
new Screenshot (5).png82.26 KB
new Screenshot (6).png90.47 KB

This issue wherein the delete link works as expected.I have some screenshots attached.

shreya shetty’s picture

shreya shetty commented
Status: Needs work » Closed (works as designed)