From 8267d1a4c48276c88d6c2b8a98b4e2595f463369 Mon Sep 17 00:00:00 2001 From: Farhaan Bukhsh Date: Apr 14 2017 10:32:37 +0000 Subject: Fix pep8 convention and chages suggested by ``@pingou`` This commit few pep8 changes and changing ``lib/git.py`` to use ``_get_project`` instead of ``get_authorized_project`` Certain formatting changes have also been resolved Signed-off-by: Farhaan Bukhsh --- diff --git a/alembic/versions/4255158a6913_create_private_column_in_project_table.py b/alembic/versions/4255158a6913_create_private_column_in_project_table.py index 289e6af..7baeef6 100644 --- a/alembic/versions/4255158a6913_create_private_column_in_project_table.py +++ b/alembic/versions/4255158a6913_create_private_column_in_project_table.py @@ -26,7 +26,7 @@ def upgrade(): op.alter_column( 'projects', - column_name='private', new_column_name='private', + column_name='private', nullable=False, existing_nullable=True) diff --git a/doc/configuration.rst b/doc/configuration.rst index f212a7d..e48b519 100644 --- a/doc/configuration.rst +++ b/doc/configuration.rst @@ -608,7 +608,7 @@ PRIVATE_PROJECTS This configuration key allows you to host private repositories. These repositories are visible only to the creator of the repository and to the -user who are given access to the repository. No information is leaked about the +users who are given access to the repository. No information is leaked about the private repository which means redis doesn't have the access to the repository and even fedmsg doesn't get any notifications. diff --git a/pagure/__init__.py b/pagure/__init__.py index 7c30903..4d975d3 100644 --- a/pagure/__init__.py +++ b/pagure/__init__.py @@ -387,7 +387,7 @@ def get_authorized_project(session, project_name, user=None, namespace=None): :type namespace: String :return: The project object if project is public or user has permissions for the project else it returns None - :rtype: Project or [Project] + :rtype: Project ''' repo = pagure.lib._get_project(session, project_name, user, namespace) diff --git a/pagure/api/fork.py b/pagure/api/fork.py index c009e8a..1a2efd0 100644 --- a/pagure/api/fork.py +++ b/pagure/api/fork.py @@ -123,7 +123,8 @@ def api_pull_request_views(repo, username=None, namespace=None): """ - repo = pagure.get_authorized_project(SESSION, repo, user=username, namespace=namespace) + repo = pagure.get_authorized_project( + SESSION, repo, user=username, namespace=namespace) if repo is None: raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT) @@ -248,7 +249,8 @@ def api_pull_request_view(repo, requestid, username=None, namespace=None): """ - repo = pagure.get_authorized_project(SESSION, repo, user=username, namespace=namespace) + repo = pagure.get_authorized_project( + SESSION, repo, user=username, namespace=namespace) if repo is None: raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT) @@ -306,7 +308,8 @@ def api_pull_request_merge(repo, requestid, username=None, namespace=None): """ # noqa output = {} - repo = pagure.get_authorized_project(SESSION, repo, user=username, namespace=namespace) + repo = pagure.get_authorized_project( + SESSION, repo, user=username, namespace=namespace) if repo is None: raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT) @@ -392,7 +395,8 @@ def api_pull_request_close(repo, requestid, username=None, namespace=None): """ # noqa output = {} - repo = pagure.get_authorized_project(SESSION, repo, user=username, namespace=namespace) + repo = pagure.get_authorized_project( + SESSION, repo, user=username, namespace=namespace) if repo is None: raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT) @@ -494,7 +498,8 @@ def api_pull_request_add_comment( } """ # noqa - repo = pagure.get_authorized_project(SESSION, repo, user=username, namespace=namespace) + repo = pagure.get_authorized_project( + SESSION, repo, user=username, namespace=namespace) output = {} @@ -637,7 +642,8 @@ def api_pull_request_add_flag(repo, requestid, username=None, namespace=None): } """ # noqa - repo = pagure.get_authorized_project(SESSION, repo, user=username, namespace=namespace) + repo = pagure.get_authorized_project( + SESSION, repo, user=username, namespace=namespace) output = {} diff --git a/pagure/hooks/files/default_hook.py b/pagure/hooks/files/default_hook.py index e679c4e..7d5e05d 100755 --- a/pagure/hooks/files/default_hook.py +++ b/pagure/hooks/files/default_hook.py @@ -38,7 +38,7 @@ def run_as_post_receive_hook(): print('user:', username) print('namespace:', namespace) - project = pagure.get_authorized_project( + project = pagure.lib._get_project( pagure.SESSION, repo, user=username, namespace=namespace) for line in sys.stdin: diff --git a/pagure/hooks/files/fedmsg_hook.py b/pagure/hooks/files/fedmsg_hook.py index 350e5ab..bc67959 100755 --- a/pagure/hooks/files/fedmsg_hook.py +++ b/pagure/hooks/files/fedmsg_hook.py @@ -55,7 +55,7 @@ for line in sys.stdin.readlines(): project_name = pagure.lib.git.get_repo_name(abspath) username = pagure.lib.git.get_username(abspath) namespace = pagure.lib.git.get_repo_namespace(abspath) - project = pagure.get_authorized_project( + project = pagure._get_project( pagure.SESSION, project_name, username, namespace=namespace) if not project: project = project_name diff --git a/pagure/hooks/files/pagure_hook_tickets.py b/pagure/hooks/files/pagure_hook_tickets.py index a0c847c..7fb8a50 100755 --- a/pagure/hooks/files/pagure_hook_tickets.py +++ b/pagure/hooks/files/pagure_hook_tickets.py @@ -65,14 +65,14 @@ def run_as_post_receive_hook(): if REDIS: print('Sending to redis to load the data') REDIS.publish('pagure.loadjson', - json.dumps({ - 'project': project.to_json(public=True), - 'abspath': abspath, - 'commits': commits, - 'data_type': 'ticket', - 'agent': os.environ.get('GL_USER'), - } - )) + json.dumps({ + 'project': project.to_json(public=True), + 'abspath': abspath, + 'commits': commits, + 'data_type': 'ticket', + 'agent': os.environ.get('GL_USER'), + }) + ) print( 'A report will be emailed to you once the load is finished') else: diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index fd2b3f0..f18e577 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -140,6 +140,7 @@ def get_next_id(session, projectid): def search_user(session, username=None, email=None, token=None, pattern=None): ''' Searches the database for the user or users matching the given + criterias. :arg session: the session to use to connect to the database. :kwarg username: the username of the user to look for. @@ -609,8 +610,6 @@ def add_issue_dependency( repofolder=ticketfolder) if not issue.private: - #pagure.lib.notify.notify_assigned_issue(issue, user_obj) - #pagure.lib.notify.notify_assigned_issue(issue_blocked, user_obj) pagure.lib.notify.log( issue.project, topic='issue.dependency.added', @@ -668,8 +667,6 @@ def remove_issue_dependency( repofolder=ticketfolder) if not issue.private: - #pagure.lib.notify.notify_assigned_issue(issue, user_obj) - #pagure.lib.notify.notify_assigned_issue(issue_blocked, user_obj) pagure.lib.notify.log( issue.project, topic='issue.dependency.removed', @@ -1129,8 +1126,8 @@ def add_pull_request_comment(session, request, commit, tree_id, filename, # Send notification to the CI server, if the comment added was a # notification and the PR is still open and project is not private if notification and request.status == 'Open' \ - and request.project.ci_hook and PAGURE_CI \ - and not request.project.private: + and request.project.ci_hook and PAGURE_CI \ + and not request.project.private: REDIS.publish('pagure.ci', json.dumps({ 'ci_type': request.project.ci_hook.ci_type, 'pr': request.to_json(public=True, with_comments=False) @@ -1188,17 +1185,18 @@ def edit_comment(session, parent, comment, user, id_ = 'issue_id' private = parent.private - pagure.lib.notify.log( - parent.project, - topic=topic, - msg={ - key: parent.to_json(public=True, with_comments=False), - 'project': parent.project.to_json(public=True), - 'comment': comment.to_json(public=True), - 'agent': user_obj.username, - }, - redis=REDIS, - ) + if not private: + pagure.lib.notify.log( + parent.project, + topic=topic, + msg={ + key: parent.to_json(public=True, with_comments=False), + 'project': parent.project.to_json(public=True), + 'comment': comment.to_json(public=True), + 'agent': user_obj.username, + }, + redis=REDIS, + ) if REDIS and not parent.project.private: if private: @@ -2000,7 +1998,6 @@ def search_projects( model.Project.private == True, ) ) - ) if fork is not None: @@ -2012,7 +2009,6 @@ def search_projects( projects = projects.filter( model.Project.is_fork == False ) - if tags: if not isinstance(tags, (list, tuple)): tags = [tags] @@ -2023,7 +2019,6 @@ def search_projects( model.TagProject.tag.in_(tags) ) - if pattern: pattern = pattern.replace('*', '%') if '%' in pattern: diff --git a/pagure/lib/git.py b/pagure/lib/git.py index fdedf19..affa5e6 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -411,7 +411,8 @@ def get_project_from_json( project_user = None if jsondata.get('parent'): project_user = user.username - project = pagure.get_authorized_project(session, name, user=project_user, namespace=namespace) + project = pagure.lib._get_project( + session, name, user=project_user, namespace=namespace) if not project: parent = None @@ -450,7 +451,8 @@ def get_project_from_json( ) session.commit() - project = pagure.get_authorized_project(session, name, user=user.username, namespace=namespace) + project = pagure.lib._get_project( + session, name, user=user.username, namespace=namespace) tags = jsondata.get('tags', None) if tags: @@ -527,7 +529,9 @@ def update_ticket_from_git( """ - repo = pagure.get_authorized_project(session, reponame, user=username, namespace=namespace) + repo = pagure.lib._get_project( + session, reponame, user=username, namespace=namespace) + if not repo: raise pagure.exceptions.PagureException( 'Unknown repo %s of username: %s in namespace: %s' % ( @@ -704,7 +708,8 @@ def update_request_from_git( """ - repo = pagure.get_authorized_project(session, reponame, user=username, namespace=namespace) + repo = pagure.lib._get_project( + session, reponame, user=username, namespace=namespace) if not repo: raise pagure.exceptions.PagureException( 'Unknown repo %s of username: %s in namespace: %s' % ( diff --git a/pagure/pfmarkdown.py b/pagure/pfmarkdown.py index 61d2b57..aa86f4e 100644 --- a/pagure/pfmarkdown.py +++ b/pagure/pfmarkdown.py @@ -316,8 +316,8 @@ def makeExtension(*arg, **kwargs): def _issue_exists(user, namespace, repo, idx): """ Utility method checking if a given issue exists. """ - repo_obj = pagure.get_authorized_project(pagure.SESSION, project_name=repo, - user=user, namespace=namespace) + repo_obj = pagure.get_authorized_project( + pagure.SESSION, project_name=repo, user=user, namespace=namespace) if not repo_obj: return False @@ -332,8 +332,8 @@ def _issue_exists(user, namespace, repo, idx): def _pr_exists(user, namespace, repo, idx): """ Utility method checking if a given PR exists. """ - repo_obj = pagure.get_authorized_project(pagure.SESSION, project_name=repo, - user=user, namespace=namespace) + repo_obj = pagure.get_authorized_project( + pagure.SESSION, project_name=repo, user=user, namespace=namespace) if not repo_obj: return False diff --git a/pagure/templates/new_project.html b/pagure/templates/new_project.html index bf22010..8ffb023 100644 --- a/pagure/templates/new_project.html +++ b/pagure/templates/new_project.html @@ -21,7 +21,7 @@ {{ render_bootstrap_field(form.url, field_description="url of the project's website") }} {{ render_bootstrap_field(form.avatar_email, field_description="libravatar email address avatar email") }} {% if config.get('PRIVATE_PROJECTS', False) %} - {{ render_bootstrap_field(form.private, field_description="To mark the repo private") }} + {{ render_bootstrap_field(form.private, field_description="To mark the repo private") }} {% endif %} {{ render_bootstrap_field(form.create_readme, field_description="Create a README file automatically") }} diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index f8ed3b1..5449f31 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -50,11 +50,10 @@ import pagure.forms import pagure import pagure.ui.plugins from pagure import (APP, SESSION, LOG, __get_file_in_tree, login_required, - is_repo_admin, admin_session_timedout) + admin_session_timedout) from pagure.lib import encoding_utils - @APP.route('/.git') @APP.route('//.git') @APP.route('/fork//.git') @@ -2102,9 +2101,6 @@ def edit_file(repo, branchname, filename, username=None, namespace=None): 403, 'You are not allowed to change the settings for this project') - if repo.private and not is_repo_admin(repo): - flask.abort(401, 'Forbidden') - user = pagure.lib.search_user( SESSION, username=flask.g.fas_user.username) diff --git a/tests/test_pagure_flask_api_issue.py b/tests/test_pagure_flask_api_issue.py index df93ee3..adbb43b 100644 --- a/tests/test_pagure_flask_api_issue.py +++ b/tests/test_pagure_flask_api_issue.py @@ -615,6 +615,8 @@ class PagureFlaskApiIssuetests(tests.Modeltests): 'title': 'test issue', 'issue_content': 'This issue needs attention', } + + # Valid request output = self.app.post( '/api/0/test/new_issue', data=data, headers=headers) self.assertEqual(output.status_code, 200)