[copr] master: Start moving authorization from views to logic (8fda311)

bkabrda at fedoraproject.org bkabrda at fedoraproject.org
Thu Mar 28 12:11:20 UTC 2013


Repository : http://git.fedorahosted.org/cgit/copr.git

On branch  : master

>---------------------------------------------------------------

commit 8fda311b272d73e41ecccb537ceee039ecf2ec67
Author: Bohuslav Kabrda <bkabrda at redhat.com>
Date:   Thu Mar 28 12:09:38 2013 +0100

    Start moving authorization from views to logic
    
    - move copr updating logic
    - improve the ActionInProgress exception so that we can print it as any other exception, while still making the Action accessible if needed


>---------------------------------------------------------------

 coprs_frontend/coprs/exceptions.py                 |    9 +++++++++
 coprs_frontend/coprs/logic/builds_logic.py         |    3 ++-
 coprs_frontend/coprs/logic/coprs_logic.py          |   20 ++++++++++++++++----
 .../coprs/views/coprs_ns/coprs_builds.py           |    2 +-
 .../coprs/views/coprs_ns/coprs_general.py          |   10 +++-------
 5 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/coprs_frontend/coprs/exceptions.py b/coprs_frontend/coprs/exceptions.py
index da744b2..f1954b2 100644
--- a/coprs_frontend/coprs/exceptions.py
+++ b/coprs_frontend/coprs/exceptions.py
@@ -17,3 +17,12 @@ class ActionInProgressException(BaseException):
     def __init__(self, msg, action):
         self.msg = msg
         self.action = action
+
+    def __unicode__(self):
+        return self.formatted_msg()
+
+    def __str__(self):
+        return self.__unicode__()
+
+    def formatted_msg(self):
+        return self.msg.format(action=self.action)
diff --git a/coprs_frontend/coprs/logic/builds_logic.py b/coprs_frontend/coprs/logic/builds_logic.py
index 247c01a..2a91f95 100644
--- a/coprs_frontend/coprs/logic/builds_logic.py
+++ b/coprs_frontend/coprs/logic/builds_logic.py
@@ -57,7 +57,8 @@ class BuildsLogic(object):
 
     @classmethod
     def add(cls, user, pkgs, copr):
-        coprs_logic.CoprsLogic.raise_if_unfinished_action(user, copr)
+        coprs_logic.CoprsLogic.raise_if_unfinished_action(user, copr,
+                                                          'Can\'t build while there is an operation in progress: {action}')
         build = models.Build(
             pkgs=pkgs,
             copr=copr,
diff --git a/coprs_frontend/coprs/logic/coprs_logic.py b/coprs_frontend/coprs/logic/coprs_logic.py
index 1a83147..2b4b2d9 100644
--- a/coprs_frontend/coprs/logic/coprs_logic.py
+++ b/coprs_frontend/coprs/logic/coprs_logic.py
@@ -92,7 +92,9 @@ class CoprsLogic(object):
 
     @classmethod
     def update(cls, user, copr, check_for_duplicates = True):
-        cls.raise_if_unfinished_action(user, copr)
+        cls.raise_if_unfinished_action(user, copr,
+                                       'Can\'t change this Copr name, another operation is in progress: {action}')
+        cls.raise_if_cant_update(user, copr, 'Only owners and admins may update their Coprs.')
 
         existing = cls.exists_for_user(copr.owner, copr.name).first()
         if existing:
@@ -115,7 +117,8 @@ class CoprsLogic(object):
     def delete(cls, user, copr, check_for_duplicates=True):
         # for the time being, we authorize user to do this in view...
         # TODO: do we want to dump the information somewhere, so that we can search it in future?
-        cls.raise_if_unfinished_action(user, copr)
+        cls.raise_if_unfinished_action(user, copr,
+                                       'Can\'t delete this Copr, another operation is in progress: {action}')
         action = models.Action(action_type=helpers.ActionTypeEnum('delete'),
                                object_type='copr',
                                object_id=copr.id,
@@ -149,13 +152,22 @@ class CoprsLogic(object):
         return actions
 
     @classmethod
-    def raise_if_unfinished_action(cls, user, copr):
+    def raise_if_unfinished_action(cls, user, copr, message):
         """This method raises ActionInProgressException if given copr has an unfinished
         action. Returns None otherwise.
         """
         unfinished_actions = cls.unfinished_actions_for(user, copr).all()
         if unfinished_actions:
-            raise exceptions.ActionInProgressException('Action in progress on this copr.', unfinished_actions[0])
+            raise exceptions.ActionInProgressException(message, unfinished_actions[0])
+
+    @classmethod
+    def raise_if_cant_update(cls, user, copr, message):
+        """This method raises InsufficientRightsException if given user cant update
+        given copr. Returns None otherwise.
+        """
+        # TODO: this is a bit inconsistent - shouldn't the user method be called can_update?
+        if not user.can_edit(copr):
+            raise exceptions.InsufficientRightsException(message)
 
 
 class CoprPermissionsLogic(object):
diff --git a/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py b/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py
index 1f91e49..a6871cd 100644
--- a/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py
+++ b/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py
@@ -59,7 +59,7 @@ def copr_new_build(username, coprname):
                 build.memory_reqs = form.memory_reqs.data
                 build.timeout = form.timeout.data
         except exceptions.ActionInProgressException as e:
-            flask.flash('Can\'t build in this Copr, there is an operation in progress: {0}'.format(e.action))
+            flask.flash(e)
             db.session.rollback()
         else:
             flask.flash("Build was added")
diff --git a/coprs_frontend/coprs/views/coprs_ns/coprs_general.py b/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
index 6da142e..adba1d0 100644
--- a/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
+++ b/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
@@ -164,10 +164,6 @@ def copr_edit(username, coprname, form=None):
 def copr_update(username, coprname):
     form = forms.CoprFormFactory.create_form_cls()()
     copr = coprs_logic.CoprsLogic.get(flask.g.user, username, coprname).first()
-    # only owner can update a copr
-    if not flask.g.user.can_edit(copr):
-        flask.flash('Only owners and admins may update their Coprs.')
-        return flask.redirect(flask.url_for('coprs_ns.copr_detail', username = copr.owner.name, coprname = form.name.data))
 
     if form.validate_on_submit():
         # we don't change owner (yet)
@@ -179,8 +175,8 @@ def copr_update(username, coprname):
 
         try:
             coprs_logic.CoprsLogic.update(flask.g.user, copr, check_for_duplicates = False) # form validation checks for duplicates
-        except exceptions.ActionInProgressException as e:
-            flask.flash('Can\'t change this Copr name, there is another operation in progress: {0}'.format(e.action))
+        except (exceptions.ActionInProgressException, exceptions.InsufficientRightsException) as e:
+            flask.flash(e)
             db.session.rollback()
         else:
             flask.flash('Copr was updated successfully.')
@@ -252,7 +248,7 @@ def copr_delete(username, coprname):
             coprs_logic.CoprsLogic.delete(flask.g.user, copr)
         except exceptions.ActionInProgressException as e:
             db.session.rollback()
-            flask.flash('Can\'t manipulate this Copr, there is another operation in progress: {0}'.format(e.action))
+            flask.flash(e)
             return flask.redirect(flask.url_for('coprs_ns.copr_detail', username=username, coprname=coprname))
         else:
             db.session.commit()



More information about the copr-devel mailing list