This might be completely unpythonic (and I'd love to learn to fix it, if it is) - some attempt at handling errors with more specific messages. Will want to do the same to more of auth.py later on.

This commit is contained in:
Ricky Zhou (周家杰) 2008-03-05 01:33:28 -05:00
parent cf4a93c389
commit a57cfc86b2
4 changed files with 79 additions and 56 deletions

View file

@ -0,0 +1,27 @@
class FASError(Exception):
'''FAS Error'''
pass
class ApplyError(FASError):
'''Raised when a user could not apply to a group'''
pass
class ApproveError(FASError):
'''Raised when a user could not be approved in a group'''
pass
class SponsorError(FASError):
'''Raised when a user could not be sponsored in a group'''
pass
class UpgradeError(FASError):
'''Raised when a user could not be upgraded in a group'''
pass
class DowngradeError(FASError):
'''Raised when a user could not be downgraded in a group'''
pass
class RemoveError(FASError):
'''Raised when a user could not be removed from a group'''
pass

View file

@ -65,14 +65,7 @@ def isApproved(person, group):
''' '''
Returns True if the user is an approved member of a group Returns True if the user is an approved member of a group
''' '''
try: if group in person.approved_memberships:
role = PersonRoles.query.filter_by(group=group, member=person).one()
except IndexError:
''' Not in the group '''
return False
except InvalidRequestError:
return False
if role.role_status == 'approved':
return True return True
else: else:
return False return False
@ -208,18 +201,16 @@ def canUpgradeUser(person, group, target):
''' '''
Returns True if the user can upgrade target in the group Returns True if the user can upgrade target in the group
''' '''
if isApproved(person, group): # Group admins can upgrade anybody.
# Group admins can upgrade anybody. # The controller should handle the case where the target
# The controller should handle the case where the target # is already a group admin.
# is already a group admin. if canAdminGroup(person, group):
if canAdminGroup(person, group): return True
return True # Sponsors can only upgrade non-sponsors (i.e. normal users)
# Sponsors can only upgrade non-sponsors (i.e. normal users) # TODO: Don't assume that canSponsorGroup means that the user is a sponsor
elif canSponsorGroup(person, group) and \ elif canSponsorGroup(person, group) and \
not canSponsorGroup(target, group): not canSponsorGroup(target, group):
return True return True
else:
return False
else: else:
return False return False

View file

@ -4,8 +4,8 @@ from turbogears.database import session
import cherrypy import cherrypy
import fas
from fas.auth import * from fas.auth import *
from fas.user import KnownUser from fas.user import KnownUser
import re import re
@ -279,12 +279,12 @@ class Group(controllers.Controller):
else: else:
try: try:
target.apply(group, person) target.apply(group, person)
except: # TODO: More specific exception here. except fas.ApplyError, e:
turbogears.flash(_('%(user)s has already applied to %(group)s!') % \ turbogears.flash(_('%(user)s could not apply to %(group)s: %(error)s') % \
{'user': target.username, 'group': group.name}) {'user': target.username, 'group': group.name, 'error': e})
turbogears.redirect('/group/view/%s' % group.name)
else: else:
import turbomail import turbomail
# TODO: How do we handle gettext calls for these kinds of emails? # TODO: How do we handle gettext calls for these kinds of emails?
# TODO: CC to right place, put a bit more thought into how to most elegantly do this # TODO: CC to right place, put a bit more thought into how to most elegantly do this
# TODO: Maybe that @fedoraproject.org (and even -sponsors) should be configurable somewhere? # TODO: Maybe that @fedoraproject.org (and even -sponsors) should be configurable somewhere?
@ -322,8 +322,9 @@ Please go to %(url)s to take action.
else: else:
try: try:
target.sponsor(group, person) target.sponsor(group, person)
except: except fas.SponsorError, e:
turbogears.flash(_("'%s' could not be sponsored!") % target.username) turbogears.flash(_("%(user)s could not be sponsored in %(group)s: %(error)s") % \
{'user': target.username, 'group': group.name, 'error': e})
turbogears.redirect('/group/view/%s' % group.name) turbogears.redirect('/group/view/%s' % group.name)
else: else:
import turbomail import turbomail
@ -359,9 +360,9 @@ propagate into the e-mail aliases and CVS repository within an hour.
else: else:
try: try:
target.remove(group, target) target.remove(group, target)
except KeyError: except fas.RemoveError, e:
turbogears.flash(_('%(name)s could not be removed from %(group)s!') % \ turbogears.flash(_("%(user)s could not be removed from %(group)s: %(error)s") % \
{'name': target.username, 'group': group.name}) {'user': target.username, 'group': group.name, 'error': e})
turbogears.redirect('/group/view/%s' % group.name) turbogears.redirect('/group/view/%s' % group.name)
else: else:
import turbomail import turbomail
@ -373,7 +374,7 @@ immediately for new operations, and should propagate into the e-mail
aliases within an hour. aliases within an hour.
''') % {'group': group.name, 'name': person.human_name, 'email': person.emails['primary'].email} ''') % {'group': group.name, 'name': person.human_name, 'email': person.emails['primary'].email}
turbomail.enqueue(message) turbomail.enqueue(message)
turbogears.flash(_('%(name)s has been removed from %(group)s!') % \ turbogears.flash(_('%(name)s has been removed from %(group)s') % \
{'name': target.username, 'group': group.name}) {'name': target.username, 'group': group.name})
turbogears.redirect('/group/view/%s' % group.name) turbogears.redirect('/group/view/%s' % group.name)
return dict() return dict()
@ -396,11 +397,9 @@ aliases within an hour.
else: else:
try: try:
target.upgrade(group, person) target.upgrade(group, person)
except TypeError, e: except fas.UpgradeError, e:
turbogears.flash(e) turbogears.flash(_('%(name)s could not be upgraded in %(group)s: %(error)s') % \
turbogears.redirect('/group/view/%s' % group.name) {'name': target.username, 'group': group.name, 'error': e})
except:
turbogears.flash(_('%(name)s could not be upgraded!') % {'name' : target.username})
turbogears.redirect('/group/view/%s' % group.name) turbogears.redirect('/group/view/%s' % group.name)
else: else:
import turbomail import turbomail
@ -437,8 +436,9 @@ into the e-mail aliases within an hour.
else: else:
try: try:
target.downgrade(group, person) target.downgrade(group, person)
except: except fas.DowngradeError, e:
turbogears.flash(_('%(username)s could not be downgraded!') % {'username': target.username}) turbogears.flash(_('%(name)s could not be downgraded in %(group)s: %(error)s') % \
{'name': target.username, 'group': group.name, 'error': e})
turbogears.redirect('/group/view/%s' % group.name) turbogears.redirect('/group/view/%s' % group.name)
else: else:
import turbomail import turbomail

