Merge lp:~acoconut/systers/flexible_essays into lp:systers

[Ana Cutillas]

After week 1. You can manually change the questions in the pck file and the program reads the dictionary and displays the questions and the boxes.

[Robin J (robin-jeffries)]

A suggestion for the future: when you put in a syslog statement, put in some identifying text, preferably the file that the message came from and what the data is. e.g.,

 syslog('info', "listinfo.py: essay question data for question %d = %s", i, mlist.questions[i][i+1][2])

In listinfo.py

I'm confused by your parameters --can you remind me what the datastructure is for mlist.questions (what the columns are? I don't understand how you can do things with mlist.questions[i][i+1][0] etc. I think this must have worked "by accident". )

I believe that mailman coding style requires 80 column line lengths. Can you shorten the long lines? (you may need to add some extra parens to make python understand that your new lines are continuation lines).

For things you are planning to do later (even if later is in the indefinite future, please start the comment with:
#TODO (your name):
that way it's easy to grep the code and find things you left for later.

As best I can tell, you didn't actually make any changes to DlistUtils.py. Is that correct? If not, can you point out the places you made changes (it looks like these were the changes to fix our nasty lower case bug; maybe you had to import that file onto your branch?)

Where do you define mlist.questions? (presumably in MailList.py, but why wasn't that included in your merge?) If you just manually created a mailing list with questions, please put that in the description of the change, showing exactly what is in that faked data structure.

Use the description of the change to tell your reviewers the things they need to know to understand this code (like the mlist.questions data structure)

review: Needs Fixing

[Ana Cutillas (acoconut)]

> Review: Needs Fixing
>
> A suggestion for the future: when you put in a syslog statement, put in
> some identifying text, preferably the file that the message came from and
> what the data is. e.g.,
>
> syslog('info', "listinfo.py: essay question data for question %d = %s",
> i, mlist.questions[i][i+1][2])
>

Mmm... I thought I had gotten rid of all the syslog statements, I was just
using them for my own testing purposes.

>
> In listinfo.py
>
> I'm confused by your parameters --can you remind me what the datastructure
> is for mlist.questions (what the columns are? I don't understand how you
> can do things with mlist.questions[i][i+1][0] etc. I think this must have
> worked "by accident". )
>

[{1: ['talk about yourself', 'talk', 'largebox', 300, 'top']}, {2: ['Are
you a woman?', 'woman', 'checkbox', 0, 'bottom']}]

So basically:

[{question_number: ['question description', 'short version of question',
'type of box', 'size of box', 'position']}]

Why do you think this works by accident?

>
> I believe that mailman coding style requires 80 column line lengths. Can
> you shorten the long lines? (you may need to add some extra parens to make
> python understand that your new lines are continuation lines).
>

Ok, I will. There are some files in the systers folders that have very long
lines too, should I shorten those too? I mean code that I didn't write but
has very long lines.

>
> For things you are planning to do later (even if later is in the
> indefinite future, please start the comment with:
> #TODO (your name):
> that way it's easy to grep the code and find things you left for later.
>
> As best I can tell, you didn't actually make any changes to DlistUtils.py.
> Is that correct? If not, can you point out the places you made changes
> (it looks like these were the changes to fix our nasty lower case bug;
> maybe you had to import that file onto your branch?)
>

I only modified listinfo.py for the changes for the first week.

>
> Where do you define mlist.questions? (presumably in MailList.py, but why
> wasn't that included in your merge?) If you just manually created a
> mailing list with questions, please put that in the description of the
> change, showing exactly what is in that faked data structure.
>

mlist.questions was something that mailman was already using. It used to
get the one question from the pickle file, now it gets the data structure I
posted earlier.

> Use the description of the change to tell your reviewers the things they
> need to know to understand this code (like the mlist.questions data
> structure)
>
>
> Please, let me know if all this made or not sense and I'll make the
changes and resubmit .

> --
> https://code.launchpad.net/~acoconut/systers/flexible_essays/+merge/111405
> You are the owner of lp:~acoconut/systers/flexible_essays.
>

[Robin J (robin-jeffries)]

1. It's fine to leave the syslog statements in (you can make a grep for them at the end of the summer). If they happen to make it into production, we turn DEBUG off there, so there is no harm. And you never know when a bug will come back to you

2. For the way you march down the mlist.questions data structure, I don't understand how you got it to work. If I understand it correctly, given

[{1: ['talk about yourself', 'talk', 'largebox', 300, 'top']}, {2: ['Are
you a woman?', 'woman', 'checkbox', 0, 'bottom']}]

your tests are for mlist.questions[i][i+1][2]

the first time through that is mlist.questions[0][1][2] (in line 15 in the diff) -- that would be 'talk' above.)

The next time through it is mlist.questions[1][2][2] -- which doesn't exist, right? shouldn't your test (and other items) be mlist.questions[i][1][2]?

(also, I missed this earlier. It would be better code to have your loop statement in line 12 in the diff be
  for question in mlist.questions:
(range has to take that number an convert it to a location in the data structure, and this just marches down the data structure)

If you do that, then the test becomes something like
if question[1][2] ==...

(see how this is clearer? You could make it even clearer by making this a named structure (where you might have question.boxtype for the above), but for the small number of times you are going to reference it, it may not be worth it. But we may want to revisit this when we see how complex the admin UI is and how much improvement in clarity it gives)

3. For the updating of mlist.questions. I understand more now, but I think there is going to be an issue when you do the admin GUI -- this is not where things that are changed in the admin gui are stored (they are not part of mlist). This is not something you introduced, but it is something you will need to correct when you get to doing the admin GUI for this. Let's leave that issue until later.

[Ana Cutillas (acoconut)]

Yes, I see what you meant about the for loop and I just got done fixing it.
I had to add an index though, I'm going to push the new version so that you
can see it. I couldn't figure out a way of doing it without an index.

As for the lines length, there are lines a lot longer that I didn't write.
I made mine shorter, let me know if I should change the ones I didn't write
too.

Thanks Robin! I love learning little things like the for loop thing. :)

On Tue, Jun 26, 2012 at 7:26 AM, Robin J <email address hidden> wrote:

> 1. It's fine to leave the syslog statements in (you can make a grep for
> them at the end of the summer). If they happen to make it into production,
> we turn DEBUG off there, so there is no harm. And you never know when a
> bug will come back to you
>
> 2. For the way you march down the mlist.questions data structure, I don't
> understand how you got it to work. If I understand it correctly, given
>
> [{1: ['talk about yourself', 'talk', 'largebox', 300, 'top']}, {2: ['Are
> you a woman?', 'woman', 'checkbox', 0, 'bottom']}]
>
> your tests are for mlist.questions[i][i+1][2]
>
> the first time through that is mlist.questions[0][1][2] (in line 15 in
> the diff) -- that would be 'talk' above.)
>
> The next time through it is mlist.questions[1][2][2] -- which doesn't
> exist, right? shouldn't your test (and other items) be
> mlist.questions[i][1][2]?
>
> (also, I missed this earlier. It would be better code to have your loop
> statement in line 12 in the diff be
> for question in mlist.questions:
> (range has to take that number an convert it to a location in the data
> structure, and this just marches down the data structure)
>
> If you do that, then the test becomes something like
> if question[1][2] ==...
>
> (see how this is clearer? You could make it even clearer by making this a
> named structure (where you might have question.boxtype for the above), but
> for the small number of times you are going to reference it, it may not be
> worth it. But we may want to revisit this when we see how complex the
> admin UI is and how much improvement in clarity it gives)
>
> 3. For the updating of mlist.questions. I understand more now, but I
> think there is going to be an issue when you do the admin GUI -- this is
> not where things that are changed in the admin gui are stored (they are not
> part of mlist). This is not something you introduced, but it is something
> you will need to correct when you get to doing the admin GUI for this.
> Let's leave that issue until later.
>
> --
> https://code.launchpad.net/~acoconut/systers/flexible_essays/+merge/111405
> You are the owner of lp:~acoconut/systers/flexible_essays.
>

[lp:~acoconut/systers/flexible_essays]

84. By Ana Cutillas <email address hidden> on 2012-06-26

Fixed the for loop that iterates through the list of questions, the
lines' length and other format issues.

[Robin J (robin-jeffries)]

That's what code reviews are for.

I will look at the changed code as soon as I can, but I have out of town
guests tonight, and I don't know if I can steal some cycles from work or
not.

Robin

On Tue, Jun 26, 2012 at 10:34 AM, Ana Cutillas <email address hidden> wrote:

> Yes, I see what you meant about the for loop and I just got done fixing it.
> I had to add an index though, I'm going to push the new version so that you
> can see it. I couldn't figure out a way of doing it without an index.
>
> As for the lines length, there are lines a lot longer that I didn't write.
> I made mine shorter, let me know if I should change the ones I didn't write
> too.
>
> Thanks Robin! I love learning little things like the for loop thing. :)
>
> On Tue, Jun 26, 2012 at 7:26 AM, Robin J <email address hidden> wrote:
>
> > 1. It's fine to leave the syslog statements in (you can make a grep for
> > them at the end of the summer). If they happen to make it into
> production,
> > we turn DEBUG off there, so there is no harm. And you never know when a
> > bug will come back to you
> >
> > 2. For the way you march down the mlist.questions data structure, I
> don't
> > understand how you got it to work. If I understand it correctly, given
> >
> > [{1: ['talk about yourself', 'talk', 'largebox', 300, 'top']}, {2: ['Are
> > you a woman?', 'woman', 'checkbox', 0, 'bottom']}]
> >
> > your tests are for mlist.questions[i][i+1][2]
> >
> > the first time through that is mlist.questions[0][1][2] (in line 15 in
> > the diff) -- that would be 'talk' above.)
> >
> > The next time through it is mlist.questions[1][2][2] -- which doesn't
> > exist, right? shouldn't your test (and other items) be
> > mlist.questions[i][1][2]?
> >
> > (also, I missed this earlier. It would be better code to have your loop
> > statement in line 12 in the diff be
> > for question in mlist.questions:
> > (range has to take that number an convert it to a location in the data
> > structure, and this just marches down the data structure)
> >
> > If you do that, then the test becomes something like
> > if question[1][2] ==...
> >
> > (see how this is clearer? You could make it even clearer by making this
> a
> > named structure (where you might have question.boxtype for the above),
> but
> > for the small number of times you are going to reference it, it may not
> be
> > worth it. But we may want to revisit this when we see how complex the
> > admin UI is and how much improvement in clarity it gives)
> >
> > 3. For the updating of mlist.questions. I understand more now, but I
> > think there is going to be an issue when you do the admin GUI -- this is
> > not where things that are changed in the admin gui are stored (they are
> not
> > part of mlist). This is not something you introduced, but it is
> something
> > you will need to correct when you get to doing the admin GUI for this.
> > Let's leave that issue until later.
> >
> > --
> >
> https://code.launchpad.net/~acoconut/systers/flexible_essays/+merge/111405
> > You are the owner of lp:~acoconut/systers/flexible_essays.
> >
>
> --
> https://code.launchpad.net/~acoconut/systers/flexible_essays/+merge/111405
> You are reviewing the pro...

[lp:~acoconut/systers/flexible_essays]

85. By Ana Cutillas <email address hidden> on 2012-06-27

Changed the data structure to store the questions. Now it looks like:

[['Talk about yourself', 'talk', 'largebox', 300, 'top'], ['Are you a woman?', 'woman', 'checkbox', 0, 'bottom']]

86. By Ana Cutillas <email address hidden> on 2012-06-28

Changed the name of the variable questions to html_questions.

87. By Ana Cutillas <email address hidden> on 2012-07-04

Added changes to the DlistUtils.py file to handle the database
operations to the new table form_question.

88. By Ana Cutillas <email address hidden> on 2012-07-25

Added the database backend and the functionality to connect forms to the database.

89. By Ana Cutillas <email address hidden> on 2012-07-27

Made changes to the database and to the way we store the answers. Made
changes to the way we handle the information coming from the form.

90. By Ana Cutillas <email address hidden> on 2012-07-27

Cleaned up listinfo.py

91. By Ana Cutillas <email address hidden> on 2012-07-28

Cleaned up.

92. By Ana Cutillas <email address hidden> on 2012-07-31

Cleaned up DlistUtils and changed the error message in subscribe. Added
comments.

93. By Ana Cutillas <email address hidden> on 2012-08-01

Removed the unused function get_answer from DlistUtils.py

94. By Ana Cutillas <email address hidden> on 2012-08-04

Added changes to show the new essays in the pending subscriptions table.

Handle the possibility of a user trying to subscribe twice before the
subscription is reviewed.

95. By Ana Cutillas <email address hidden> on 2012-08-04

Added the functionality to check that the answers aren't longer than
what the database can hold.

96. By Ana Cutillas <email address hidden> on 2012-08-04

Added the error message saying that the user hasn't been subscribe to
the list when a user tries to subscribe with a pending subscription.

97. By Ana Cutillas <email address hidden> on 2012-08-07

Made the changes suggested by Robin: concerning error messages and
asserting the size of the answers.

Added the code to mark answers as accepted in the database when a user
is accepted.

98. By Ana Cutillas <email address hidden> on 2012-08-09

Changed the logic in the checking of answers in subscribe.py
Cleaned up DlistUtils.py

99. By Ana Cutillas <email address hidden> on 2012-08-09

Added support for setting answers to rejected in the database.

Unmerged revisions

99. By Ana Cutillas <email address hidden> on 2012-08-09

Added support for setting answers to rejected in the database.

98. By Ana Cutillas <email address hidden> on 2012-08-09

Changed the logic in the checking of answers in subscribe.py
Cleaned up DlistUtils.py

97. By Ana Cutillas <email address hidden> on 2012-08-07

Made the changes suggested by Robin: concerning error messages and
asserting the size of the answers.

Added the code to mark answers as accepted in the database when a user
is accepted.

96. By Ana Cutillas <email address hidden> on 2012-08-04

Added the error message saying that the user hasn't been subscribe to
the list when a user tries to subscribe with a pending subscription.

95. By Ana Cutillas <email address hidden> on 2012-08-04

Added the functionality to check that the answers aren't longer than
what the database can hold.

94. By Ana Cutillas <email address hidden> on 2012-08-04

Added changes to show the new essays in the pending subscriptions table.

Handle the possibility of a user trying to subscribe twice before the
subscription is reviewed.

93. By Ana Cutillas <email address hidden> on 2012-08-01

Removed the unused function get_answer from DlistUtils.py

92. By Ana Cutillas <email address hidden> on 2012-07-31

Cleaned up DlistUtils and changed the error message in subscribe. Added
comments.

91. By Ana Cutillas <email address hidden> on 2012-07-28

Cleaned up.

90. By Ana Cutillas <email address hidden> on 2012-07-27

Cleaned up listinfo.py

=== modified file 'Mailman/Cgi/listinfo.py'
--- Mailman/Cgi/listinfo.py	2009-08-14 15:41:17 +0000
+++ Mailman/Cgi/listinfo.py	2012-06-26 17:39:24 +0000
@@ -22,6 +22,7 @@
 import os
 import cgi
 
+from Mailman import DlistUtils
 from Mailman import mm_cfg
 from Mailman import Utils
 from Mailman import MailList
@@ -36,7 +37,6 @@
 i18n.set_language(mm_cfg.DEFAULT_SERVER_LANGUAGE)
 
 
-

 def main():
     parts = Utils.GetPathPieces()
     if not parts:
@@ -62,7 +62,6 @@
     list_listinfo(mlist, language)
 
 
-

 def listinfo_overview(msg=''):
     # Present the general listinfo overview
     hostname = Utils.get_domain()
@@ -152,7 +151,6 @@
     print doc.Format()
 
 
-

 def list_listinfo(mlist, lang):
     # Generate list specific listinfo
     doc = HeadlessDocument()
@@ -190,9 +188,24 @@
     # If activated, the content and presence of questions and a field to answer them. Added by Systers.
     try:
         if mlist.essay_enabled:
-            replacements['<mm-questions>'] = """<TR>
-            <TD COLSPAN="3">""" + mlist.questions
-            replacements['<mm-essay-box>'] = TextArea('essay', '', rows=8, cols=60).Format() + """        </TD></TR>"""# Added to support subscribe essays
+            questions = [] # Added to support subscribe essays
+            index = 1
+            for question in mlist.questions:
+                questions.append("""<TR> 
+                <TD COLSPAN="3">""" + question[index][0] + """</TD>""")
+                if (question[index][2] == 'smallbox'):
+                    questions.append("""<TD>""" + TextArea(question[index][1],\
+                            '', rows=1, cols=60).Format() + """</TD></TR>""") 
+                elif (question[index][2] == 'largebox'):
+                    questions.append("""<TD>""" + TextArea(question[index][1], \
+                            '', rows=8, cols=60).Format() + """</TD></TR>""") 
+                elif (question[index][2] == 'checkbox'):
+                    #TODO (Ana Cutillas): consider creating a class like TextArea for checkboxes?
+                    questions.append("""<TD><input type='checkbox' name='""" + \
+                            question[index][1] + """' value ='""" + \
+                            question[index][1] + """'></TD></TR>""") 
+                index = index + 1
+            replacements['<mm-questions>'] = ''.join(questions)
         else:
             replacements['<mm-questions>'] = ''
             replacements['<mm-essay-box>'] = ''

=== modified file 'Mailman/DlistUtils.py'
--- Mailman/DlistUtils.py	2010-01-31 20:29:48 +0000
+++ Mailman/DlistUtils.py	2012-06-26 17:39:24 +0000
@@ -40,17 +40,17 @@
         if self == classObject:
             if self.store == None:#for the case when same object is used for calling more than one functions
                 self.store = Store(self.database)
-                result = f(self,*args)
+                result = f(self, *args)
                 self.store.commit()
                 self.store.close()
                 self.store = None
             else:#To handle the nesting issue
-                result = f(self,*args)
+                result = f(self, *args)
                 self.store.commit()
         else:#for the case whenever a new object calls its decorated member function
             classObject = self
             self.store = Store(self.database)
-            result = f(self,*args)
+            result = f(self, *args)
             self.store.commit()
             self.store.close()
             self.store = None
@@ -60,14 +60,14 @@
 #Define all the classes corresponding to the tables in the database
 class Subscriber(object):
     __storm_table__ = "subscriber"
-    subscriber_id = Int(primary = True,default = AutoReload)
+    subscriber_id = Int(primary = True, default = AutoReload)
     mailman_key = Unicode()
     preference = Int(default = 1)
     format = Int(default = 3)
     deleted = Bool(default = False)
     suppress = Int(default = 0)
 
-    def __init__(self,mlist):
+    def __init__(self, mlist):
         self.mlist = mlist
         self.database = getConn(mlist)
 
@@ -76,11 +76,11 @@
         if addr == None:
             return None
 
-        command = "result = self.store.find(Subscriber,Subscriber.mailman_key == unicode(addr.lower(),'utf-8'))\na = [(subscriber.subscriber_id) for subscriber in result]\n"
+        command = "result = self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(addr.lower(),'utf-8'))\na = [(subscriber.subscriber_id) for subscriber in result]\n"
         if DEBUG_MODE:
             syslog('info', "DlistUtils:(getSubscriber_id_raw)executing query:\n%s", command)
         #storm recognizes unicode only
-        result = self.store.find(Subscriber,Subscriber.mailman_key == unicode(addr.lower(),"utf-8")) 
+        result = self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(addr,"utf-8")) 
         a = [(subscriber.subscriber_id) for subscriber in result]
         if DEBUG_MODE:
             syslog('info', 'value of a is: %s\n', a)
@@ -136,70 +136,70 @@
     @decfunc
     def setDisable(self, member, flag):
         """Disable/enable delivery based on mm_cfg.DisableDelivery"""
-        command = "result = self.store.find(Subscriber,Subscriber.mailman_key == unicode(member,'utf-8'))\noldval = [(subscriber.suppress) for subscriber in result]\n"
+        command = "result = self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(member,'utf-8'))\noldval = [(subscriber.suppress) for subscriber in result]\n"
         if DEBUG_MODE:
-            syslog('info', 'DlistUtils(setDisable):Executing query:\n%s\n Member whoes suppress value is to be found \n %s', command,member)
-        result = self.store.find(Subscriber,Subscriber.mailman_key == unicode(member,"utf-8"))
+            syslog('info', 'DlistUtils(setDisable):Executing query:\n%s\n Member whose suppress value is to be found \n %s', command, member)
+        result = self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(member,"utf-8"))
         oldval = [(subscriber.suppress) for subscriber in result]
 	if oldval == []:
 	    if DEBUG_MODE:
 	        syslog('info','oldval is an empty list.\nThis can happen either because of\n 1)Permission issues (Do a: bin/check_perms)\n 2)Inconsistency between database and pickle files (A user is in the database but not in pickle files or vice versa,Do a bin/find_problems.py)')
         oldval = oldval[0]
 	if DEBUG_MODE:
-            syslog('info','the value of oldval is %s:',oldval)
+            syslog('info','the value of oldval is %s:', oldval)
 
         if flag:
             newval = oldval | 1          # Disable delivery
         else:
             newval = oldval & ~1          # Enable delivery
         if DEBUG_MODE:
-            syslog('info','the value of newval is %s:',newval)
-        command = "self.store.find(Subscriber,Subscriber.mailman_key == unicode(member,'utf-8')).set(suppress = newval)\n"
+            syslog('info','the value of newval is %s:', newval)
+        command = "self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(member,'utf-8')).set(suppress = newval)\n"
         if DEBUG_MODE:
             syslog('info', 'DlistUtils(setDisable):Executing query:\n%s', command)
-        self.store.find(Subscriber,Subscriber.mailman_key == unicode(member,"utf-8")).set(suppress = newval)       
+        self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(member,"utf-8")).set(suppress = newval)       
 
     @decfunc
     def setDigest(self, member, flag):
         """Disable/enable delivery based on user digest status"""
 
-        command = "result = self.store.find(Subscriber,Subscriber.mailman_key == unicode(member,'utf-8'))\noldval = [(subscriber.suppress) for subscriber in result]\n"
+        command = "result = self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(member,'utf-8'))\noldval = [(subscriber.suppress) for subscriber in result]\n"
         if DEBUG_MODE:
             syslog('info', 'DlistUtils(setDigest):Executing query:\n%s', command)
-        result = self.store.find(Subscriber,Subscriber.mailman_key == unicode(member,"utf-8"))
+        result = self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(member,"utf-8"))
         oldval = [(subscriber.suppress) for subscriber in result]
 	if oldval == []:
 	    if DEBUG_MODE:
 	        syslog('info','oldval is an empty list.\nThis can happen either because of\n 1)permission issues (Do a: bin/check_perms)\n 2)Inconsistency between database and pickle files (A user is in the database but not in pickle files or vice versa,Do a bin/find_problems.py)')
         oldval = oldval[0]
         if DEBUG_MODE:
-            syslog('info','value of oldval %s:',oldval)
+            syslog('info','value of oldval %s:', oldval)
 
         if flag:
             newval = oldval | 2          # Suppress delivery (in favor of digests)
         else:
             newval = oldval & ~2          # Enable delivery (instead of digests)
 
-        command = "self.store.find(Subscriber,Subscriber.mailman_key == unicode(member,'utf-8')).set(suppress = newval)\n"
+        command = "self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(member,'utf-8')).set(suppress = newval)\n"
         if DEBUG_MODE:
             syslog('info', 'DlistUtils(setDigest):Executing query:\n%s', command)
-        self.store.find(Subscriber,Subscriber.mailman_key == unicode(member,"utf-8")).set(suppress = newval)
+        self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(member,"utf-8")).set(suppress = newval)
 
     @decfunc
     def changePreference(self, member, preference):
         """Change a user's default preference for new threads."""
-        command = "self.store.find(Subscriber,Subscriber.mailman_key == unicode(member,'utf-8')).set(preference = preference)\n"
+        command = "self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(member,'utf-8')).set(preference = preference)\n"
         if DEBUG_MODE:
             syslog('info', 'DlistUtils(changePreference):Executing query:\n%s', command)    
-        self.store.find(Subscriber,Subscriber.mailman_key == unicode(member,"utf-8")).set(preference = preference)
+        self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(member,"utf-8")).set(preference = preference)
 
     @decfunc
     def changeFormat(self, member, format):
         """Change a user's preferred delivery format (plain text and/or html)"""
-        command = "self.store.find(Subscriber,Subscriber.mailman_key == unicode(member,'utf-8')).set(format = format)\n"
+        command = "self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(member,'utf-8')).set(format = format)\n"
         if DEBUG_MODE:
             syslog('info', 'DlistUtils(changeFormat):Executing query:\n%s', command)
-        self.store.find(Subscriber,Subscriber.mailman_key == unicode(member,"utf-8")).set(format = format)    
+        self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(member,"utf-8")).set(format = format)    
 
     ### We have to watch out for a tricky case (that actually happened):
     ### User [e-mail] changes her address to something already in the
@@ -207,32 +207,32 @@
     @decfunc
     def changeAddress(self, oldaddr, newaddr):
         """Change email address in SQL database"""
-        #if DEBUG_MODE:
-        #syslog('info', "Changing email address on %s from '%s' to '%s'", self.mlist.internal_name(), oldaddr, newaddr)
+        if DEBUG_MODE:
+        	syslog('info', "Changing email address on %s from '%s' to '%s'", self.mlist.internal_name(), oldaddr, newaddr)
         ## Check if newaddr is in sql database        
-        num_matches = self.store.find(Subscriber,Subscriber.mailman_key == unicode(newaddr,"utf-8")).count()
+        num_matches = self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(newaddr.lower(),"utf-8")).count()
 
         if num_matches > 1:
-            #syslog('error', 'Multiple users with same key (%s): %s', self.mlist.internal_name(), newaddr)
+            syslog('error', 'Multiple users with same key (%s): %s', self.mlist.internal_name(), newaddr)
             return
         if num_matches == 1:
             self.mergeSubscribers(oldaddr, newaddr)
 
-        command = "num_matches = self.store.find(Subscriber,Subscriber.mailman_key == unicode(newaddr,'utf-8')).count()\nself.store.find(Subscriber,Subscriber.mailman_key == unicode(oldaddr,'utf-8')).set(mailman_key = unicode(newaddr,'utf-8'))\nself.store.find(Subscriber,Subscriber.mailman_key == unicode(newaddr,'utf-8')).set(deleted = False)\n"
+        command = "num_matches = self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(newaddr,'utf-8')).count()\nself.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(oldaddr,'utf-8')).set(mailman_key = unicode(newaddr,'utf-8'))\nself.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(newaddr.lower(),'utf-8')).set(deleted = False)\n"
         if DEBUG_MODE:
             syslog('info', 'DlistUtils(changeAddress):Executing query:\n%s', command)
-        self.store.find(Subscriber,Subscriber.mailman_key == unicode(oldaddr,"utf-8")).set(mailman_key = unicode(newaddr,"utf-8")) 
+        self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(oldaddr,"utf-8")).set(mailman_key = unicode(newaddr,"utf-8")) 
         self.store.commit()            
-        self.store.find(Subscriber,Subscriber.mailman_key == unicode(newaddr,"utf-8")).set(deleted = False)
+        self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(newaddr.lower(),"utf-8")).set(deleted = False)
 
     @decfunc
     def unsubscribeFromList(self, key):
         """Indicate that a user has unsubscribed by setting the deleted flag in the subscriber table."""
 
-        command = "self.store.find(Subscriber,Subscriber.mailman_key == unicode(key,'utf-8')).set(deleted = True)\nself.store.find(Alias,Alias.subscriber_id == subscriber_id).remove()\n"
+        command = "self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(key,'utf-8')).set(deleted = True)\nself.store.find(Alias,Alias.subscriber_id == subscriber_id).remove()\n"
         if DEBUG_MODE:
             syslog('info', 'DlisUtils(unsubscribeFromList):Executing query:\n%s', command)          
-        self.store.find(Subscriber,Subscriber.mailman_key == unicode(key,"utf-8")).set(deleted = True)
+        self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(key,"utf-8")).set(deleted = True)
         # get the subscriber id from the mailman_key & delete all aliases  
         subscriber_id = self.getSubscriber_id_raw(key)
 
@@ -245,20 +245,27 @@
     def subscribeToList(self, key):
         """Add a member to the subscriber database, or change the record from deleted if it was already present."""
 
-        command = "count = (self.store.find(Subscriber,Subscriber.mailman_key == unicode(key,'utf-8'))).count()\n"
+        command = "count = (self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(key.lower(),'utf-8'))).count()\n"
         if DEBUG_MODE:
             syslog('info', 'DlistUtils(subscribeToList):Executing query:\n%s', command)
         # First see if member is subscribed with deleted field = false
-        count = (self.store.find(Subscriber,Subscriber.mailman_key == unicode(key,"utf-8"))).count()
+        count = (self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(key.lower(),"utf-8"))).count()
 
         if count:
