[Merge] lp:~varun/postorius/postorius_gsoc2013_modified into lp:postorius

Varun Sharma has proposed merging lp:~varun/postorius/postorius_gsoc2013_modified into lp:postorius. Requested reviews: Florian Fuchs (flo-fuchs) For more details, see: https://code.launchpad.net/~varun/postorius/postorius_gsoc2013_modified/+mer... Added deletion of user functionality for superusers. Added view user_delete to views/user.py and template user_confirm_delete.html to templates/users. As per the suggestions from florian and steve, i have improved few things from my last merge request like handling of MailmanApiError and showing email id in notification rather than display name which was not a mandatory field. Plus removed extra return redirect statements from try/except blocks. -- https://code.launchpad.net/~varun/postorius/postorius_gsoc2013_modified/+mer... Your team Mailman Coders is subscribed to branch lp:postorius.

Review: Needs Fixing Hi Varun, thank you for doing another merge request! I have a couple of small issues, all concerning the ``user_delete`` view (some of them were already mentionend in Stephen Turnbull's comment on your last merge proposal): 1) ``except MailmanAPIError`` covers ``MailmanUser.objects.get`` (like it should), but doesn't cover ``user.delete()``. But it's possible (even if unlikely) that the REST service is running when you retrieve the user object, but isn't any more once you attempt to delete the user. Is the extra nesting necessary? 2) I think you can get rid of one of the two user_id assignments. Actually, I think you can get rid of both: We know from the url definition that this view function should only be called with a user_id argument. So it could be made a mandatory argument, instead of assigning it from **kwargs. If (for some reason) the function will be called *without* the user_id argument, the resulting TypeError is exactly what we want. 3) In an effort to make the try block as simple as possible, the ``messages.success`` call could probably be moved outside. 4) There are some minor PEP8 violations (argument list: no whitespace after ','; email_id assignment: no whitespace around '='). Cheers Florian -- https://code.launchpad.net/~varun/postorius/postorius_gsoc2013_modified/+mer... Your team Mailman Coders is subscribed to branch lp:postorius.
participants (2)
-
Florian Fuchs
-
Varun Sharma