Hello

When user types a write-in choice of his own, it could happen that this choice already exists in the poll. In this case, advpoll_form_submit() function will both create a new write-in choice and select an existing one, and then it will try to add both existing and new choices as user votes. Of course it will raise an error if maximum choices is set to 1 for this poll, but the new write-in will be added anyway. This way I can add any number of same write-ins to the poll and it would cause pain to poll moderator when he will try to merge or delete these write-ins.

The correct behaviour, as I see it, is to check if the write-in already exists and if it does - to add user vote to the existing choice instead of creating a new one with the same name.

I attached a patch doing just that. It should be noted though, that I didn't test it with every option of the module, only with common 1-choice select-or-write-in polls.

CommentFileSizeAuthor
#2 2329021-2.patch1.34 KBheykarthikwithu
write-in-duplicate.patch1.41 KBgraker
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tripper54’s picture

tripper54 CreditAttribution: tripper54 commented
Status: Needs review » Needs work
Issue tags: +Needs reroll

Looks legit, however the patch does not apply due to being rolled in the docroot, and there are a few little code issues.

  1. index 5891260..0cb6d0c 100644
--- a/sites/all/modules/advpoll/advpoll.module

    Patch should be rolled from the module folder, not the site docroot. 

  2. +++ b/sites/all/modules/advpoll/advpoll.module
@@ -705,8 +705,18 @@ function advpoll_form_submit($form, &$form_state) {
+        //we have to check if this $writein repeats previous choices. ¶

    Trailing whitespace, No leading space or capitalisation. 

  3. +++ b/sites/all/modules/advpoll/advpoll.module
@@ -705,8 +705,18 @@ function advpoll_form_submit($form, &$form_state) {
+        //If so, there's no need to create another one, we should fallback to previous

    No leading space or full stop. 

  4. +++ b/sites/all/modules/advpoll/advpoll.module
@@ -705,8 +705,18 @@ function advpoll_form_submit($form, &$form_state) {
+      if ($writein) {

    Do we really have to check this twice? 

  5. +++ b/sites/all/modules/advpoll/advpoll.module
@@ -730,9 +740,13 @@ function advpoll_form_submit($form, &$form_state) {
+  ¶

    Trailing whitespace.

heykarthikwithu’s picture

heykarthikwithu
Primary language English
Location Bengaluru 🌍
CreditAttribution: heykarthikwithu as a volunteer and at Valuebound commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
2329021-2.patch1.34 KB

As per #1.

tripper54’s picture

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

See points 2,3,4 & 5 in #1.