-            command = "self.store.find(Subscriber,Subscriber.mailman_key == unicode(key,'utf-8')).set(deleted = False)"
+            command = "self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(key.lower(),'utf-8')).set(deleted = False)"
             if DEBUG_MODE:
                 syslog('info', 'DlistUtils(subscribeToList):Executing query:\n%s', command)
-            self.store.find(Subscriber,Subscriber.mailman_key == unicode(key,"utf-8")).set(deleted = False)
+            self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(key.lower(),"utf-8")).set(deleted = False)
+
+	    result = self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(key.lower(),"utf-8"))
+            Email = [(subscriber.mailman_key) for subscriber in result]
+	    Email = Email[0]
+	    if Email != key: #That is if one was in lowercase and other in uppercase then update mailman_key with case sensitivity same as key
+		self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(key.lower(),"utf-8")).set(mailman_key = unicode(key,"utf-8"))
+	    
         else:
             # format is text-only (1) by default
-            command = "subscriber = self.store.add(Subscriber())\nsubscriber.mailman_key = unicode(key,'utf-8')\nsubscriber.preference = 1\nsubscriber.deleted = False\nsubscriber.format = 1\nsubscriber.suppress = 0"
+            command = "subscriber = self.store.add(Subscriber())\nmailman_key = unicode(key,'utf-8')\npreference = 1\ndeleted = False\nformat = 1\nsuppress = 0"
             if DEBUG_MODE:
                 syslog('info', 'DlistUtils(subscribeToList)Executing query:\n%s', command)
             self.mailman_key = unicode(key,"utf-8")
