From cbadc19baaa77db583959f42df98cc1e39aef795 Mon Sep 17 00:00:00 2001 From: Matt Prahl Date: Apr 11 2017 13:41:42 +0000 Subject: Move `get_project_users` and `get_project_groups` to the Project model and move logic from ui.repo.view_settings to model.Project --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 499b99b..f1b46c3 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -926,8 +926,7 @@ def add_user_to_project(session, project, new_user, user, access='admin'): users = set([ user_.user - for user_ in get_project_users( - session, project, access, combine=False) + for user_ in project.get_project_users(access, combine=False) ]) users.add(project.user.user) @@ -1017,8 +1016,7 @@ def add_group_to_project( groups = set([ group.group_name - for group in get_project_groups( - session, project, access, combine=False) + for group in project.get_project_groups(access, combine=False) ]) if new_group in groups: @@ -4116,102 +4114,6 @@ def get_access_levels(session): return [access_level.access for access_level in access_level_objs] -def get_project_users(session, project_obj, access, combine=True): - ''' Returns the list of users/groups of the project according - to the given access. - - :arg session: the session to use to connect to the database. - :arg project_obj: SQLAlchemy object of Project class. - :arg access: the access level to query for, can be: 'admin', - 'commit' or 'ticket'. - :type access: string - :arg combine: The access levels have some hierarchy - - like: all the users having commit access also has - ticket access and the admins have all the access - that commit and ticket access users have. If combine - is set to False, this function will only return those - users which have the given access and no other access. - ex: if access is 'ticket' and combine is True, it will - return all the users with ticket access which includes - all the committers and admins. If combine were False, - it would have returned only the users with ticket access - and would not have included committers and admins. - :type combine: boolean - ''' - - if access not in ['admin', 'commit', 'ticket']: - raise pagure.exceptions.AccessLevelNotFound( - 'The access level does not exist') - - if combine: - if access == 'admin': - return project_obj.admins - elif access == 'commit': - return project_obj.committers - elif access == 'ticket': - return project_obj.users - else: - if access == 'admin': - return project_obj.admins - elif access == 'commit': - committers = set(project_obj.committers) - admins = set(project_obj.admins) - return list(committers - admins) - elif access == 'ticket': - committers = set(project_obj.committers) - admins = set(project_obj.admins) - users = set(project_obj.users) - return list(users - committers - admins) - - -def get_project_groups(session, project_obj, access, combine=True): - ''' Returns the list of groups of the project according - to the given access. - - :arg session: the session to use to connect to the database. - :arg project_obj: SQLAlchemy object of Project class. - :arg access: the access level to query for, can be: 'admin', - 'commit' or 'ticket'. - :type access: string - :arg combine: The access levels have some hierarchy - - like: all the groups having commit access also has - ticket access and the admin_groups have all the access - that committer_groups and ticket access groups have. - If combine is set to False, this function will only return - those groups which have the given access and no other access. - ex: if access is 'ticket' and combine is True, it will - return all the groups with ticket access which includes - all the committer_groups and admin_groups. If combine were False, - it would have returned only the groups with ticket access - and would not have included committer_groups and admin_groups. - :type combine: boolean - ''' - - if access not in ['admin', 'commit', 'ticket']: - raise pagure.exceptions.AccessLevelNotFound( - 'The access level does not exist') - - if combine: - if access == 'admin': - return project_obj.admin_groups - elif access == 'commit': - return project_obj.committer_groups - elif access == 'ticket': - return project_obj.groups - else: - if access == 'admin': - return project_obj.admin_groups - elif access == 'commit': - committers = set(project_obj.committer_groups) - admins = set(project_obj.admin_groups) - return list(committers - admins) - elif access == 'ticket': - committers = set(project_obj.committer_groups) - admins = set(project_obj.admin_groups) - groups = set(project_obj.groups) - return list(groups - committers - admins) - - def get_obj_access(session, project_obj, obj): ''' Returns the level of access the user/group has on the project. diff --git a/pagure/lib/model.py b/pagure/lib/model.py index b6fbedb..4554bb5 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -29,6 +29,8 @@ from sqlalchemy.orm import scoped_session from sqlalchemy.orm import relation from sqlalchemy.orm import validates +import pagure.exceptions + CONVENTION = { "ix": 'ix_%(table_name)s_%(column_0_label)s', @@ -671,6 +673,116 @@ class Project(BASE): return contributors + def get_project_users(self, access, combine=True): + ''' Returns the list of users/groups of the project according + to the given access. + + :arg access: the access level to query for, can be: 'admin', + 'commit' or 'ticket'. + :type access: string + :arg combine: The access levels have some hierarchy - + like: all the users having commit access also has + ticket access and the admins have all the access + that commit and ticket access users have. If combine + is set to False, this function will only return those + users which have the given access and no other access. + ex: if access is 'ticket' and combine is True, it will + return all the users with ticket access which includes + all the committers and admins. If combine were False, + it would have returned only the users with ticket access + and would not have included committers and admins. + :type combine: boolean + ''' + + if access not in ['admin', 'commit', 'ticket']: + raise pagure.exceptions.AccessLevelNotFound( + 'The access level does not exist') + + if combine: + if access == 'admin': + return self.admins + elif access == 'commit': + return self.committers + elif access == 'ticket': + return self.users + else: + if access == 'admin': + return self.admins + elif access == 'commit': + committers = set(self.committers) + admins = set(self.admins) + return list(committers - admins) + elif access == 'ticket': + committers = set(self.committers) + admins = set(self.admins) + users = set(self.users) + return list(users - committers - admins) + + def get_project_groups(self, access, combine=True): + ''' Returns the list of groups of the project according + to the given access. + + :arg access: the access level to query for, can be: 'admin', + 'commit' or 'ticket'. + :type access: string + :arg combine: The access levels have some hierarchy - + like: all the groups having commit access also has + ticket access and the admin_groups have all the access + that committer_groups and ticket access groups have. + If combine is set to False, this function will only return + those groups which have the given access and no other access. + ex: if access is 'ticket' and combine is True, it will + return all the groups with ticket access which includes + all the committer_groups and admin_groups. If combine were False, + it would have returned only the groups with ticket access + and would not have included committer_groups and admin_groups. + :type combine: boolean + ''' + + if access not in ['admin', 'commit', 'ticket']: + raise pagure.exceptions.AccessLevelNotFound( + 'The access level does not exist') + + if combine: + if access == 'admin': + return self.admin_groups + elif access == 'commit': + return self.committer_groups + elif access == 'ticket': + return self.groups + else: + if access == 'admin': + return self.admin_groups + elif access == 'commit': + committers = set(self.committer_groups) + admins = set(self.admin_groups) + return list(committers - admins) + elif access == 'ticket': + committers = set(self.committer_groups) + admins = set(self.admin_groups) + groups = set(self.groups) + return list(groups - committers - admins) + + @property + def access_users(self): + ''' Return a dictionary with all user access + ''' + return { + 'admin': self.get_project_users(access='admin', combine=False), + 'commit': self.get_project_users(access='commit', combine=False), + 'ticket': self.get_project_users(access='ticket', combine=False), + } + + @property + def access_groups(self): + ''' Return a dictionary with all group access + ''' + return { + 'admin': self.get_project_groups(access='admin', combine=False), + 'commit': self.get_project_groups(access='commit', combine=False), + 'ticket': self.get_project_groups(access='ticket', combine=False), + } + def to_json(self, public=False, api=False): ''' Return a representation of the project as JSON. ''' diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index 9b75f4c..afa0a17 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -1044,53 +1044,13 @@ def view_settings(repo, username=None, namespace=None): if flask.request.method == 'GET' and branchname: branches_form.branches.data = branchname - access_users = { - 'admin': pagure.lib.get_project_users( - SESSION, - project_obj=repo, - access='admin', - combine=False - ), - 'commit': pagure.lib.get_project_users( - SESSION, - project_obj=repo, - access='commit', - combine=False - ), - 'ticket': pagure.lib.get_project_users( - SESSION, - project_obj=repo, - access='ticket', - combine=False - ), - } - access_groups = { - 'admin': pagure.lib.get_project_groups( - SESSION, - project_obj=repo, - access='admin', - combine=False - ), - 'commit': pagure.lib.get_project_groups( - SESSION, - project_obj=repo, - access='commit', - combine=False - ), - 'ticket': pagure.lib.get_project_groups( - SESSION, - project_obj=repo, - access='ticket', - combine=False - ), - } return flask.render_template( 'settings.html', select='settings', username=username, repo=repo, - access_users=access_users, - access_groups=access_groups, + access_users=repo.access_users, + access_groups=repo.access_groups, form=form, tag_form=tag_form, branches_form=branches_form, diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py index 9c5a6b2..8f0f52d 100644 --- a/tests/test_pagure_lib.py +++ b/tests/test_pagure_lib.py @@ -3552,8 +3552,8 @@ class PagureLibtests(tests.Modeltests): ) def test_get_project_users(self): - ''' Test the get_project_users method in pagure.lib.__init__ - when combine is True ''' + ''' Test the get_project_users method when combine is True + ''' tests.create_projects(self.session) project = pagure.lib.get_project(self.session, name='test') @@ -3562,11 +3562,7 @@ class PagureLibtests(tests.Modeltests): # which means the an admin is a user, committer as well # and a committer is also a user # and a user is just a user - users = pagure.lib.get_project_users( - self.session, - project_obj=project, - access='admin', - ) + users = project.get_project_users(access='admin') # Only pingou is the admin as of now # But, he is the creator and @@ -3579,9 +3575,7 @@ class PagureLibtests(tests.Modeltests): # Wrong access level, should raise Accesslevelnotfound exception self.assertRaises( pagure.exceptions.AccessLevelNotFound, - pagure.lib.get_project_users, - self.session, - project_obj=project, + project.get_project_users, access='owner', ) @@ -3598,31 +3592,19 @@ class PagureLibtests(tests.Modeltests): self.assertEqual(msg, 'User added') project = pagure.lib.get_project(self.session, name='test') - users = pagure.lib.get_project_users( - self.session, - project_obj=project, - access='admin', - ) + users = project.get_project_users(access='admin') self.assertEqual(len(users), 1) self.assertEqual(users[0].username, 'foo') # foo should be a committer as well, since he is an admin - users = pagure.lib.get_project_users( - self.session, - project_obj=project, - access='commit', - ) + users = project.get_project_users(access='commit') self.assertEqual(len(users), 1) self.assertEqual(users[0].username, 'foo') # the admin also has ticket access - users = pagure.lib.get_project_users( - self.session, - project_obj=project, - access='ticket', - ) + users = project.get_project_users(access='ticket') self.assertEqual(len(users), 1) self.assertEqual(users[0].username, 'foo') @@ -3640,27 +3622,15 @@ class PagureLibtests(tests.Modeltests): project = pagure.lib.get_project(self.session, name='test') # No admin now, even though pingou the creator is there - users = pagure.lib.get_project_users( - self.session, - project_obj=project, - access='admin', - ) + users = project.get_project_users(access='admin') self.assertEqual(len(users), 0) - users = pagure.lib.get_project_users( - self.session, - project_obj=project, - access='commit', - ) + users = project.get_project_users(access='commit') # foo is the committer currently self.assertEqual(len(users), 1) self.assertEqual(users[0].username, 'foo') - users = pagure.lib.get_project_users( - self.session, - project_obj=project, - access='ticket', - ) + users = project.get_project_users(access='ticket') # foo also has ticket rights self.assertEqual(len(users), 1) @@ -3679,34 +3649,22 @@ class PagureLibtests(tests.Modeltests): project = pagure.lib.get_project(self.session, name='test') # No admin now, even though pingou the creator is there - users = pagure.lib.get_project_users( - self.session, - project_obj=project, - access='admin', - ) + users = project.get_project_users(access='admin') self.assertEqual(len(users), 0) - users = pagure.lib.get_project_users( - self.session, - project_obj=project, - access='commit', - ) + users = project.get_project_users(access='commit') # foo deosn't have commit rights now self.assertEqual(len(users), 0) - users = pagure.lib.get_project_users( - self.session, - project_obj=project, - access='ticket', - ) + users = project.get_project_users(access='ticket') # foo does have tickets right though self.assertEqual(len(users), 1) self.assertEqual(users[0].username, 'foo') def test_get_project_users_combine_false(self): - ''' Test the get_project_users method in pagure.lib.__init__ - when combine is False ''' + ''' Test the get_project_users method when combine is False + ''' tests.create_projects(self.session) project = pagure.lib.get_project(self.session, name='test') @@ -3724,32 +3682,17 @@ class PagureLibtests(tests.Modeltests): self.assertEqual(msg, 'User added') # only one admin - users = pagure.lib.get_project_users( - self.session, - project_obj=project, - access='admin', - combine=False, - ) + users = project.get_project_users(access='admin', combine=False) self.assertEqual(len(users), 1) self.assertEqual(users[0].username, 'foo') # No user with only commit access - users = pagure.lib.get_project_users( - self.session, - project_obj=project, - access='commit', - combine=False, - ) + users = project.get_project_users(access='commit', combine=False) self.assertEqual(len(users), 0) # No user with only ticket access - users = pagure.lib.get_project_users( - self.session, - project_obj=project, - access='ticket', - combine=False, - ) + users = project.get_project_users(access='ticket', combine=False) self.assertEqual(len(users), 0) # Update the access level of foo user to commit @@ -3765,31 +3708,16 @@ class PagureLibtests(tests.Modeltests): # He is just a committer project = pagure.lib.get_project(self.session, name='test') - users = pagure.lib.get_project_users( - self.session, - project_obj=project, - access='admin', - combine=False, - ) + users = project.get_project_users(access='admin', combine=False) self.assertEqual(len(users), 0) # He is just a committer - users = pagure.lib.get_project_users( - self.session, - project_obj=project, - access='commit', - combine=False, - ) + users = project.get_project_users(access='commit', combine=False) self.assertEqual(len(users), 1) self.assertEqual(users[0].username, 'foo') # He is just a committer - users = pagure.lib.get_project_users( - self.session, - project_obj=project, - access='ticket', - combine=False, - ) + users = project.get_project_users(access='ticket', combine=False) self.assertEqual(len(users), 0) # Update the access level of foo user to ticket @@ -3805,36 +3733,21 @@ class PagureLibtests(tests.Modeltests): # He is just a ticketer project = pagure.lib.get_project(self.session, name='test') - users = pagure.lib.get_project_users( - self.session, - project_obj=project, - access='admin', - combine=False, - ) + users = project.get_project_users(access='admin',combine=False) self.assertEqual(len(users), 0) # He is just a ticketer - users = pagure.lib.get_project_users( - self.session, - project_obj=project, - access='commit', - combine=False, - ) + users = project.get_project_users(access='commit', combine=False) self.assertEqual(len(users), 0) # He is just a ticketer - users = pagure.lib.get_project_users( - self.session, - project_obj=project, - access='ticket', - combine=False, - ) + users = project.get_project_users(access='ticket', combine=False) self.assertEqual(len(users), 1) self.assertEqual(users[0].username, 'foo') def test_get_project_groups(self): - ''' Test the get_project_groups method in pagure.lib - when combine is True ''' + ''' Test the get_project_groups method when combine is True + ''' # Create some projects tests.create_projects(self.session) @@ -3870,11 +3783,7 @@ class PagureLibtests(tests.Modeltests): # Now, the group is an admin in the project # so, it must have access to everything project = pagure.lib.get_project(self.session, name='test') - groups = pagure.lib.get_project_groups( - self.session, - project_obj=project, - access='admin', - ) + groups = project.get_project_groups(access='admin') self.assertEqual(len(groups), 1) self.assertEqual(groups[0].display_name, 'Justice League') self.assertEqual(len(project.admin_groups), 1) @@ -3884,11 +3793,7 @@ class PagureLibtests(tests.Modeltests): ) # The group should be committer as well - groups = pagure.lib.get_project_groups( - self.session, - project_obj=project, - access='commit', - ) + groups = project.get_project_groups(access='commit') self.assertEqual(len(groups), 1) self.assertEqual(groups[0].display_name, 'Justice League') self.assertEqual(len(project.committer_groups), 1) @@ -3898,11 +3803,7 @@ class PagureLibtests(tests.Modeltests): ) # The group should be ticketer as well - groups = pagure.lib.get_project_groups( - self.session, - project_obj=project, - access='ticket', - ) + groups = project.get_project_groups(access='ticket') self.assertEqual(len(groups), 1) self.assertEqual(groups[0].display_name, 'Justice League') self.assertEqual(len(project.groups), 1) @@ -3925,20 +3826,12 @@ class PagureLibtests(tests.Modeltests): # It shouldn't be an admin project = pagure.lib.get_project(self.session, name='test') - groups = pagure.lib.get_project_groups( - self.session, - project_obj=project, - access='admin', - ) + groups = project.get_project_groups(access='admin') self.assertEqual(len(groups), 0) self.assertEqual(len(project.admin_groups), 0) # It is a committer - groups = pagure.lib.get_project_groups( - self.session, - project_obj=project, - access='commit', - ) + groups = project.get_project_groups(access='commit') self.assertEqual(len(groups), 1) self.assertEqual(groups[0].display_name, 'Justice League') self.assertEqual(len(project.committer_groups), 1) @@ -3948,11 +3841,7 @@ class PagureLibtests(tests.Modeltests): ) # The group should be ticketer as well - groups = pagure.lib.get_project_groups( - self.session, - project_obj=project, - access='ticket', - ) + groups = project.get_project_groups(access='ticket') self.assertEqual(len(groups), 1) self.assertEqual(groups[0].display_name, 'Justice League') self.assertEqual(len(project.groups), 1) @@ -3974,29 +3863,17 @@ class PagureLibtests(tests.Modeltests): # It is not an admin project = pagure.lib.get_project(self.session, name='test') - groups = pagure.lib.get_project_groups( - self.session, - project_obj=project, - access='admin', - ) + groups = project.get_project_groups(access='admin') self.assertEqual(len(groups), 0) self.assertEqual(len(project.admin_groups), 0) # The group shouldn't be a committer - groups = pagure.lib.get_project_groups( - self.session, - project_obj=project, - access='commit', - ) + groups = project.get_project_groups(access='commit') self.assertEqual(len(groups), 0) self.assertEqual(len(project.committer_groups), 0) # The group should be ticketer - groups = pagure.lib.get_project_groups( - self.session, - project_obj=project, - access='ticket', - ) + groups = project.get_project_groups(access='ticket') self.assertEqual(len(groups), 1) self.assertEqual(groups[0].display_name, 'Justice League') self.assertEqual(len(project.groups), 1) @@ -4006,8 +3883,8 @@ class PagureLibtests(tests.Modeltests): ) def test_get_project_groups_combine_false(self): - ''' Test the get_project_groups method in pagure.lib - when combine is False ''' + ''' Test the get_project_groups method when combine is False + ''' # Create some projects tests.create_projects(self.session) @@ -4043,12 +3920,7 @@ class PagureLibtests(tests.Modeltests): # Now, the group is an admin in the project # so, it must have access to everything project = pagure.lib.get_project(self.session, name='test') - groups = pagure.lib.get_project_groups( - self.session, - project_obj=project, - access='admin', - combine=False, - ) + groups = project.get_project_groups(access='admin', combine=False) self.assertEqual(len(groups), 1) self.assertEqual(groups[0].display_name, 'Justice League') self.assertEqual(len(project.admin_groups), 1) @@ -4058,21 +3930,11 @@ class PagureLibtests(tests.Modeltests): ) # The group shoudn't be a committer - groups = pagure.lib.get_project_groups( - self.session, - project_obj=project, - access='commit', - combine=False, - ) + groups = project.get_project_groups(access='commit', combine=False) self.assertEqual(len(groups), 0) # The group shoudn't be a ticketer - groups = pagure.lib.get_project_groups( - self.session, - project_obj=project, - access='ticket', - combine=False, - ) + groups = project.get_project_groups(access='ticket', combine=False) self.assertEqual(len(groups), 0) # Update the access level of the group, JL to commit @@ -4089,21 +3951,11 @@ class PagureLibtests(tests.Modeltests): # It shouldn't be an admin project = pagure.lib.get_project(self.session, name='test') - groups = pagure.lib.get_project_groups( - self.session, - project_obj=project, - access='admin', - combine=False, - ) + groups = project.get_project_groups(access='admin', combine=False) self.assertEqual(len(groups), 0) # It is a committer - groups = pagure.lib.get_project_groups( - self.session, - project_obj=project, - access='commit', - combine=False, - ) + groups = project.get_project_groups(access='commit', combine=False) self.assertEqual(len(groups), 1) self.assertEqual(groups[0].display_name, 'Justice League') self.assertEqual(len(project.committer_groups), 1) @@ -4113,12 +3965,7 @@ class PagureLibtests(tests.Modeltests): ) # The group shouldn't be ticketer - groups = pagure.lib.get_project_groups( - self.session, - project_obj=project, - access='ticket', - combine=False, - ) + groups = project.get_project_groups(access='ticket', combine=False) self.assertEqual(len(groups), 0) # Update the access of group JL to ticket @@ -4134,30 +3981,15 @@ class PagureLibtests(tests.Modeltests): # It is not an admin project = pagure.lib.get_project(self.session, name='test') - groups = pagure.lib.get_project_groups( - self.session, - project_obj=project, - access='admin', - combine=False, - ) + groups = project.get_project_groups(access='admin', combine=False) self.assertEqual(len(groups), 0) # The group shouldn't be a committer - groups = pagure.lib.get_project_groups( - self.session, - project_obj=project, - access='commit', - combine=False, - ) + groups = project.get_project_groups(access='commit', combine=False) self.assertEqual(len(groups), 0) # The group should be ticketer - groups = pagure.lib.get_project_groups( - self.session, - project_obj=project, - access='ticket', - combine=False, - ) + groups = project.get_project_groups(access='ticket', combine=False) self.assertEqual(len(groups), 1) self.assertEqual(groups[0].display_name, 'Justice League') self.assertEqual(len(project.groups), 1)