View file

@ -42,7 +42,10 @@ from turbogears.database import session
from turbogears import identity from turbogears import identity
import turbogears
from fedora.tg.json import SABase from fedora.tg.json import SABase
import fas
# Bind us to the database defined in the config file. # Bind us to the database defined in the config file.
get_engine() get_engine()
@ -118,18 +121,21 @@ class People(SABase):
''' '''
Apply a person to a group Apply a person to a group
''' '''
role = PersonRoles() if group in cls.memberships:
role.role_status = 'unapproved' raise fas.ApplyError, _('user is already in this group')
role.role_type = 'user' else:
role.member = cls role = PersonRoles()
role.group = group role.role_status = 'unapproved'
role.role_type = 'user'
role.member = cls
role.group = group
def approve(cls, group, requester): def approve(cls, group, requester):
''' '''
Approve a person in a group - requester for logging purposes Approve a person in a group - requester for logging purposes
''' '''
if group in cls.approved_memberships: if group not in cls.unapproved_memberships:
raise '%s is already approved in %s' % (cls.username, group.name) raise fas.ApproveError, _('user is not an unapproved member')
else: else:
role = PersonRoles.query.filter_by(member=cls, group=group).one() role = PersonRoles.query.filter_by(member=cls, group=group).one()
role.role_status = 'approved' role.role_status = 'approved'
@ -139,11 +145,11 @@ class People(SABase):
Upgrade a user in a group - requester for logging purposes Upgrade a user in a group - requester for logging purposes
''' '''
if not group in cls.memberships: if not group in cls.memberships:
raise '%s not a member of %s' % (group.name, cls.memberships) raise fas.UpgradeError, _('user is not a member')
else: else:
role = PersonRoles.query.filter_by(member=cls, group=group).one() role = PersonRoles.query.filter_by(member=cls, group=group).one()
if role.role_type == 'administrator': if role.role_type == 'administrator':
raise '%s is already an admin in %s' % (cls.username, group.name) raise fas.UpgradeError, _('administrators cannot be upgraded any further')
elif role.role_type == 'sponsor': elif role.role_type == 'sponsor':
role.role_type = 'administrator' role.role_type = 'administrator'
elif role.role_type == 'user': elif role.role_type == 'user':
@ -154,11 +160,11 @@ class People(SABase):
Downgrade a user in a group - requester for logging purposes Downgrade a user in a group - requester for logging purposes
''' '''
if not group in cls.memberships: if not group in cls.memberships:
raise '%s not a member of %s' % (group.name, cls.memberships) raise fas.DowngradeError, _('user is not a member')
else: else:
role = PersonRoles.query.filter_by(member=cls, group=group).one() role = PersonRoles.query.filter_by(member=cls, group=group).one()
if role.role_type == 'user': if role.role_type == 'user':
raise '%s is already a user in %s, did you mean to remove?' % (cls.username, group.name) raise fas.DowngradeError, _('users cannot be downgraded any further')
elif role.role_type == 'sponsor': elif role.role_type == 'sponsor':
role.role_type = 'user' role.role_type = 'user'
elif role.role_type == 'administrator': elif role.role_type == 'administrator':
@ -166,20 +172,19 @@ class People(SABase):
def sponsor(cls, group, requester): def sponsor(cls, group, requester):
# If we want to do logging, this might be the place. # If we want to do logging, this might be the place.
if not group in cls.memberships: if not group in cls.unapproved_memberships:
raise '%s not a member of %s' % (group.name, cls.memberships) raise fas.SponsorError, _('user is not an unapproved member')
role = PersonRoles.query.filter_by(member=cls, group=group).one() role = PersonRoles.query.filter_by(member=cls, group=group).one()
role.role_status = 'approved' role.role_status = 'approved'
role.sponsor_id = requester.id role.sponsor_id = requester.id
role.approval = datetime.now(pytz.utc) role.approval = datetime.now(pytz.utc)
def remove(cls, group, requester): def remove(cls, group, requester):
role = PersonRoles.query.filter_by(member=cls, group=group).one() if not group in cls.memberships:
try: raise fas.RemoveError, _('user is not a member')
else:
role = PersonRoles.query.filter_by(member=cls, group=group).one()
session.delete(role) session.delete(role)
except TypeError:
pass
# Handle somehow.
def __repr__(cls): def __repr__(cls):
return "User(%s,%s)" % (cls.username, cls.human_name) return "User(%s,%s)" % (cls.username, cls.human_name)