@@ -273,12 +280,12 @@
         # use the original subscriber id as the ID going forward
         if DEBUG_MODE:
             syslog('info', 'Executing commands of mergeSubscribers:')
-        result = self.store.find(Subscriber,Subscriber.mailman_key == unicode(oldaddr,"utf-8"))
+        result = self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(oldaddr,"utf-8"))
         new_id = [(subscriber.subscriber_id) for subscriber in result]
         new_id = new_id[0]
         if DEBUG_MODE:
             syslog('info', 'the value of new_id: %s', new_id)
-        result = self.store.find(Subscriber,Subscriber.mailman_key == unicode(newaddr,"utf-8"))
+        result = self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(newaddr.lower(),"utf-8"))
         obsolete_id = [(subscriber.subscriber_id) for subscriber in result]
         obsolete_id = obsolete_id[0]    
         if DEBUG_MODE:
@@ -291,13 +298,13 @@
 
 class Message(object):
     __storm_table__ = "message"
-    message_id = Int(primary = True,default = AutoReload)
+    message_id = Int(primary = True, default = AutoReload)
     sender_id = Int()
-    subscriber = Reference(sender_id,Subscriber.subscriber_id)
+    subscriber = Reference(sender_id, Subscriber.subscriber_id)
     subject = Unicode()
     thread_id = Int()
 
