[PATCH] support of fully anonymous polls

Looks like a nice feature. I think we should consider using a positional or kw arg rather than appending a pattern to the title. isAnonymousPoll should be is_anonymous_poll or just is_anonymous per the style used in the code.

Replying to rjollos:

Looks like a nice feature. I think we should consider using a positional or kw arg rather than appending a pattern to the title.

I wonder how such arg can be programmed in a macro, even if the whole string is splitted in a string list by separator ';'.

Good point. I seem to have remember dealing with this for another plugin. I'll do some research and get back to you.

I also have another small patch that introduces a second pattern "(-)" which means the poll is closed. If one changes from "(*)" to "(-)", the anonymous polling is closed and no input is possible henceforward. I also somehow dislike that pattern trick but it's compatible with the ';'-separator divide mechanism.

Please do go ahead and post your new patch then. If we come up with a good way to handle kw args, we can always modify it. Could you take care of the style issue not in comment:1 as well?

Here we go, with closed-poll support now. Hope you like it. This replaces this first patch here:

Index: pollmacro/trunk/tracpoll/tracpoll.py
===================================================================
--- pollmacro/trunk/tracpoll/tracpoll.py	(revision 12061)
+++ pollmacro/trunk/tracpoll/tracpoll.py	(working copy)
@@ -1,3 +1,4 @@
+import hashlib
 import os
 import re
 import pickle
@@ -31,6 +32,8 @@
                 poll = pickle.load(fd)
             finally:
                 fd.close()
