Problem/Motivation

When you try to remove a poll choice by saving the poll edit form with the poll choice removed, the poll choice is not actually removed from the poll.

This is because poll_update() does not actually remove the poll choice from the database.

Proposed resolution

D7/D8

Add code to poll_update() which deletes a choice from the poll_choice table when the poll is re-submitted with the title of a specific choice is blank. Also, test coverage should be added.

D8 Patch: #13

Bug also exists on D7, backport of final accepted patch will be needed.

Remaining tasks

  1. #13 (D8) needs review and testing.
  2. Once the issue is resolved for D8, we need a D7 backport.

User interface changes

None, except poll choice removal will actually work.

API changes

None.

Original report by ntigh52

When I try to remove choices from poll its not response.
I "clean" the Choice blank and still not response.
its also in my localhost.
php 5.3.4
mysql 5.1.53

CommentFileSizeAuthor
#14 1214876-test-only-should-fail.patch1.55 KBbrianV
#13 1214876-remove-poll-choice-with-tests.patch2 KBbrianV
#12 1214876-remove-poll-choice-with-tests-D8.patch2 KBbrianV
#7 1214876-poll-remove-choice-7.patch457 bytesShyamala
#4 1214876-poll-remove-choice-4.patch363 bytesShyamala
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

StevenPatz’s picture

StevenPatz
he/him
Primary language English
Location Alexandria VA
CreditAttribution: StevenPatz commented
Category: bug » support
Priority: Critical » Normal
Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

I have the same issue. D7.7

ntigh52’s picture

ntigh52 CreditAttribution: ntigh52 commented
Version: 7.4 » 7.7

I dont know how to fix it.
But To "Solve" that issue... You can create a new poll with the same question and chioces and fill the numbers of votes...
of course until this issue will fix.

Shyamala’s picture

Shyamala CreditAttribution: Shyamala commented
FileSize
1214876-poll-remove-choice-4.patch363 bytes

When the choice is empty we are not deleting the choices from the poll_choice table. Attached patch should resolve the same.

Shyamala’s picture

Shyamala CreditAttribution: Shyamala commented
Status: Active » Needs review

Changing status

Status: Needs review » Needs work

The last submitted patch, 1214876-poll-remove-choice-4.patch, failed testing.

Shyamala’s picture

Shyamala CreditAttribution: Shyamala commented
FileSize
1214876-poll-remove-choice-7.patch457 bytes

Repatching

Shyamala’s picture

Shyamala CreditAttribution: Shyamala commented
Status: Needs work » Needs review
brianV’s picture

brianV CreditAttribution: brianV commented
Status: Needs review » Needs work

I thihnk your fix is correct, but a few comments:

1. This should be done on D8 first, then backported to D7.
2. This needs test coverage, since the current tests obviously don't pick up that the choice is never deleted..
3. Your patch has incorrect whitespacing before the beginning of the db_delete() statement.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Title: Remove a poll choice » Removing a poll choice doesn't actually work
Category: support » bug
Priority: Normal » Major
Issue tags: +Needs tests

I think this is correct. An issue summary would be helpful, because I can't tease out an actual bug report of the OP, nor any of the comments until #9.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Version: 7.7 » 8.x-dev

Oops. One more.

brianV’s picture

brianV CreditAttribution: brianV commented
Issue summary: View changes

Update the summary to an actual bug report.

brianV’s picture

brianV CreditAttribution: brianV commented
Status: Needs work » Needs review
FileSize
1214876-remove-poll-choice-with-tests-D8.patch2 KB

Here is a patch that should resolve this. Includes test coverage testing that poll choices are removed successfully.

brianV’s picture

brianV CreditAttribution: brianV commented
Issue summary: View changes

fix typo

brianV’s picture

brianV CreditAttribution: brianV commented
FileSize
1214876-remove-poll-choice-with-tests.patch2 KB

Resubmitting without -D8 suffix so testbot will look at it. Identical to patch in #12...

brianV’s picture

brianV CreditAttribution: brianV commented
Issue summary: View changes

Create proper issue summary as per webchick's request.

brianV’s picture

brianV CreditAttribution: brianV commented
FileSize
1214876-test-only-should-fail.patch1.55 KB

Also uploading the test only to show that it fails without the bug fix included.

marcingy’s picture

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

Ran locally initally with path in #14 which failed as expected and then #13 which passed. Looks good to me.

marcingy’s picture

marcingy CreditAttribution: marcingy commented
Issue tags: -Needs tests

adding backport

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 1214876-test-only-should-fail.patch, failed testing.

marcingy’s picture

marcingy CreditAttribution: marcingy commented
Status: Needs work » Reviewed & tested by the community

Patch in #13 is good and rtbc

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Issue tags: +Needs backport to D7

Fixing tag.

webchick’s picture

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

Committed and pushed to 8.x and 7.x. Thanks!

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

Anonymous’s picture

(not verified) CreditAttribution: commented
Issue summary: View changes

Update link to latest patch.