-    def __init__(self,mlist):
+    def __init__(self, mlist):
         self.mlist = mlist
         self.database = getConn(mlist)
 
@@ -314,7 +321,7 @@
             threadID = -1
         command = "message = self.store.add(Message())\nmessage.sender_id = senderID\nmessage.thread_id = threadID\nmessage.subject = unicode(subject,'utf-8')\nmessageID = self.store.find(Message).max(Message.message_id)\n"
         if DEBUG_MODE:
-            syslog('info','DlistUtils:(createMessage)executing query:\n%s',command)
+            syslog('info','DlistUtils:(createMessage)executing query:\n%s', command)
         #message_id has autoreload set,its value will be serially updated in database 
         self.sender_id = senderID
         self.thread_id = threadID
@@ -322,19 +329,19 @@
         self.store.add(self) 
         messageID = self.store.find(Message).max(Message.message_id)     
         if DEBUG_MODE:
-            syslog('info','Result of query(messageID) is: %s\n',messageID)
+            syslog('info','Result of query(messageID) is: %s\n', messageID)
         return messageID
 
 class Thread(object):
     __storm_table__ = "thread"
-    thread_id = Int(primary = True,default = AutoReload)
+    thread_id = Int(primary = True, default = AutoReload)
     thread_name = Unicode()
     base_message_id = Int()