+            if self.title.endswith('(-)'):
+                self.title = self.title.replace('(-)', '(*)')
             assert self.title == poll['title'], \
                    'Stored poll is not the same as this one.'
             self.votes = dict([(k, v) for k, v in poll['votes'].iteritems()
@@ -49,7 +52,7 @@
         finally:
             fd.close()
 
-    def populate(self, req):
+    def populate(self, req, is_anonymous_poll):
         """ Update poll based on HTTP request. """
         if req.args.get('poll', '') == self.key:
             vote = req.args.get('vote', '')
@@ -58,15 +61,19 @@
             if vote not in self.votes:
                 raise TracError('No such vote %s' % vote)
             username = req.authname or 'anonymous'
+            if is_anonymous_poll:
+                username = hashlib.sha1(username).hexdigest()
             for v, voters in self.votes.items():
                 if username in voters:
                     self.votes[v].remove(username)
             self.votes[vote] = self.votes[vote] + [username]
             self.save()
 
-    def render(self, env, req):
+    def render(self, env, req, is_anonymous_poll, is_closed_poll):
         out = StringIO()
         can_vote = req.perm.has_permission('POLL_VOTE')
+        if is_closed_poll:
+            can_vote = False
         if can_vote:
             out.write('<form id="%(id)s" method="get" action="%(href)s#%(id)s">\n'
                       '<input type="hidden" name="poll" value="%(id)s"/>\n'
@@ -76,6 +83,8 @@
                   ' <ul>\n'
                   % escape(self.title))
         username = req.authname or 'anonymous'
+        if is_anonymous_poll:
+            username = hashlib.sha1(username).hexdigest()
         for id, style, vote in self.vote_defs:
             hid = escape(str(id))
             out.write('<li%s>\n' % (style and ' class="%s"' % style or ''))
@@ -90,13 +99,16 @@
             else:
                 out.write(vote)
             if self.votes[id]:
-                out.write(' <span class="voters">(<span class="voter">' +
-                          '</span>, <span class="voter">'.join(self.votes[id]) +
-                          '</span>)</span>')
+                if is_anonymous_poll:
+                    out.write(' <span class="voters">(%s)</span>' % len(self.votes[id]))
+                else:
+                    out.write(' <span class="voters">(<span class="voter">' +
+                              '</span>, <span class="voter">'.join(self.votes[id]) +
+                              '</span>)</span>')
             out.write('</li>\n')
         if can_vote:
-        else:
+        elif not is_closed_poll:
             out.write("<br/><i>You don't have permission to vote. You may need to login.</i>")
         out.write(' </ul>\n</fieldset>\n')
         can_vote and out.write('</form>\n')
@@ -128,6 +140,8 @@
                       'Path where poll pickle dumps should be stored.')
 
     def expand_macro(self, formatter, name, content):
+        is_anonymous_poll = False
+        is_closed_poll = False
         req = formatter.req
         if not content:
             return system_message("A title must be provided as the first argument to the poll macro")
@@ -136,9 +150,14 @@
         if len(content) < 2:
             return system_message("One or more options must be provided to vote on.")
         title = content.pop(0)
-        return self.render_poll(req, title, content)
+        if title.endswith('(*)'):
+            is_anonymous_poll = True
+        if title.endswith('(-)'):
+            is_anonymous_poll = True
+            is_closed_poll = True
+        return self.render_poll(req, title, content, is_anonymous_poll, is_closed_poll)
 
-    def render_poll(self, req, title, votes):
+    def render_poll(self, req, title, votes, is_anonymous_poll, is_closed_poll):
         add_stylesheet(req, 'poll/css/poll.css')
         if not req.perm.has_permission('POLL_VIEW'):
             return ''
@@ -184,8 +203,8 @@
 
         poll = Poll(self.base_dir, title, all_votes)
         if req.perm.has_permission('POLL_VOTE'):
-            poll.populate(req)
-        return poll.render(self.env, req)
+            poll.populate(req, is_anonymous_poll)
+        return poll.render(self.env, req, is_anonymous_poll, is_closed_poll)
 
     # IPermissionRequestor methods
     def get_permission_actions(self):

Addon: Not tested with closing of non-anonymous polls. Could be a TODO but could give you an idea with the given patch.

Replying to anonymous:

Replying to rjollos:

Looks like a nice feature. I think we should consider using a positional or kw arg rather than appending a pattern to the title.

AFAICT +1

I wonder how such arg can be programmed in a macro, even if the whole string is splitted in a string list by separator ';'.

If I understood correctly your implicit question , that's what trac.wiki.api.parse_args is for .

PS: Notice that commas in argument values have to be escaped

Replying to olemis:

PS: Notice that commas in argument values have to be escaped

Yeah, I think this is the best way to go. I've been trying to think about how we might better handle cases that the user hasn't escaped the args, since it will particularly cause problems for users that are upgrading and never took care to escape the poll question.

I haven't tested this out yet, but I was thinking of employing one of the following solutions:

• Define the variable that determines is_anonymous_poll as a kwarg and then append all of the regular args together, since it's safe to assume that if more than one regular arg exists then poll question wasn't escaped properly.
• Define is_anonymous_poll as a regular arg, but when evaluating the output of parse_args, if there isn't a case-insensitive match to true, assume that the poll question wasn't escaped properly and append the arg to the 0th arg.

A prerequisite to setting up the above behavior is to wire up unit tests, which will be very valuable here.

Replying to olemis:

Replying to anonymous:

Replying to rjollos:

Looks like a nice feature. I think we should consider using a positional or kw arg rather than appending a pattern to the title.

AFAICT +1

Would it be compatible to older versions of poll, if we first split by ';', get a resulting string list, and then analyze the last one in the string list if it contains existing args?

Let's talk with the help of an example:

For instance, [[Poll(Please, tell me what do you think about foo?;Yes, it's OK;No, I don't like it;is_anonymous_poll=true)]] will then be split into 4 strings where the 4th one contains the arguments that we then analyze with trac.wiki.api.parse_args , and the check of the analyzing will have success on checking for known arguments.

If we just have [[Poll(Please, tell me what do you think about foo?;Yes, it's OK;No, I don't like it)]] , after the ';'-split, trac.wiki.api.parse_args will read the last partial string as (['No', 'I don't like it']), and then it can be decided as another poll answer instead of an argument list, because at least one of the strings is not known as valid argument.

Will this approach work?