Almost working patch attached - drag and drop poll module!

This patch changes a lot of lines because I needed to add a "weight" field to the form (which *should* have been there before)...

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

catch’s picture

subscribing.

quicksketch’s picture

Assigned: dmitrig01 » quicksketch
Category: task » feature
Priority: Normal » Critical
Status: Needs work » Needs review
FileSize
22.28 KB

Well I hit this one hard to try to get it completed. Unfortunately, adding ordering was not as easy as anticipated. Sorting the table rows was easy, but saving the values was another matter. Because poll module actually used the choice position (chorder column) as the key for saving results, rearranging the choices was not a safe or efficient thing to do.

This patch adds drag and drop, and also the necessary database change to support this rearranging:
- The 'chorder' column is converted to 'weight', since it no longer is the exact position for a choice.
- The 'chid' column is added to the poll_votes table and is used as the key for joining, rather than chorder.

Conveniently, a primary key was added to the poll_choices table when the schema changes went in, even though this key wasn't being used for anything. Now it is. :)

I tested this pretty well, and running an upgrade path from 5 to 6 with several polls and thousands of votes pre-registered. Browser checked in Opera, Safari, and Firefox.

quicksketch’s picture

FileSize
22.31 KB

This patch sets the weight column to size = 'tiny' and fixes a spacing issue '#parents' => array('choice', $key,'chtext'), becomes '#parents' => array('choice', $key, 'chtext'),.

Stefan Nagtegaal’s picture

Status: Needs review » Reviewed & tested by the community

Nice! This is ready to... :-)

catch’s picture

Just to note both this and the upload patch claim system_update_40 - should one or both be rerolled assuming the other also gets in? We're short for time now.

Gábor Hojtsy’s picture

Coming with multiple database changes, I hand this off to Dries for consideration. I was asked to stand by the exception list.

quicksketch’s picture

FileSize
22.35 KB

Reroll to make this system_update_6041() after upload module changes.

quicksketch’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
22.45 KB

Poll module might have gotten the pocket-veto for D6, but just in case this patch adds table checks for poll_votes and poll_choices in the system update, similar to the followup patch for upload.

Gábor Hojtsy’s picture

Version: 6.x-dev » 7.x-dev
Status: Needs review » Reviewed & tested by the community

Unfortunately, this

- adds new functionality to polls (ordering was never possible)
- changes much more in the database then what the upload did

So I need to postpone this for Drupal 7. Please understand that we try to focus on fixing what is already there in Drupal 6 and we have gone quite far with drag and drop support anyway. Let's focus on what we have there, and do not introduce new features. As far as I know, we added drag and drop to all orderable lists to wherever it was possible, and we even made uploads orderable, although they were never orderable before. Get this in early in Drupal 7.

Polls are in need of other improvements anyways. While you can add options, and even modify poll counts, you cannot remove items them for example. This shows even more when an ordering feature is added.

Gábor Hojtsy’s picture

Priority: Critical » Normal
quicksketch’s picture

Status: Reviewed & tested by the community » Needs work

No problems Gábor, I agree with the decision here. It would have been possible to change the schema less and simply add another weight column in poll module as we did with upload, but it wasn't the clean fix. Poll module is still in need of some pretty substantial changes, but we significantly improved it during this release cycle so we can be happy with that. :)

Marking CNW since it'll definitely need changing when development opens for 7 again. Thanks!

quicksketch’s picture

Status: Needs work » Needs review
FileSize
18.6 KB

Poll module is back baby :)

Here's a reroll for Drupal 7. Still fixes the database schema to use (the previously unused) chid auto-increment column instead of the position in the list of questions.

Even if Poll gets removed from core, I wouldn't be offended. But while it's here and we have a fix, might as well get it done.

Dries’s picture

Status: Needs review » Needs work

The poll module is not going to get removed from core.

I don't think the attached patch is correct: the weight field is missing from the database scheme upon a fresh install?

quicksketch’s picture

Status: Needs work » Needs review
FileSize
20.58 KB

Thanks Dries. "chorder" definitely needed to be renamed to "weight". I didn't re-roll the patch quite right. Here's one that works on a clean install of Drupal and an upgraded D6 site.

Dries’s picture

I think this looks OK, except for the choice number on the form. It's somewhat odd/silly to show the choice number. Otherwise looks great.

paul.lovvik’s picture

FileSize
21.35 KB

I have hidden the choice numbers. Note that on a new installation I found it necessary to clear the cached data before this change takes effect.

The drag and drop operation is working great!

quicksketch’s picture

Status: Needs review » Needs work

We need to remove the JS that affected the reordering of these numbers also.

+  // 1. Change the button title to reflect the behavior when using JavaScript.
+  // 2. Reorder the choice count when table rows are swapped.
+  drupal_add_js('if (Drupal.jsEnabled) {
+    $(document).ready(function() {
+      $("#edit-poll-more").val("'. t('Add another choice') .'");
+      Drupal.tableDrag["poll-choice-table"].row.prototype.onSwap = function() {
+        $("#poll-choice-table span.choice-number").each(function(n) { $(this).text((n+1) + ".") });
+      };
+    });
+  }', 'inline');
paul.lovvik’s picture

Status: Needs work » Needs review
FileSize
20.58 KB

Great point. Now choice number is not being used at all. I have removed the choice number rather than simply hiding it and accordingly I changed the CSS class name to "choice-flag".

Dries’s picture

Status: Needs review » Fixed

Committed to CVS HEAD. :-)

Anonymous’s picture

Status: Fixed » Closed (fixed)

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

Damien Tournoud’s picture

Status: Closed (fixed) » Active

I'm sorry to reopen that issue, but I need some clarifications on changes introduced in this patch.

More precisely, $poll->choice is now keyed by chid and so are poll edit and poll vote forms. That means the key is no longer relative but is now shared between all polls. You can't easily reference the first choice of a poll anymore.

This caused issue in testing, because we don't have yet a mechanism to pull form choices outside of a form. A patch was committed to fix poll.test a while ago [1], but it's nothing more than a mere hack: we just hope that the choice ids will come out the way we want them to.

Is there a way to change that behavior and to use node-relative keying instead of absolute keying there? After all the natural primary key of {poll_choice} is (nid, weight).

[1] http://drupal.org/node/260492

quicksketch’s picture

Status: Active » Closed (fixed)

we just hope that the choice ids will come out the way we want them to

We don't have to "hope" that the choice ids come out in the correct order, since they're always sorted by weight. You can easily get the first item in a list by using reset():

$first_choice = reset($node->choice);

Or you could use array_values() if you want to convert them to ordered indexes. The change is more helpful than the old approach since we can easily convert them to meaningless zero-indexed arrays at any time. In any case, I'd suggest opening a new issue rather than asking a question this issue.