-    message = Reference(base_message_id,Message.message_id)
+    message = Reference(base_message_id, Message.message_id)
     status = Int(default = 0)
     parent = Int()
 
-    def __init__(self,mlist):
+    def __init__(self, mlist):
         self.mlist = mlist
         self.database = getConn(mlist)
 
@@ -347,7 +354,7 @@
         senderID = subscriber.getSubscriber_id(msg, msgdata, safe=1, loose=1)
         command = "thread = self.store.add(Thread())\nthread.base_message_id = msgdata['message_id']\nthreadID = self.store.find(Thread).max(Thread.thread_id)\nself.store.find(Message,Message.message_id == msgdata['message_id']).set(thread_id = threadID)\n"
         if DEBUG_MODE:
-            syslog('info','DlistUtils:(createThread)executing query:\n%s',command)
+            syslog('info','DlistUtils:(createThread)executing query:\n%s', command)
         self.base_message_id = msgdata['message_id']
         self.store.add(self)
         threadID = self.store.find(Thread).max(Thread.thread_id)          
@@ -370,7 +377,7 @@
         threadBase = threadBase[:13]
         command = "num = self.store.find(Thread,Thread.thread_name == unicode(threadBase,'utf-8')).count()\nself.store.find(Thread,Thread.thread_id == threadID).set(thread_name = threadName)\n"
         if DEBUG_MODE:
