Code review comment for lp:~acoconut/systers/flexible_essays

Revision history for this message
Robin J (robin-jeffries) wrote :

Ahh, now I understand why we can never make sense of the order the questions come in. Could you change it so they are ordered by subscribe time? (maybe we have to add that field? Or does the database know when the item is entered?) I have always considered the ordering to be random, and when you have "saved" some items to process later, they get reordered and you can't tell the old ones from the new ones.

Also change the comment about modifying this to add essays, since it's not an essay any more. Something like "the answers to the questions asked on the subscribe page?

I think you need to rethink the error checking in subscribe.py First, it's going to confuse the next person who reads your code when you say if len(answer)<1500, since that only applies to text boxes - a checkbox can't be that big. So a branch that has different logic for textboxes than checkboxes will make it clearer. Also, this error message

 results.append(_('At least one of your answers is too long. Your answers must be less than 1500 characters long.'))

is not very use friendly. Tell them which question is too long. You already have info about the questions -- use the long text. (Suppose there are 2 checkboxes and one is under 1500 chars, but not by a lot and the other is over. That will drive the subscriber crazy trying to figure out which ones needs to change)

For results = _("""There is a pending subscription with this email.""")

how about adding "You may not subscribe a second time. The moderator will contact you if there are any questions about the answers you gave originally."

Can you look up and see if there is a way to set a given change (e.g., one that has been approved) as the baseline, so that I can look at only new changes. It's hard to review this stuff when the diffs don't give me much information about what you changed since I last looked at it.

« Back to merge proposal