diff --git a/fas/fas/__init__.py b/fas/fas/__init__.py index e69de29..2146c0d 100644 --- a/fas/fas/__init__.py +++ b/fas/fas/__init__.py @@ -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 diff --git a/fas/fas/auth.py b/fas/fas/auth.py index 809a26c..faab6d2 100644 --- a/fas/fas/auth.py +++ b/fas/fas/auth.py @@ -65,14 +65,7 @@ def isApproved(person, group): ''' Returns True if the user is an approved member of a group ''' - try: - 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': + if group in person.approved_memberships: return True else: return False @@ -208,18 +201,16 @@ def canUpgradeUser(person, group, target): ''' Returns True if the user can upgrade target in the group ''' - if isApproved(person, group): - # Group admins can upgrade anybody. - # The controller should handle the case where the target - # is already a group admin. - if canAdminGroup(person, group): - return True - # Sponsors can only upgrade non-sponsors (i.e. normal users) - elif canSponsorGroup(person, group) and \ - not canSponsorGroup(target, group): - return True - else: - return False + # Group admins can upgrade anybody. + # The controller should handle the case where the target + # is already a group admin. + if canAdminGroup(person, group): + return True + # 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 \ + not canSponsorGroup(target, group): + return True else: return False diff --git a/fas/fas/group.py b/fas/fas/group.py index 780d6b6..5029664 100644 --- a/fas/fas/group.py +++ b/fas/fas/group.py @@ -4,8 +4,8 @@ from turbogears.database import session import cherrypy +import fas from fas.auth import * - from fas.user import KnownUser import re @@ -279,12 +279,12 @@ class Group(controllers.Controller): else: try: target.apply(group, person) - except: # TODO: More specific exception here. - turbogears.flash(_('%(user)s has already applied to %(group)s!') % \ - {'user': target.username, 'group': group.name}) + except fas.ApplyError, e: + turbogears.flash(_('%(user)s could not apply to %(group)s: %(error)s') % \ + {'user': target.username, 'group': group.name, 'error': e}) + turbogears.redirect('/group/view/%s' % group.name) else: import turbomail - # 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: Maybe that @fedoraproject.org (and even -sponsors) should be configurable somewhere? @@ -322,8 +322,9 @@ Please go to %(url)s to take action. else: try: target.sponsor(group, person) - except: - turbogears.flash(_("'%s' could not be sponsored!") % target.username) + except fas.SponsorError, e: + 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) else: import turbomail @@ -359,9 +360,9 @@ propagate into the e-mail aliases and CVS repository within an hour. else: try: target.remove(group, target) - except KeyError: - turbogears.flash(_('%(name)s could not be removed from %(group)s!') % \ - {'name': target.username, 'group': group.name}) + except fas.RemoveError, e: + turbogears.flash(_("%(user)s could not be removed from %(group)s: %(error)s") % \ + {'user': target.username, 'group': group.name, 'error': e}) turbogears.redirect('/group/view/%s' % group.name) else: import turbomail @@ -373,7 +374,7 @@ immediately for new operations, and should propagate into the e-mail aliases within an hour. ''') % {'group': group.name, 'name': person.human_name, 'email': person.emails['primary'].email} 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}) turbogears.redirect('/group/view/%s' % group.name) return dict() @@ -396,11 +397,9 @@ aliases within an hour. else: try: target.upgrade(group, person) - except TypeError, e: - turbogears.flash(e) - turbogears.redirect('/group/view/%s' % group.name) - except: - turbogears.flash(_('%(name)s could not be upgraded!') % {'name' : target.username}) + except fas.UpgradeError, e: + turbogears.flash(_('%(name)s could not be upgraded in %(group)s: %(error)s') % \ + {'name': target.username, 'group': group.name, 'error': e}) turbogears.redirect('/group/view/%s' % group.name) else: import turbomail @@ -437,8 +436,9 @@ into the e-mail aliases within an hour. else: try: target.downgrade(group, person) - except: - turbogears.flash(_('%(username)s could not be downgraded!') % {'username': target.username}) + except fas.DowngradeError, e: + 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) else: import turbomail diff --git a/fas/fas/model.py b/fas/fas/model.py index a88a07a..97bb28b 100644 --- a/fas/fas/model.py +++ b/fas/fas/model.py @@ -42,7 +42,10 @@ from turbogears.database import session from turbogears import identity +import turbogears + from fedora.tg.json import SABase +import fas # Bind us to the database defined in the config file. get_engine() @@ -118,18 +121,21 @@ class People(SABase): ''' Apply a person to a group ''' - role = PersonRoles() - role.role_status = 'unapproved' - role.role_type = 'user' - role.member = cls - role.group = group + if group in cls.memberships: + raise fas.ApplyError, _('user is already in this group') + else: + role = PersonRoles() + role.role_status = 'unapproved' + role.role_type = 'user' + role.member = cls + role.group = group def approve(cls, group, requester): ''' Approve a person in a group - requester for logging purposes ''' - if group in cls.approved_memberships: - raise '%s is already approved in %s' % (cls.username, group.name) + if group not in cls.unapproved_memberships: + raise fas.ApproveError, _('user is not an unapproved member') else: role = PersonRoles.query.filter_by(member=cls, group=group).one() role.role_status = 'approved' @@ -139,11 +145,11 @@ class People(SABase): Upgrade a user in a group - requester for logging purposes ''' 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: role = PersonRoles.query.filter_by(member=cls, group=group).one() 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': role.role_type = 'administrator' elif role.role_type == 'user': @@ -154,11 +160,11 @@ class People(SABase): Downgrade a user in a group - requester for logging purposes ''' 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: role = PersonRoles.query.filter_by(member=cls, group=group).one() 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': role.role_type = 'user' elif role.role_type == 'administrator': @@ -166,20 +172,19 @@ class People(SABase): def sponsor(cls, group, requester): # If we want to do logging, this might be the place. - if not group in cls.memberships: - raise '%s not a member of %s' % (group.name, cls.memberships) + if not group in cls.unapproved_memberships: + raise fas.SponsorError, _('user is not an unapproved member') role = PersonRoles.query.filter_by(member=cls, group=group).one() role.role_status = 'approved' role.sponsor_id = requester.id role.approval = datetime.now(pytz.utc) def remove(cls, group, requester): - role = PersonRoles.query.filter_by(member=cls, group=group).one() - try: + if not group in cls.memberships: + raise fas.RemoveError, _('user is not a member') + else: + role = PersonRoles.query.filter_by(member=cls, group=group).one() session.delete(role) - except TypeError: - pass - # Handle somehow. def __repr__(cls): return "User(%s,%s)" % (cls.username, cls.human_name)