-            syslog('info','DlistUtils:(createThread)executing query:\n%s',command)
+            syslog('info','DlistUtils:(createThread)executing query:\n%s', command)
         # If the threadBase is not unique, make threadBase unique by appending a number
         num = self.store.find(Thread,Thread.thread_name.like(unicode(threadBase + "%","utf-8"))).count()
         if not num:
@@ -405,26 +412,24 @@
         msg['To'] =  '%s+%s@%s' % (self.mlist.internal_name(),
                              	   name,
                              	   self.mlist.host_name)    
-        for i in (1,2):
+        for i in (1, 2):
             # different footers for different prefs, so we need to queue separately
             if(i==1):
                 #For condition where preference = True
                 pref=True
-           #     command = "lists = [(subscriber.mailman_key.encode('utf-8')) for subscriber in result_new_sql]\n"
-            #    if DEBUG_MODE:
-             #       syslog('info', 'DlistUtils:(newThread)executing query:\n%sfor pref = true\n', command)
+                if DEBUG_MODE:
+                    syslog('info', 'DlistUtils:(newThread)executing query:\nfor pref = true\n\n')
             if(i==2):
                 #For condition where preference = False
                 pref=False
-              #  command = "lists = [(subscriber.mailman_key.encode('utf-8')) for subscriber in result_new_sql]\n"
-               # if DEBUG_MODE:
-                #    syslog('info', 'DlistUtils:(newThread)executing query:\n%sfor pref = false\n', command)
+                if DEBUG_MODE:
+                    syslog('info', 'DlistUtils:(newThread)executing query:\nfor pref = false\n\n')
 
             #Execute a SELECT statement, to find the list of matching subscribers.
             result_new_sql = self.store.find(Subscriber,And(Subscriber.preference == pref,Subscriber.deleted == False,Subscriber.suppress == 0))
             lists = [(subscriber.mailman_key.encode('utf-8')) for subscriber in result_new_sql]
             if DEBUG_MODE:
-                syslog('info', 'value of lists: %s\n', lists)
+                syslog('info','value of lists: %s\n', lists)
             self.email_recipients(msg, msgdata, lists, pref)
 
         # Make original message go to nobody (but be archived)
@@ -514,12 +519,12 @@
         override = Override(self.mlist)
         override.override(msg, msgdata, 0)
 
-    def alphanumericOnly(self,s):
+    def alphanumericOnly(self, s):
         """Filter any non-letter characters from a string"""
         result = [letter for letter in s if letter in string.ascii_letters or letter in string.digits]
         return string.join(result, '')
 
-    def subjectToName(self,subject, threadID):
+    def subjectToName(self, subject, threadID):
         """Return a lower-case name for a new thread based on the subject, if present, or on the threadID"""
         result = None
 
@@ -589,9 +594,9 @@
     __storm_table__ = "alias"    
     pseudonym = Unicode(primary = True)
     subscriber_id = Int()
-    subscriber = Reference(subscriber_id,Subscriber.subscriber_id)
+    subscriber = Reference(subscriber_id, Subscriber.subscriber_id)
 
-    def __init__(self,mlist):
+    def __init__(self, mlist):
         self.mlist = mlist
         self.database = getConn(mlist)
 
@@ -604,8 +609,8 @@
              syslog('info', 'DlistUtils(canonicalize_sender)Executing query:\n%s', command)
 
         for alias in aliases:
-            #if DEBUG_MODE:
-                #syslog('info', 'Checking if <%s> is an alias', alias)
+            if DEBUG_MODE:
+                syslog('info', 'Checking if <%s> is an alias', alias)
             result = self.store.find((Subscriber,Alias),And(Alias.pseudonym == unicode(alias,'utf-8'),Subscriber.subscriber_id == Alias.subscriber_id))
             lists = [(subscriber.mailman_key.encode('utf-8')) for (subscriber,alias) in result]
 
@@ -654,12 +659,12 @@
     __storm_table__ = "override"
     __storm_primary__ = "subscriber_id", "thread_id"
     subscriber_id = Int()
-    subscriber = Reference(subscriber_id,Subscriber.subscriber_id)
+    subscriber = Reference(subscriber_id, Subscriber.subscriber_id)
     thread_id = Int()
-    thread = Reference(thread_id,Thread.thread_id)
+    thread = Reference(thread_id, Thread.thread_id)
     preference = Int()
 
-    def __init__(self,mlist):
+    def __init__(self, mlist):
         self.mlist = mlist
         self.database = getConn(mlist)
 
@@ -709,10 +714,9 @@
 
         return 1
 
-    def overrideURL(self,list_name, host_name, thread_reference, preference):
+    def overrideURL(self, list_name, host_name, thread_reference, preference):
         return 'http://%s/mailman/options/%s?override=%s&preference=%d' % (host_name, list_name, thread_reference, preference)
 
-
 #Functions Independent of the database tables in DlistUtils
 
 def GetEmailAddresses(mlist, subscribers):
@@ -752,13 +756,22 @@
 
 def _create_database(name):
     """Create a database. This requires us to not be in a transaction."""
-    conn = pgdb.connect(host='localhost', database='postgres',user='mailman', password='mailman')
+    conn = pgdb.connect(host='localhost', database='postgres', user='mailman', password='mailman')
     old_iso_lvl = conn.isolation_level
     conn.set_isolation_level(ISOLATION_LEVEL_AUTOCOMMIT)
     cursor = conn.cursor()
     cursor.execute('create database "%s"' % name) 
     conn.set_isolation_level(old_iso_lvl)
 
+def _remove_database(name):
+    """Remove a database. This requires us to not be in a transaction."""
+    conn = pgdb.connect(host='localhost', database='postgres', user='mailman', password='mailman')
+    old_iso_lvl = conn.isolation_level
+    conn.set_isolation_level(ISOLATION_LEVEL_AUTOCOMMIT)
+    cursor = conn.cursor()
+    cursor.execute('DROP DATABASE "%s"' % name) 
+    conn.set_isolation_level(old_iso_lvl)
+
 def executeSQL(mlist, commands):
     """Execute a sequence of SQL commands that create tables in the database."""
     database = getConn(mlist)
@@ -834,3 +847,9 @@
     mlist.Save()
     if alreadyLocked == 0:
         mlist.Unlock()
+
+def remove_dlist(listname):
+    """ Deletes the corresponding postgres database and tables."""
+    _remove_database(listname)
+    if DEBUG_MODE:
+         syslog('info', "Database %s removed\n", listname)

=== modified file 'bin/rmlist'
--- bin/rmlist	2009-07-31 10:49:01 +0000
+++ bin/rmlist	2012-06-26 17:39:24 +0000
@@ -152,12 +152,7 @@
 
     # Remove the database in PostgreSQL if it was a dynamic list. Added by Systers.
     try:
-        conn = DlistUtils.getConn(None)
-        old_iso_lvl = conn.isolation_level
-        conn.set_isolation_level(ISOLATION_LEVEL_AUTOCOMMIT)
-        cursor = conn.cursor()
-        cursor.execute('DROP DATABASE "%s"' % listname)
-        conn.set_isolation_level(old_iso_lvl)
+        DlistUtils.remove_dlist(listname)
     except:
         syslog('error', 'Tried to remove the database for %s, no such database (ok if it was a non dlist). If it was a dlist, the postgreSQL database for this list needs to be removed manually.' % listname)