From c27d6105a9fb1e62f798061dcfaa7cb1d4021d9d Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 22 2017 16:19:17 +0000 Subject: [PATCH 1/8] Return the flag UID when adding or updating a flag on a PR not in fedmsg The UID of a flag is what allows to update it, so it shouldn't be part of the fedmsg message we sent. However, if the user is not specifying the UID, then they have no way to know it unless we return it, so when adding or updating a flag, return the JSON representation of that flag object. Adjust the unit-tests accordingly. Fixes https://pagure.io/pagure/issue/1588 Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/api/fork.py b/pagure/api/fork.py index 1dc7f5e..89d495f 100644 --- a/pagure/api/fork.py +++ b/pagure/api/fork.py @@ -648,13 +648,39 @@ def api_pull_request_add_flag(repo, requestid, username=None, namespace=None): :: { - "message": "Flag added" + "flag": { + "comment": "Tests failed", + "date_created": "1510742565", + "percent": 0, + "pull_request_uid": "62b49f00d489452994de5010565fab81", + "url": "http://jenkins.cloud.fedoraproject.org/", + "user": { + "default_email": "bar@pingou.com", + "emails": ["bar@pingou.com", "foo@pingou.com"], + "fullname": "PY C", + "name": "pingou"}, + "username": "Jenkins"}, + "message": u"Flag added", + "uid": u"jenkins_build_pagure_100+seed" } :: { - "message": "Flag updated" + "flag": { + "comment": "Tests failed", + "date_created": "1510742565", + "percent": 0, + "pull_request_uid": "62b49f00d489452994de5010565fab81", + "url": "http://jenkins.cloud.fedoraproject.org/", + "user": { + "default_email": "bar@pingou.com", + "emails": ["bar@pingou.com", "foo@pingou.com"], + "fullname": "PY C", + "name": "pingou"}, + "username": "Jenkins"}, + "message": u"Flag updated", + "uid": u"jenkins_build_pagure_100+seed" } """ # noqa @@ -690,7 +716,7 @@ def api_pull_request_add_flag(repo, requestid, username=None, namespace=None): uid = form.uid.data.strip() if form.uid.data else None try: # New Flag - message = pagure.lib.add_pull_request_flag( + message, uid = pagure.lib.add_pull_request_flag( SESSION, request=request, username=username, @@ -703,7 +729,10 @@ def api_pull_request_add_flag(repo, requestid, username=None, namespace=None): requestfolder=APP.config['REQUESTS_FOLDER'], ) SESSION.commit() + pr_flag = pagure.lib.get_pull_request_flag_by_uid(SESSION, uid) output['message'] = message + output['uid'] = uid + output['flag'] = pr_flag.to_json() except pagure.exceptions.PagureException as err: raise pagure.exceptions.APIError( 400, error_code=APIERROR.ENOCODE, error=str(err)) diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 754da8a..77e1c57 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -1316,7 +1316,7 @@ def add_pull_request_flag(session, request, username, percent, comment, url, redis=REDIS, ) - return 'Flag %s' % action + return ('Flag %s' % action, pr_flag.uid) def new_project(session, user, name, blacklist, allowed_prefix, diff --git a/pagure/lib/model.py b/pagure/lib/model.py index 05e3f90..236793b 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -2001,7 +2001,6 @@ class PullRequestFlag(BASE): ''' output = { - 'uid': self.uid, 'pull_request_uid': self.pull_request_uid, 'username': self.username, 'percent': self.percent, diff --git a/tests/test_pagure_flask_api_fork.py b/tests/test_pagure_flask_api_fork.py index 5c10619..8e52ff9 100644 --- a/tests/test_pagure_flask_api_fork.py +++ b/tests/test_pagure_flask_api_fork.py @@ -31,6 +31,8 @@ import tests class PagureFlaskApiForktests(tests.Modeltests): """ Tests for the flask API of pagure for issue """ + maxDiff = None + def setUp(self): """ Set up the environnment, ran before every tests. """ super(PagureFlaskApiForktests, self).setUp() @@ -1064,9 +1066,26 @@ class PagureFlaskApiForktests(tests.Modeltests): '/api/0/test/pull-request/1/flag', data=data, headers=headers) self.assertEqual(output.status_code, 200) data = json.loads(output.data) + data['flag']['date_created'] = u'1510742565' + data['flag']['pull_request_uid'] = u'62b49f00d489452994de5010565fab81' self.assertDictEqual( data, - {'message': 'Flag added'} + { + u'flag': { + u'comment': u'Tests failed', + u'date_created': u'1510742565', + u'percent': 0, + u'pull_request_uid': u'62b49f00d489452994de5010565fab81', + u'url': u'http://jenkins.cloud.fedoraproject.org/', + u'user': { + u'default_email': u'bar@pingou.com', + u'emails': [u'bar@pingou.com', u'foo@pingou.com'], + u'fullname': u'PY C', + u'name': u'pingou'}, + u'username': u'Jenkins'}, + u'message': u'Flag added', + u'uid': u'jenkins_build_pagure_100+seed' + } ) # One flag added @@ -1089,9 +1108,26 @@ class PagureFlaskApiForktests(tests.Modeltests): '/api/0/test/pull-request/1/flag', data=data, headers=headers) self.assertEqual(output.status_code, 200) data = json.loads(output.data) + data['flag']['date_created'] = u'1510742565' + data['flag']['pull_request_uid'] = u'62b49f00d489452994de5010565fab81' self.assertDictEqual( data, - {'message': 'Flag updated'} + { + u'flag': { + u'comment': u'Tests passed', + u'date_created': u'1510742565', + u'percent': 100, + u'pull_request_uid': u'62b49f00d489452994de5010565fab81', + u'url': u'http://jenkins.cloud.fedoraproject.org/', + u'user': { + u'default_email': u'bar@pingou.com', + u'emails': [u'bar@pingou.com', u'foo@pingou.com'], + u'fullname': u'PY C', + u'name': u'pingou'}, + u'username': u'Jenkins'}, + u'message': u'Flag updated', + u'uid': u'jenkins_build_pagure_100+seed' + } ) # One flag added @@ -1212,9 +1248,26 @@ class PagureFlaskApiForktests(tests.Modeltests): '/api/0/test/pull-request/1/flag', data=data, headers=headers) self.assertEqual(output.status_code, 200) data = json.loads(output.data) + data['flag']['date_created'] = u'1510742565' + data['flag']['pull_request_uid'] = u'62b49f00d489452994de5010565fab81' self.assertDictEqual( data, - {'message': 'Flag added'} + { + u'flag': { + u'comment': u'Tests failed', + u'date_created': u'1510742565', + u'percent': 0, + u'pull_request_uid': u'62b49f00d489452994de5010565fab81', + u'url': u'http://jenkins.cloud.fedoraproject.org/', + u'user': { + u'default_email': u'bar@pingou.com', + u'emails': [u'bar@pingou.com', u'foo@pingou.com'], + u'fullname': u'PY C', + u'name': u'pingou'}, + u'username': u'Jenkins'}, + u'message': u'Flag added', + u'uid': u'jenkins_build_pagure_100+seed' + } ) # One flag added @@ -1237,9 +1290,26 @@ class PagureFlaskApiForktests(tests.Modeltests): '/api/0/test/pull-request/1/flag', data=data, headers=headers) self.assertEqual(output.status_code, 200) data = json.loads(output.data) + data['flag']['date_created'] = u'1510742565' + data['flag']['pull_request_uid'] = u'62b49f00d489452994de5010565fab81' self.assertDictEqual( data, - {'message': 'Flag updated'} + { + u'flag': { + u'comment': u'Tests passed', + u'date_created': u'1510742565', + u'percent': 100, + u'pull_request_uid': u'62b49f00d489452994de5010565fab81', + u'url': u'http://jenkins.cloud.fedoraproject.org/', + u'user': { + u'default_email': u'bar@pingou.com', + u'emails': [u'bar@pingou.com', u'foo@pingou.com'], + u'fullname': u'PY C', + u'name': u'pingou'}, + u'username': u'Jenkins'}, + u'message': u'Flag updated', + u'uid': u'jenkins_build_pagure_100+seed' + } ) # One flag added diff --git a/tests/test_pagure_flask_api_ui_private_repo.py b/tests/test_pagure_flask_api_ui_private_repo.py index 12d5bb6..0346a74 100644 --- a/tests/test_pagure_flask_api_ui_private_repo.py +++ b/tests/test_pagure_flask_api_ui_private_repo.py @@ -1653,9 +1653,26 @@ class PagurePrivateRepotest(tests.Modeltests): '/api/0/test4/pull-request/1/flag', data=data, headers=headers) self.assertEqual(output.status_code, 200) data = json.loads(output.get_data(as_text=True)) + data['flag']['date_created'] = u'1510742565' + data['flag']['pull_request_uid'] = u'62b49f00d489452994de5010565fab81' self.assertDictEqual( data, - {'message': 'Flag added'} + { + u'flag': { + u'comment': u'Tests failed', + u'date_created': u'1510742565', + u'percent': 0, + u'pull_request_uid': u'62b49f00d489452994de5010565fab81', + u'url': u'http://jenkins.cloud.fedoraproject.org/', + u'user': { + u'default_email': u'bar@pingou.com', + u'emails': [u'bar@pingou.com', u'foo@pingou.com'], + u'fullname': u'PY C', + u'name': u'pingou'}, + u'username': u'Jenkins'}, + u'message': u'Flag added', + u'uid': u'jenkins_build_pagure_100+seed' + } ) # One flag added @@ -1678,9 +1695,26 @@ class PagurePrivateRepotest(tests.Modeltests): '/api/0/test4/pull-request/1/flag', data=data, headers=headers) self.assertEqual(output.status_code, 200) data = json.loads(output.get_data(as_text=True)) + data['flag']['date_created'] = u'1510742565' + data['flag']['pull_request_uid'] = u'62b49f00d489452994de5010565fab81' self.assertDictEqual( data, - {'message': 'Flag updated'} + { + u'flag': { + u'comment': u'Tests passed', + u'date_created': u'1510742565', + u'percent': 100, + u'pull_request_uid': u'62b49f00d489452994de5010565fab81', + u'url': u'http://jenkins.cloud.fedoraproject.org/', + u'user': { + u'default_email': u'bar@pingou.com', + u'emails': [u'bar@pingou.com', u'foo@pingou.com'], + u'fullname': u'PY C', + u'name': u'pingou'}, + u'username': u'Jenkins'}, + u'message': u'Flag updated', + u'uid': u'jenkins_build_pagure_100+seed' + } ) # One flag added diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py index 0ebd630..9f17980 100644 --- a/tests/test_pagure_lib.py +++ b/tests/test_pagure_lib.py @@ -2601,7 +2601,7 @@ class PagureLibtests(tests.Modeltests): token='aaabbbcccddd', requestfolder=None, ) - self.assertEqual(msg, 'Flag added') + self.assertEqual(msg, ('Flag added', 'jenkins_build_pagure_34')) self.session.commit() self.assertEqual(len(request.flags), 1) From 8715b5a7afb6b92fe4fda4e257af42d99934e802 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 22 2017 16:19:17 +0000 Subject: [PATCH 2/8] Add flags on commits This commit allow to flag commits much like PR can be flagged. Signed-off-by: Pierre-Yves Chibon --- diff --git a/alembic/versions/2b626a16542e_commit_flag.py b/alembic/versions/2b626a16542e_commit_flag.py new file mode 100644 index 0000000..503cff0 --- /dev/null +++ b/alembic/versions/2b626a16542e_commit_flag.py @@ -0,0 +1,54 @@ +"""commit flag + +Revision ID: 2b626a16542e +Revises: 2fb229dac744 +Create Date: 2017-11-15 10:06:55.088665 + +""" + +import datetime + +from alembic import op +import sqlalchemy as sa + +# revision identifiers, used by Alembic. +revision = '2b626a16542e' +down_revision = '2fb229dac744' + + +def upgrade(): + ''' Create the commit_flags table. ''' + + op.create_table( + 'commit_flags', + sa.Column('id', sa.Integer, primary_key=True), + sa.Column('uid', sa.String(32), unique=True, nullable=False), + sa.Column('commit_hash', sa.String(40), index=True, nullable=False), + sa.Column( + 'token_id', sa.String(64), + sa.ForeignKey('tokens.id'), nullable=False), + sa.Column( + 'project_id', + sa.Integer, + sa.ForeignKey( + 'projects.id', onupdate='CASCADE', ondelete='CASCADE', + ), + nullable=False, index=True), + sa.Column( + 'user_id', sa.Integer, + sa.ForeignKey('users.id', onupdate='CASCADE'), + nullable=False, index=True), + sa.Column('username', sa.Text(), nullable=False), + sa.Column('percent', sa.Integer(), nullable=False), + sa.Column('comment', sa.Text(), nullable=False), + sa.Column('url', sa.Text(), nullable=False), + sa.Column( + 'date_created', sa.DateTime, nullable=False, + default=datetime.datetime.utcnow), + ) + + +def downgrade(): + ''' Drop the commit_flags table. ''' + + op.drop_table('commit_flags') diff --git a/pagure/api/__init__.py b/pagure/api/__init__.py index 6ce2212..293f0ab 100644 --- a/pagure/api/__init__.py +++ b/pagure/api/__init__.py @@ -90,6 +90,7 @@ class APIERROR(enum.Enum): EMODIFYPROJECTNOTALLOWED = 'You are not allowed to modify this project' EINVALIDPERPAGEVALUE = 'The per_page value must be between 1 and 100' EGITERROR = 'An error occured during a git operation' + ENOCOMMIT = 'No such commit found in this repository' def get_authorized_api_project(SESSION, repo, user=None, namespace=None): @@ -441,6 +442,7 @@ def api(): api_fork_project_doc = load_doc(project.api_fork_project) api_generate_acls_doc = load_doc(project.api_generate_acls) api_new_branch_doc = load_doc(project.api_new_branch) + api_commit_add_flag_doc = load_doc(project.api_commit_add_flag) issues = [] if pagure.APP.config.get('ENABLE_TICKETS', True): @@ -507,7 +509,8 @@ def api(): api_git_branches_doc, api_fork_project_doc, api_generate_acls_doc, - api_new_branch_doc + api_new_branch_doc, + api_commit_add_flag_doc, ], issues=issues, requests=[ diff --git a/pagure/api/project.py b/pagure/api/project.py index b06de7a..0346c69 100644 --- a/pagure/api/project.py +++ b/pagure/api/project.py @@ -12,9 +12,10 @@ import flask from sqlalchemy.exc import SQLAlchemyError from six import string_types -from pygit2 import GitError +from pygit2 import GitError, Repository import pagure +import pagure.forms import pagure.exceptions import pagure.lib import pagure.lib.git @@ -1267,3 +1268,175 @@ def api_new_branch(repo, username=None, namespace=None): output = {'message': 'Project branch was created'} jsonout = flask.jsonify(output) return jsonout + + +@API.route('//c//flag', methods=['POST']) +@API.route('///c//flag', methods=['POST']) +@API.route('/fork///c//flag', methods=['POST']) +@API.route( + '/fork////c//flag', + methods=['POST']) +@api_login_required(acls=['commit_flag']) +@api_method +def api_commit_add_flag(repo, commit_hash, username=None, namespace=None): + """ + Flag a commit + ------------------- + Add or edit flags on a commit. + + :: + + POST /api/0//c//flag + POST /api/0///c//flag + + :: + + POST /api/0/fork///c//flag + POST /api/0/fork////c//flag + + Input + ^^^^^ + + +---------------+---------+--------------+-----------------------------+ + | Key | Type | Optionality | Description | + +===============+=========+==============+=============================+ + | ``username`` | string | Mandatory | | The name of the | + | | | | application to be | + | | | | presented to users | + | | | | on the pull request page | + +---------------+---------+--------------+-----------------------------+ + | ``percent`` | int | Mandatory | | A percentage of | + | | | | completion compared to | + | | | | the goal. The percentage | + | | | | also determine the | + | | | | background color of the | + | | | | flag on the pull-request | + | | | | page | + +---------------+---------+--------------+-----------------------------+ + | ``comment`` | string | Mandatory | | A short message | + | | | | summarizing the | + | | | | presented results | + +---------------+---------+--------------+-----------------------------+ + | ``url`` | string | Mandatory | | A URL to the result | + | | | | of this flag | + +---------------+---------+--------------+-----------------------------+ + | ``uid`` | string | Optional | | A unique identifier used | + | | | | to identify a flag on a | + | | | | pull-request. If the | + | | | | provided UID matches an | + | | | | existing one, then the | + | | | | API call will update the | + | | | | existing one rather than | + | | | | create a new one. | + | | | | Maximum Length: 32 | + | | | | characters. Default: an | + | | | | auto generated UID | + +---------------+---------+--------------+-----------------------------+ + + + Sample response + ^^^^^^^^^^^^^^^ + + :: + + { + "flag": { + "comment": "Tests passed", + "commit_hash": "62b49f00d489452994de5010565fab81", + "date_created": "1510742565", + "percent": 100, + "url": "http://jenkins.cloud.fedoraproject.org/", + "user": { + "default_email": "bar@pingou.com", + "emails": ["bar@pingou.com", "foo@pingou.com"], + "fullname": "PY C", + "name": "pingou"}, + "username": "Jenkins" + }, + "message": "Flag added", + "uid": "b1de8f80defd4a81afe2e09f39678087" + } + + :: + + { + "flag": { + "comment": "Tests passed", + "commit_hash": "62b49f00d489452994de5010565fab81", + "date_created": "1510742565", + "percent": 100, + "url": "http://jenkins.cloud.fedoraproject.org/", + "user": { + "default_email": "bar@pingou.com", + "emails": ["bar@pingou.com", "foo@pingou.com"], + "fullname": "PY C", + "name": "pingou"}, + "username": "Jenkins" + }, + "message": "Flag updated", + "uid": "b1de8f80defd4a81afe2e09f39678087" + } + + """ # noqa + + repo = get_authorized_api_project( + SESSION, repo, user=username, namespace=namespace) + + output = {} + + if repo is None: + raise pagure.exceptions.APIError( + 404, error_code=APIERROR.ENOPROJECT) + + if flask.g.token.project and repo != flask.g.token.project: + raise pagure.exceptions.APIError( + 401, error_code=APIERROR.EINVALIDTOK) + + reponame = pagure.get_repo_path(repo) + repo_obj = Repository(reponame) + try: + repo_obj.get(commit_hash) + except ValueError: + raise pagure.exceptions.APIError( + 404, error_code=APIERROR.ENOCOMMIT) + + form = pagure.forms.AddPullRequestFlagForm(csrf_enabled=False) + if form.validate_on_submit(): + username = form.username.data + percent = form.percent.data + comment = form.comment.data.strip() + url = form.url.data.strip() + uid = form.uid.data.strip() if form.uid.data else None + try: + # New Flag + message, uid = pagure.lib.add_commit_flag( + session=SESSION, + repo=repo, + commit_hash=commit_hash, + username=username, + percent=percent, + comment=comment, + url=url, + uid=uid, + user=flask.g.fas_user.username, + token=flask.g.token.id, + ) + SESSION.commit() + c_flag = pagure.lib.get_commit_flag_by_uid(SESSION, uid) + output['message'] = message + output['uid'] = uid + output['flag'] = c_flag.to_json() + except pagure.exceptions.PagureException as err: + raise pagure.exceptions.APIError( + 400, error_code=APIERROR.ENOCODE, error=str(err)) + except SQLAlchemyError as err: # pragma: no cover + APP.logger.exception(err) + SESSION.rollback() + raise pagure.exceptions.APIError( + 400, error_code=APIERROR.EDBERROR) + else: + raise pagure.exceptions.APIError( + 400, error_code=APIERROR.EINVALIDREQ, errors=form.errors) + + jsonout = flask.jsonify(output) + return jsonout diff --git a/pagure/default_config.py b/pagure/default_config.py index f2e1bb1..f98d19b 100644 --- a/pagure/default_config.py +++ b/pagure/default_config.py @@ -262,7 +262,8 @@ ACLS = { 'issue_update_custom_fields': 'Update the custom fields of an issue', 'issue_update_milestone': 'Update the milestone of an issue', 'modify_project': 'Modify an existing project', - 'generate_acls_project': 'Generate the Gitolite ACLs on a project' + 'generate_acls_project': 'Generate the Gitolite ACLs on a project', + 'commit_flag': 'Flag a commit', } # List of ACLs which a regular user is allowed to associate to an API token @@ -285,7 +286,8 @@ ADMIN_API_ACLS = [ 'pull_request_flag', 'pull_request_comment', 'pull_request_merge', - 'generate_acls_project' + 'generate_acls_project', + 'commit_flag', ] # Bootstrap URLS diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 77e1c57..54ef5f8 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -1319,6 +1319,71 @@ def add_pull_request_flag(session, request, username, percent, comment, url, return ('Flag %s' % action, pr_flag.uid) +def add_commit_flag( + session, repo, commit_hash, username, status, percent, comment, url, + uid, user, token): + ''' Add a flag to a add_commit_flag. ''' + user_obj = get_user(session, user) + + action = 'added' + c_flag = get_commit_flag_by_uid(session, uid) + if c_flag: + action = 'updated' + c_flag.comment = comment + c_flag.percent = percent + c_flag.url = url + else: + c_flag = model.CommitFlag( + uid=uid or uuid.uuid4().hex, + project_id=repo.id, + commit_hash=commit_hash, + username=username, + percent=percent, + comment=comment, + url=url, + user_id=user_obj.id, + token_id=token, + ) + session.add(c_flag) + # Make sure we won't have SQLAlchemy error before we continue + session.flush() + + pagure.lib.notify.log( + repo, + topic='commit.flag.%s' % action, + msg=dict( + repo=repo.to_json(public=True), + flag=c_flag.to_json(public=True), + agent=user_obj.username, + ), + redis=REDIS, + ) + + return ('Flag %s' % action, c_flag.uid) + + +def get_commit_flag(session, project, commit_hash): + ''' Return the commit flags corresponding to the specified git hash + (commitid) in the specified repository. + + :arg session: the session with which to connect to the database + :arg repo: the pagure.lib.model.Project object corresponding to the + project whose commit has been flagged + :arg commit_hash: the hash of the commit who has been flagged + :return: list of pagure.lib.model.CommitFlag objects or an empty list + + ''' + query = session.query( + model.CommitFlag + ).filter( + model.CommitFlag.project_id == project.id + ).filter( + model.CommitFlag.commit_hash == commit_hash + ) + + return query.all() + + def new_project(session, user, name, blacklist, allowed_prefix, gitfolder, docfolder, ticketfolder, requestfolder, description=None, url=None, avatar_email=None, @@ -2811,6 +2876,27 @@ def get_pull_request_flag_by_uid(session, flag_uid): return query.first() +def get_commit_flag_by_uid(session, flag_uid): + ''' Return the flag corresponding to the specified unique identifier. + + :arg session: the session to use to connect to the database. + :arg flag_uid: the unique identifier of a request. This identifier is + unique accross all flags on this pagure instance and should be + unique accross multiple pagure instances as well + :type request_uid: str or None + + :return: A single Issue object. + :rtype: pagure.lib.model.PullRequestFlag + + ''' + query = session.query( + model.CommitFlag + ).filter( + model.CommitFlag.uid == flag_uid.strip() if flag_uid else None + ) + return query.first() + + def set_up_user(session, username, fullname, default_email, emails=None, ssh_key=None, keydir=None): ''' Set up a new user into the database or update its information. ''' diff --git a/pagure/lib/model.py b/pagure/lib/model.py index 236793b..e137112 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -2013,6 +2013,74 @@ class PullRequestFlag(BASE): return output +class CommitFlag(BASE): + """ Stores the flags attached to a commit. + + Table -- commit_flags + """ + + __tablename__ = 'commit_flags' + + id = sa.Column(sa.Integer, primary_key=True) + commit_hash = sa.Column(sa.String(40), index=True, nullable=False) + project_id = sa.Column( + sa.Integer, + sa.ForeignKey( + 'projects.id', onupdate='CASCADE', ondelete='CASCADE', + ), + nullable=False, index=True) + token_id = sa.Column( + sa.String(64), sa.ForeignKey( + 'tokens.id', + ), + nullable=False) + user_id = sa.Column( + sa.Integer, + sa.ForeignKey( + 'users.id', onupdate='CASCADE', + ), + nullable=False, + index=True) + uid = sa.Column(sa.String(32), unique=True, nullable=False) + username = sa.Column( + sa.Text(), + nullable=False) + percent = sa.Column( + sa.Integer(), + nullable=False) + comment = sa.Column( + sa.Text(), + nullable=False) + url = sa.Column( + sa.Text(), + nullable=False) + + date_created = sa.Column(sa.DateTime, nullable=False, + default=datetime.datetime.utcnow) + + user = relation('User', foreign_keys=[user_id], + remote_side=[User.id], + backref=backref( + 'commit_flags', + order_by="CommitFlag.date_created")) + + def to_json(self, public=False): + ''' Returns a dictionnary representation of the commit flag. + + ''' + output = { + 'commit_hash': self.commit_hash, + 'username': self.username, + 'percent': self.percent, + 'comment': self.comment, + 'url': self.url, + 'date_created': self.date_created.strftime('%s'), + 'user': self.user.to_json(public=public), + } + + return output + + class PagureGroupType(BASE): """ A list of the type a group can have definition. diff --git a/pagure/templates/commit.html b/pagure/templates/commit.html index 0534892..00a9c93 100644 --- a/pagure/templates/commit.html +++ b/pagure/templates/commit.html @@ -129,6 +129,31 @@ {% endif %} +
+
+
    + {% for flag in flags %} +
  • +
    + {{ flag.username }} +
    {{ flag.percent }}%
    +
    +
    + {{ flag.comment }} +
    + {{ flag.date_created | humanize }}
    +
    +
    +
  • + {% endfor %} +
+
+
+ {% set filecount = 0 %} {% for patch in diff %} {% set filecount = filecount + 1 %} diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index aa20dc7..5dfc70c 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -753,6 +753,7 @@ def view_commit(repo, commitid, username=None, namespace=None): commit=commit, diff=diff, form=pagure.forms.ConfirmationForm(), + flags=pagure.lib.get_commit_flag(SESSION, repo, commitid), ) diff --git a/tests/test_pagure_flask_api_issue.py b/tests/test_pagure_flask_api_issue.py index 4665619..1a71fe1 100644 --- a/tests/test_pagure_flask_api_issue.py +++ b/tests/test_pagure_flask_api_issue.py @@ -2522,7 +2522,7 @@ class PagureFlaskApiIssuetests(tests.Modeltests): # is required item = pagure.lib.model.TokenAcl( token_id='pingou_foo', - acl_id=5, + acl_id=6, ) self.session.add(item) self.session.commit() diff --git a/tests/test_pagure_flask_api_project.py b/tests/test_pagure_flask_api_project.py index 4327b52..9643dcd 100644 --- a/tests/test_pagure_flask_api_project.py +++ b/tests/test_pagure_flask_api_project.py @@ -2713,5 +2713,250 @@ class PagureFlaskApiProjecttests(tests.Modeltests): self.assertEqual(data, expected_output) self.assertIn('test123', repo_obj.listall_branches()) + +class PagureFlaskApiProjectFlagtests(tests.Modeltests): + """ Tests for the flask API of pagure for flagging commit in project + """ + + def setUp(self): + """ Set up the environnment, ran before every tests. """ + super(PagureFlaskApiProjectFlagtests, self).setUp() + + pagure.APP.config['TESTING'] = True + pagure.SESSION = self.session + pagure.api.SESSION = self.session + pagure.api.project.SESSION = self.session + pagure.lib.SESSION = self.session + + tests.create_projects(self.session) + repo_path = os.path.join(self.path, 'repos') + self.git_path = os.path.join(repo_path, 'test.git') + tests.create_projects_git(repo_path, bare=True) + tests.add_content_git_repo(self.git_path) + tests.create_tokens(self.session, project_id=None) + tests.create_tokens_acl( + self.session, 'aaabbbcccddd', 'commit_flag') + + def test_flag_commit_missing_percent(self): + """ Test flagging a commit with missing precentage. """ + repo_obj = pygit2.Repository(self.git_path) + commit = repo_obj.revparse_single('HEAD') + + headers = {'Authorization': 'token aaabbbcccddd'} + data = { + 'username': 'Jenkins', + 'comment': 'Tests passed', + 'url': 'http://jenkins.cloud.fedoraproject.org/', + 'uid': 'jenkins_build_pagure_100+seed', + } + output = self.app.post( + '/api/0/test/c/%s/flag' % commit.oid.hex, + headers=headers, data=data) + self.assertEqual(output.status_code, 400) + data = json.loads(output.data) + expected_output = { + "error": "Invalid or incomplete input submited", + "error_code": "EINVALIDREQ", + "errors": { + "percent": [ + "This field is required." + ] + } + } + self.assertEqual(data, expected_output) + + def test_flag_commit_missing_username(self): + """ Test flagging a commit with missing username. """ + repo_obj = pygit2.Repository(self.git_path) + commit = repo_obj.revparse_single('HEAD') + + headers = {'Authorization': 'token aaabbbcccddd'} + data = { + 'percent': 100, + 'comment': 'Tests passed', + 'url': 'http://jenkins.cloud.fedoraproject.org/', + 'uid': 'jenkins_build_pagure_100+seed', + } + output = self.app.post( + '/api/0/test/c/%s/flag' % commit.oid.hex, + headers=headers, data=data) + self.assertEqual(output.status_code, 400) + data = json.loads(output.data) + expected_output = { + "error": "Invalid or incomplete input submited", + "error_code": "EINVALIDREQ", + "errors": { + "username": [ + "This field is required." + ] + } + } + self.assertEqual(data, expected_output) + + def test_flag_commit_missing_comment(self): + """ Test flagging a commit with missing comment. """ + repo_obj = pygit2.Repository(self.git_path) + commit = repo_obj.revparse_single('HEAD') + + headers = {'Authorization': 'token aaabbbcccddd'} + data = { + 'username': 'Jenkins', + 'percent': 100, + 'url': 'http://jenkins.cloud.fedoraproject.org/', + 'uid': 'jenkins_build_pagure_100+seed', + } + output = self.app.post( + '/api/0/test/c/%s/flag' % commit.oid.hex, + headers=headers, data=data) + self.assertEqual(output.status_code, 400) + data = json.loads(output.data) + expected_output = { + "error": "Invalid or incomplete input submited", + "error_code": "EINVALIDREQ", + "errors": { + "comment": [ + "This field is required." + ] + } + } + self.assertEqual(data, expected_output) + + def test_flag_commit_missing_url(self): + """ Test flagging a commit with missing url. """ + repo_obj = pygit2.Repository(self.git_path) + commit = repo_obj.revparse_single('HEAD') + + headers = {'Authorization': 'token aaabbbcccddd'} + data = { + 'username': 'Jenkins', + 'percent': 100, + 'comment': 'Tests passed', + 'uid': 'jenkins_build_pagure_100+seed', + } + output = self.app.post( + '/api/0/test/c/%s/flag' % commit.oid.hex, + headers=headers, data=data) + self.assertEqual(output.status_code, 400) + data = json.loads(output.data) + expected_output = { + "error": "Invalid or incomplete input submited", + "error_code": "EINVALIDREQ", + "errors": { + "url": [ + "This field is required." + ] + } + } + self.assertEqual(data, expected_output) + + def test_flag_commit_invalid_token(self): + """ Test flagging a commit with missing info. """ + repo_obj = pygit2.Repository(self.git_path) + commit = repo_obj.revparse_single('HEAD') + + headers = {'Authorization': 'token 123'} + data = { + 'username': 'Jenkins', + 'percent': 100, + 'comment': 'Tests passed', + 'url': 'http://jenkins.cloud.fedoraproject.org/', + 'uid': 'jenkins_build_pagure_100+seed', + } + output = self.app.post( + '/api/0/test/c/%s/flag' % commit.oid.hex, + headers=headers, data=data) + self.assertEqual(output.status_code, 401) + data = json.loads(output.data) + expected_output = { + "error": "Invalid or expired token. Please visit " + "https://pagure.org/ to get or renew your API token.", + "error_code": "EINVALIDTOK" + } + self.assertEqual(data, expected_output) + + def test_flag_commit_with_uid(self): + """ Test flagging a commit with provided uid. """ + repo_obj = pygit2.Repository(self.git_path) + commit = repo_obj.revparse_single('HEAD') + + headers = {'Authorization': 'token aaabbbcccddd'} + data = { + 'username': 'Jenkins', + 'percent': 100, + 'comment': 'Tests passed', + 'url': 'http://jenkins.cloud.fedoraproject.org/', + 'uid': 'jenkins_build_pagure_100+seed', + } + output = self.app.post( + '/api/0/test/c/%s/flag' % commit.oid.hex, + headers=headers, data=data) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + data['flag']['date_created'] = u'1510742565' + data['flag']['commit_hash'] = u'62b49f00d489452994de5010565fab81' + expected_output = { + u'flag': { + u'comment': u'Tests passed', + u'commit_hash': u'62b49f00d489452994de5010565fab81', + u'date_created': u'1510742565', + u'percent': 100, + u'url': u'http://jenkins.cloud.fedoraproject.org/', + u'user': { + u'default_email': u'bar@pingou.com', + u'emails': [u'bar@pingou.com', u'foo@pingou.com'], + u'fullname': u'PY C', + u'name': u'pingou'}, + u'username': u'Jenkins' + }, + u'message': u'Flag added', + u'uid': u'jenkins_build_pagure_100+seed' + } + + self.assertEqual(data, expected_output) + + def test_flag_commit_without_uid(self): + """ Test flagging a commit with missing info. """ + repo_obj = pygit2.Repository(self.git_path) + commit = repo_obj.revparse_single('HEAD') + + headers = {'Authorization': 'token aaabbbcccddd'} + data = { + 'username': 'Jenkins', + 'percent': 100, + 'comment': 'Tests passed', + 'url': 'http://jenkins.cloud.fedoraproject.org/', + } + output = self.app.post( + '/api/0/test/c/%s/flag' % commit.oid.hex, + headers=headers, data=data) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + self.assertNotEqual( + data['uid'], + u'jenkins_build_pagure_100+seed' + ) + data['flag']['date_created'] = u'1510742565' + data['flag']['commit_hash'] = u'62b49f00d489452994de5010565fab81' + data['uid'] = 'b1de8f80defd4a81afe2e09f39678087' + expected_output = { + u'flag': { + u'comment': u'Tests passed', + u'commit_hash': u'62b49f00d489452994de5010565fab81', + u'date_created': u'1510742565', + u'percent': 100, + u'url': u'http://jenkins.cloud.fedoraproject.org/', + u'user': { + u'default_email': u'bar@pingou.com', + u'emails': [u'bar@pingou.com', u'foo@pingou.com'], + u'fullname': u'PY C', + u'name': u'pingou'}, + u'username': u'Jenkins' + }, + u'message': u'Flag added', + u'uid': u'b1de8f80defd4a81afe2e09f39678087' + } + self.assertEqual(data, expected_output) + + if __name__ == '__main__': unittest.main(verbosity=2) diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py index 9f17980..0247854 100644 --- a/tests/test_pagure_lib.py +++ b/tests/test_pagure_lib.py @@ -5482,6 +5482,7 @@ foo bar self.assertEqual( [a.name for a in acls], [ + 'commit_flag', 'create_project', 'fork_project', 'generate_acls_project', From 1ffce3a2071f59fea451f05ce84cc2035d4fdbdb Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 22 2017 16:44:48 +0000 Subject: [PATCH 3/8] Add a small documentation blob about flags for commits and PRs Signed-off-by: Pierre-Yves Chibon --- diff --git a/doc/usage/_static/pagure_commit_flag.png b/doc/usage/_static/pagure_commit_flag.png new file mode 100644 index 0000000..cce6b5f Binary files /dev/null and b/doc/usage/_static/pagure_commit_flag.png differ diff --git a/doc/usage/_static/pagure_flag_pr.png b/doc/usage/_static/pagure_flag_pr.png new file mode 100644 index 0000000..9d1205b Binary files /dev/null and b/doc/usage/_static/pagure_flag_pr.png differ diff --git a/doc/usage/flags.rst b/doc/usage/flags.rst new file mode 100644 index 0000000..10e3b30 --- /dev/null +++ b/doc/usage/flags.rst @@ -0,0 +1,38 @@ + +.. _flags: + +Flags +===== + +Pagure offers the possibility to flag pull-requests and commits. A flag +is a way for a third-party tool to provide feedback on a pull-request or a +commit. + +This feedback can be as simple as the outcome of running the tests, or some +lint tool, or test coverage evolution. + + +Add a flag +---------- + +Flags can be set via the API, see the ``/api/`` url in your pagure instance +or at `pagure.io/api/ `_ and look for the endpoints +with the titles: ``Flag a commit`` or ``Flag a pull-request``. + + +.. _example_flag_commit: + +Example of two flags on a commit: +--------------------------------- + +.. image:: _static/pagure_commit_flag.png + :target: ../_images/pagure_commit_flag.png + + +.. _example_flag_pr: + +Example of two flags on a pull-request: +--------------------------------------- + +.. image:: _static/pagure_flag_pr.png + :target: ../_images/pagure_flag_pr.png diff --git a/doc/usage/index.rst b/doc/usage/index.rst index e8f2f8a..681eb2c 100644 --- a/doc/usage/index.rst +++ b/doc/usage/index.rst @@ -40,6 +40,7 @@ Contents: project_settings project_acls roadmap + flags using_doc using_webhooks ticket_templates From abcbf77ee5fc52b19691733804999732392855cc Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 22 2017 16:45:23 +0000 Subject: [PATCH 4/8] Add a status field to flags This status is used in addition to the percentage making it optional. For flags on commit the status is mandatory, on pull-requests it is optional and will be inferred from the percentage specified (or not). This keeps the backward compatibility on the API endpoint to flag PR while enforcing the new behavior for the new API endpoint for commits. Signed-off-by: Pierre-Yves Chibon --- diff --git a/alembic/versions/2b626a16542e_commit_flag.py b/alembic/versions/2b626a16542e_commit_flag.py index 503cff0..20ab714 100644 --- a/alembic/versions/2b626a16542e_commit_flag.py +++ b/alembic/versions/2b626a16542e_commit_flag.py @@ -38,8 +38,9 @@ def upgrade(): 'user_id', sa.Integer, sa.ForeignKey('users.id', onupdate='CASCADE'), nullable=False, index=True), + sa.Column('status', sa.String(32), nullable=False), sa.Column('username', sa.Text(), nullable=False), - sa.Column('percent', sa.Integer(), nullable=False), + sa.Column('percent', sa.Integer(), nullable=True), sa.Column('comment', sa.Text(), nullable=False), sa.Column('url', sa.Text(), nullable=False), sa.Column( diff --git a/alembic/versions/6119fbbcc8e9_migrate_current_flag.py b/alembic/versions/6119fbbcc8e9_migrate_current_flag.py new file mode 100644 index 0000000..8274738 --- /dev/null +++ b/alembic/versions/6119fbbcc8e9_migrate_current_flag.py @@ -0,0 +1,42 @@ +"""Migrate current flag + +Revision ID: 6119fbbcc8e9 +Revises: 2b626a16542e +Create Date: 2017-11-16 15:11:28.199971 + +""" + +# revision identifiers, used by Alembic. +revision = '6119fbbcc8e9' +down_revision = '2b626a16542e' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + """ Add the status column to pull_request_flags and migrate the data. + """ + op.add_column( + 'pull_request_flags', + sa.Column('status', sa.String(32), nullable=True) + ) + op.execute( + 'UPDATE pull_request_flags SET status=\'success\' ' + 'WHERE percent in (100, \'100\')') + op.execute( + 'UPDATE pull_request_flags SET status=\'failure\' ' + 'WHERE percent not in (100, \'100\')') + op.alter_column( + 'pull_request_flags', 'status', + nullable=False, existing_nullable=True) + + +def downgrade(): + """ Drop the status column in pull_request_flags. + + We can't undo the change to the status column since it may now + contain empty rows. + + """ + op.drop_column('pull_request_flags', 'status') diff --git a/pagure/api/fork.py b/pagure/api/fork.py index 89d495f..332f1e0 100644 --- a/pagure/api/fork.py +++ b/pagure/api/fork.py @@ -611,14 +611,6 @@ def api_pull_request_add_flag(repo, requestid, username=None, namespace=None): | | | | presented to users | | | | | on the pull request page | +---------------+---------+--------------+-----------------------------+ - | ``percent`` | int | Mandatory | | A percentage of | - | | | | completion compared to | - | | | | the goal. The percentage | - | | | | also determine the | - | | | | background color of the | - | | | | flag on the pull-request | - | | | | page | - +---------------+---------+--------------+-----------------------------+ | ``comment`` | string | Mandatory | | A short message | | | | | summarizing the | | | | | presented results | @@ -626,6 +618,25 @@ def api_pull_request_add_flag(repo, requestid, username=None, namespace=None): | ``url`` | string | Mandatory | | A URL to the result | | | | | of this flag | +---------------+---------+--------------+-----------------------------+ + | ``status`` | string | Optional | | The status of the task, | + | | | | can be any of: success, | + | | | | failure, error, pending, | + | | | | canceled | + | | | | If not provided it will be| + | | | | set to ``success`` if | + | | | | percent is higher than 0, | + | | | | ``failure`` if it is 0 and| + | | | | ``pending`` if percent is | + | | | | not specified | + +---------------+---------+--------------+-----------------------------+ + | ``percent`` | int | Optional | | A percentage of | + | | | | completion compared to | + | | | | the goal. The percentage | + | | | | also determine the | + | | | | background color of the | + | | | | flag on the pull-request | + | | | | page | + +---------------+---------+--------------+-----------------------------+ | ``uid`` | string | Optional | | A unique identifier used | | | | | to identify a flag on a | | | | | pull-request. If the | @@ -638,9 +649,6 @@ def api_pull_request_add_flag(repo, requestid, username=None, namespace=None): | | | | characters. Default: an | | | | | auto generated UID | +---------------+---------+--------------+-----------------------------+ - | ``commit`` | string | Optional | | The hash of the commit | - | | | | you use | - +---------------+---------+--------------+-----------------------------+ Sample response ^^^^^^^^^^^^^^^ @@ -653,6 +661,7 @@ def api_pull_request_add_flag(repo, requestid, username=None, namespace=None): "date_created": "1510742565", "percent": 0, "pull_request_uid": "62b49f00d489452994de5010565fab81", + "status": "error", "url": "http://jenkins.cloud.fedoraproject.org/", "user": { "default_email": "bar@pingou.com", @@ -672,6 +681,7 @@ def api_pull_request_add_flag(repo, requestid, username=None, namespace=None): "date_created": "1510742565", "percent": 0, "pull_request_uid": "62b49f00d489452994de5010565fab81", + "status": "error", "url": "http://jenkins.cloud.fedoraproject.org/", "user": { "default_email": "bar@pingou.com", @@ -707,19 +717,30 @@ def api_pull_request_add_flag(repo, requestid, username=None, namespace=None): if not request: raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOREQ) - form = pagure.forms.AddPullRequestFlagForm(csrf_enabled=False) + if 'status' in flask.request.form: + form = pagure.forms.AddPullRequestFlagForm(csrf_enabled=False) + else: + form = pagure.forms.AddPullRequestFlagFormV1(csrf_enabled=False) if form.validate_on_submit(): username = form.username.data - percent = form.percent.data + percent = form.percent.data.strip() or None comment = form.comment.data.strip() url = form.url.data.strip() uid = form.uid.data.strip() if form.uid.data else None + if 'status' in flask.request.form: + status = form.status.data.strip() + else: + if percent is None: + status = 'pending' + else: + status = 'success' if percent != '0' else 'failure' try: # New Flag message, uid = pagure.lib.add_pull_request_flag( SESSION, request=request, username=username, + status=status, percent=percent, comment=comment, url=url, diff --git a/pagure/api/project.py b/pagure/api/project.py index 0346c69..f527a75 100644 --- a/pagure/api/project.py +++ b/pagure/api/project.py @@ -1303,15 +1303,7 @@ def api_commit_add_flag(repo, commit_hash, username=None, namespace=None): | ``username`` | string | Mandatory | | The name of the | | | | | application to be | | | | | presented to users | - | | | | on the pull request page | - +---------------+---------+--------------+-----------------------------+ - | ``percent`` | int | Mandatory | | A percentage of | - | | | | completion compared to | - | | | | the goal. The percentage | - | | | | also determine the | - | | | | background color of the | - | | | | flag on the pull-request | - | | | | page | + | | | | on the commit pages | +---------------+---------+--------------+-----------------------------+ | ``comment`` | string | Mandatory | | A short message | | | | | summarizing the | @@ -1320,9 +1312,21 @@ def api_commit_add_flag(repo, commit_hash, username=None, namespace=None): | ``url`` | string | Mandatory | | A URL to the result | | | | | of this flag | +---------------+---------+--------------+-----------------------------+ + | ``status`` | string | Mandatory | | The status of the task, | + | | | | can be any of: success, | + | | | | failure, error, pending, | + | | | | canceled | + +---------------+---------+--------------+-----------------------------+ + | ``percent`` | int | Optional | | A percentage of | + | | | | completion compared to | + | | | | the goal. The percentage | + | | | | also determine the | + | | | | background color of the | + | | | | flag on the pages | + +---------------+---------+--------------+-----------------------------+ | ``uid`` | string | Optional | | A unique identifier used | - | | | | to identify a flag on a | - | | | | pull-request. If the | + | | | | to identify a flag across | + | | | | all projects. If the | | | | | provided UID matches an | | | | | existing one, then the | | | | | API call will update the | @@ -1345,6 +1349,7 @@ def api_commit_add_flag(repo, commit_hash, username=None, namespace=None): "commit_hash": "62b49f00d489452994de5010565fab81", "date_created": "1510742565", "percent": 100, + "status": "success", "url": "http://jenkins.cloud.fedoraproject.org/", "user": { "default_email": "bar@pingou.com", @@ -1365,6 +1370,7 @@ def api_commit_add_flag(repo, commit_hash, username=None, namespace=None): "commit_hash": "62b49f00d489452994de5010565fab81", "date_created": "1510742565", "percent": 100, + "status": "success", "url": "http://jenkins.cloud.fedoraproject.org/", "user": { "default_email": "bar@pingou.com", @@ -1403,10 +1409,11 @@ def api_commit_add_flag(repo, commit_hash, username=None, namespace=None): form = pagure.forms.AddPullRequestFlagForm(csrf_enabled=False) if form.validate_on_submit(): username = form.username.data - percent = form.percent.data + percent = form.percent.data.strip() or None comment = form.comment.data.strip() url = form.url.data.strip() uid = form.uid.data.strip() if form.uid.data else None + status = form.status.data.strip() try: # New Flag message, uid = pagure.lib.add_commit_flag( @@ -1416,6 +1423,7 @@ def api_commit_add_flag(repo, commit_hash, username=None, namespace=None): username=username, percent=percent, comment=comment, + status=status, url=url, uid=uid, user=flask.g.fas_user.username, diff --git a/pagure/forms.py b/pagure/forms.py index c19e735..3f7fd3e 100644 --- a/pagure/forms.py +++ b/pagure/forms.py @@ -471,12 +471,13 @@ class AddPullRequestCommentForm(PagureForm): ) -class AddPullRequestFlagForm(PagureForm): - ''' Form to add a flag to a pull-request. ''' +class AddPullRequestFlagFormV1(PagureForm): + ''' Form to add a flag to a pull-request or commit. ''' username = wtforms.TextField( 'Username', [wtforms.validators.Required()]) percent = wtforms.TextField( - 'Percentage of completion', [wtforms.validators.Required()]) + 'Percentage of completion', + [wtforms.validators.optional()]) comment = wtforms.TextAreaField( 'Comment', [wtforms.validators.Required()]) url = wtforms.TextField( @@ -485,6 +486,21 @@ class AddPullRequestFlagForm(PagureForm): 'UID', [wtforms.validators.optional()]) +class AddPullRequestFlagForm(AddPullRequestFlagFormV1): + ''' Form to add a flag to a pull-request or commit. ''' + status = wtforms.SelectField( + 'status', + [wtforms.validators.Required()], + choices=[ + ('success', 'success'), + ('failure', 'failure'), + ('error', 'error'), + ('canceled', 'canceled'), + ('pending', 'pending'), + ], + ) + + class UserSettingsForm(PagureForm): ''' Form to create or edit project. ''' ssh_key = wtforms.TextAreaField( diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 54ef5f8..703d313 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -1276,7 +1276,7 @@ def edit_comment(session, parent, comment, user, def add_pull_request_flag(session, request, username, percent, comment, url, - uid, user, token, requestfolder): + status, uid, user, token, requestfolder): ''' Add a flag to a pull-request. ''' user_obj = get_user(session, user) @@ -1285,6 +1285,7 @@ def add_pull_request_flag(session, request, username, percent, comment, url, if pr_flag: action = 'updated' pr_flag.comment = comment + pr_flag.status = status pr_flag.percent = percent pr_flag.url = url else: @@ -1294,6 +1295,7 @@ def add_pull_request_flag(session, request, username, percent, comment, url, username=username, percent=percent, comment=comment, + status=status, url=url, user_id=user_obj.id, token_id=token, @@ -1331,6 +1333,7 @@ def add_commit_flag( action = 'updated' c_flag.comment = comment c_flag.percent = percent + c_flag.status = status c_flag.url = url else: c_flag = model.CommitFlag( @@ -1338,6 +1341,7 @@ def add_commit_flag( project_id=repo.id, commit_hash=commit_hash, username=username, + status=status, percent=percent, comment=comment, url=url, diff --git a/pagure/lib/model.py b/pagure/lib/model.py index e137112..59aafc5 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -1959,6 +1959,9 @@ class PullRequestFlag(BASE): 'tokens.id', ), nullable=True) + status = sa.Column( + sa.String(32), + nullable=False) user_id = sa.Column( sa.Integer, sa.ForeignKey( @@ -1971,7 +1974,7 @@ class PullRequestFlag(BASE): nullable=False) percent = sa.Column( sa.Integer(), - nullable=False) + nullable=True) comment = sa.Column( sa.Text(), nullable=False) @@ -2005,6 +2008,7 @@ class PullRequestFlag(BASE): 'username': self.username, 'percent': self.percent, 'comment': self.comment, + 'status': self.status, 'url': self.url, 'date_created': self.date_created.strftime('%s'), 'user': self.user.to_json(public=public), @@ -2042,12 +2046,15 @@ class CommitFlag(BASE): nullable=False, index=True) uid = sa.Column(sa.String(32), unique=True, nullable=False) + status = sa.Column( + sa.String(32), + nullable=False) username = sa.Column( sa.Text(), nullable=False) percent = sa.Column( sa.Integer(), - nullable=False) + nullable=True) comment = sa.Column( sa.Text(), nullable=False) @@ -2073,6 +2080,7 @@ class CommitFlag(BASE): 'username': self.username, 'percent': self.percent, 'comment': self.comment, + 'status': self.status, 'url': self.url, 'date_created': self.date_created.strftime('%s'), 'user': self.user.to_json(public=public), diff --git a/tests/test_pagure_flask_api_fork.py b/tests/test_pagure_flask_api_fork.py index 8e52ff9..5bdda48 100644 --- a/tests/test_pagure_flask_api_fork.py +++ b/tests/test_pagure_flask_api_fork.py @@ -1055,8 +1055,7 @@ class PagureFlaskApiForktests(tests.Modeltests): data = { 'username': 'Jenkins', - 'percent': 0, - 'comment': 'Tests failed', + 'comment': 'Tests running', 'url': 'http://jenkins.cloud.fedoraproject.org/', 'uid': 'jenkins_build_pagure_100+seed', } @@ -1072,10 +1071,11 @@ class PagureFlaskApiForktests(tests.Modeltests): data, { u'flag': { - u'comment': u'Tests failed', + u'comment': u'Tests running', u'date_created': u'1510742565', - u'percent': 0, + u'percent': None, u'pull_request_uid': u'62b49f00d489452994de5010565fab81', + u'status': u'pending', u'url': u'http://jenkins.cloud.fedoraproject.org/', u'user': { u'default_email': u'bar@pingou.com', @@ -1092,10 +1092,10 @@ class PagureFlaskApiForktests(tests.Modeltests): request = pagure.lib.search_pull_requests( self.session, project_id=1, requestid=1) self.assertEqual(len(request.flags), 1) - self.assertEqual(request.flags[0].comment, 'Tests failed') - self.assertEqual(request.flags[0].percent, 0) + self.assertEqual(request.flags[0].comment, 'Tests running') + self.assertEqual(request.flags[0].percent, None) - # Update flag + # Update flag - w/o providing the status data = { 'username': 'Jenkins', 'percent': 100, @@ -1118,6 +1118,7 @@ class PagureFlaskApiForktests(tests.Modeltests): u'date_created': u'1510742565', u'percent': 100, u'pull_request_uid': u'62b49f00d489452994de5010565fab81', + u'status': u'success', u'url': u'http://jenkins.cloud.fedoraproject.org/', u'user': { u'default_email': u'bar@pingou.com', @@ -1243,7 +1244,7 @@ class PagureFlaskApiForktests(tests.Modeltests): 'uid': 'jenkins_build_pagure_100+seed', } - # Valid request + # Valid request - w/o providing the status output = self.app.post( '/api/0/test/pull-request/1/flag', data=data, headers=headers) self.assertEqual(output.status_code, 200) @@ -1258,6 +1259,7 @@ class PagureFlaskApiForktests(tests.Modeltests): u'date_created': u'1510742565', u'percent': 0, u'pull_request_uid': u'62b49f00d489452994de5010565fab81', + u'status': u'failure', u'url': u'http://jenkins.cloud.fedoraproject.org/', u'user': { u'default_email': u'bar@pingou.com', @@ -1284,6 +1286,7 @@ class PagureFlaskApiForktests(tests.Modeltests): 'comment': 'Tests passed', 'url': 'http://jenkins.cloud.fedoraproject.org/', 'uid': 'jenkins_build_pagure_100+seed', + 'status': 'success', } output = self.app.post( @@ -1300,6 +1303,7 @@ class PagureFlaskApiForktests(tests.Modeltests): u'date_created': u'1510742565', u'percent': 100, u'pull_request_uid': u'62b49f00d489452994de5010565fab81', + u'status': u'success', u'url': u'http://jenkins.cloud.fedoraproject.org/', u'user': { u'default_email': u'bar@pingou.com', diff --git a/tests/test_pagure_flask_api_project.py b/tests/test_pagure_flask_api_project.py index 9643dcd..ab7fe80 100644 --- a/tests/test_pagure_flask_api_project.py +++ b/tests/test_pagure_flask_api_project.py @@ -2737,7 +2737,7 @@ class PagureFlaskApiProjectFlagtests(tests.Modeltests): tests.create_tokens_acl( self.session, 'aaabbbcccddd', 'commit_flag') - def test_flag_commit_missing_percent(self): + def test_flag_commit_missing_status(self): """ Test flagging a commit with missing precentage. """ repo_obj = pygit2.Repository(self.git_path) commit = repo_obj.revparse_single('HEAD') @@ -2758,8 +2758,8 @@ class PagureFlaskApiProjectFlagtests(tests.Modeltests): "error": "Invalid or incomplete input submited", "error_code": "EINVALIDREQ", "errors": { - "percent": [ - "This field is required." + "status": [ + "Not a valid choice" ] } } @@ -2776,6 +2776,7 @@ class PagureFlaskApiProjectFlagtests(tests.Modeltests): 'comment': 'Tests passed', 'url': 'http://jenkins.cloud.fedoraproject.org/', 'uid': 'jenkins_build_pagure_100+seed', + 'status': 'success', } output = self.app.post( '/api/0/test/c/%s/flag' % commit.oid.hex, @@ -2804,6 +2805,7 @@ class PagureFlaskApiProjectFlagtests(tests.Modeltests): 'percent': 100, 'url': 'http://jenkins.cloud.fedoraproject.org/', 'uid': 'jenkins_build_pagure_100+seed', + 'status': 'success', } output = self.app.post( '/api/0/test/c/%s/flag' % commit.oid.hex, @@ -2832,6 +2834,7 @@ class PagureFlaskApiProjectFlagtests(tests.Modeltests): 'percent': 100, 'comment': 'Tests passed', 'uid': 'jenkins_build_pagure_100+seed', + 'status': 'success', } output = self.app.post( '/api/0/test/c/%s/flag' % commit.oid.hex, @@ -2874,6 +2877,33 @@ class PagureFlaskApiProjectFlagtests(tests.Modeltests): } self.assertEqual(data, expected_output) + def test_flag_commit_invalid_status(self): + """ Test flagging a commit with an invalid status. """ + repo_obj = pygit2.Repository(self.git_path) + commit = repo_obj.revparse_single('HEAD') + + headers = {'Authorization': 'token aaabbbcccddd'} + data = { + 'username': 'Jenkins', + 'percent': 100, + 'comment': 'Tests passed', + 'url': 'http://jenkins.cloud.fedoraproject.org/', + 'status': 'foobar', + } + output = self.app.post( + '/api/0/test/c/%s/flag' % commit.oid.hex, + headers=headers, data=data) + self.assertEqual(output.status_code, 400) + data = json.loads(output.data) + self.assertEqual( + data, + { + u'errors': {u'status': [u'Not a valid choice']}, + u'error_code': u'EINVALIDREQ', + u'error': u'Invalid or incomplete input submited' + } + ) + def test_flag_commit_with_uid(self): """ Test flagging a commit with provided uid. """ repo_obj = pygit2.Repository(self.git_path) @@ -2886,6 +2916,7 @@ class PagureFlaskApiProjectFlagtests(tests.Modeltests): 'comment': 'Tests passed', 'url': 'http://jenkins.cloud.fedoraproject.org/', 'uid': 'jenkins_build_pagure_100+seed', + 'status': 'success', } output = self.app.post( '/api/0/test/c/%s/flag' % commit.oid.hex, @@ -2900,6 +2931,7 @@ class PagureFlaskApiProjectFlagtests(tests.Modeltests): u'commit_hash': u'62b49f00d489452994de5010565fab81', u'date_created': u'1510742565', u'percent': 100, + u'status': 'success', u'url': u'http://jenkins.cloud.fedoraproject.org/', u'user': { u'default_email': u'bar@pingou.com', @@ -2925,6 +2957,7 @@ class PagureFlaskApiProjectFlagtests(tests.Modeltests): 'percent': 100, 'comment': 'Tests passed', 'url': 'http://jenkins.cloud.fedoraproject.org/', + 'status': 'success', } output = self.app.post( '/api/0/test/c/%s/flag' % commit.oid.hex, @@ -2944,6 +2977,7 @@ class PagureFlaskApiProjectFlagtests(tests.Modeltests): u'commit_hash': u'62b49f00d489452994de5010565fab81', u'date_created': u'1510742565', u'percent': 100, + u'status': 'success', u'url': u'http://jenkins.cloud.fedoraproject.org/', u'user': { u'default_email': u'bar@pingou.com', From 4437424d1d68c0cd5661f9cc99a3f52fd68f2cf0 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 22 2017 16:45:23 +0000 Subject: [PATCH 5/8] Expand the documentation about flags Signed-off-by: Pierre-Yves Chibon --- diff --git a/doc/usage/flags.rst b/doc/usage/flags.rst index 10e3b30..8304706 100644 --- a/doc/usage/flags.rst +++ b/doc/usage/flags.rst @@ -20,6 +20,40 @@ or at `pagure.io/api/ `_ and look for the endpoints with the titles: ``Flag a commit`` or ``Flag a pull-request``. +- **uid**: the API endpoints to add flag have an optional UID argument. It + is a unique identifier (of maximum 32 characters) that is unique for the + entire pagure instance and allows to edit a flag. + If it is not specified by the user/tool adding the flag, it will be + automatically generated and in either case, will be returned in the JSON + data returned by the API endpoints. Note that this is the only time you + would be able to retrieve this identifier if you do not specify it + yourself. + +- **status**: this field indicates the status of the task in the system + running it. Pagure supports the following statuses: + + - ``success``: the task ended successfully. + - ``canceled``: the task was canceled. + - ``failure``: the task ended but failed. + - ``error``: the task did not end at all. + - ``pending``: the results of this task are pending. + (for ``failure`` vs ``error`` think of the test system ran the tests but + they failed vs the test system did not get to run the tests) + +- **percent**: this is an optional field which allows to provide some more + details about the outcome of the task. For example this could be used for + test coverage, or the number of test that failed/passed. + +- **username**: the name of the system running the tests. While not being + restricted in length, a shorter name will render better in the interface. + +- **comment**: a free text form not restricted in length (however, here as + well if the comment is too long it may render off in the interface). + +- **url**: the url the flag is linked to and where the user should be able + to retrieve more information about the task and its outcome. + + .. _example_flag_commit: Example of two flags on a commit: From dc4c195ceb1b399676a0dca5da9e72914bea1241 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 22 2017 16:45:23 +0000 Subject: [PATCH 6/8] Make flag's UID be unique to the commit/PR being flagged Signed-off-by: Pierre-Yves Chibon --- diff --git a/alembic/versions/01e58ee9eccb_adjust_constraints_on_pull_request_flags.py b/alembic/versions/01e58ee9eccb_adjust_constraints_on_pull_request_flags.py new file mode 100644 index 0000000..30d7dc9 --- /dev/null +++ b/alembic/versions/01e58ee9eccb_adjust_constraints_on_pull_request_flags.py @@ -0,0 +1,42 @@ +"""Adjust constraints on pull_request_flags + +Revision ID: 01e58ee9eccb +Revises: 6119fbbcc8e9 +Create Date: 2017-11-16 16:50:47.278252 + +""" + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = '01e58ee9eccb' +down_revision = '6119fbbcc8e9' + + +def upgrade(): + """ Remove the unique constraints on UID in pull_request_flags and make + it a composite unique constraint on UID + pull_request_uid. + """ + # alter the constraints + op.drop_constraint('pull_request_flags_uid_key', 'pull_request_flags') + op.create_unique_constraint( + "pull_request_flags_uid_pull_request_uid_key", + 'pull_request_flags', + ["uid", "pull_request_uid"] + ) + + +def downgrade(): + """ Remove the composite unique constraints on UID + pull_request_uid + in pull_request_flags and make it an unique constraint on UID . + """ + op.drop_constraint( + 'pull_request_flags_uid_pull_request_uid_key', + 'pull_request_flags') + op.create_unique_constraint( + "pull_request_flags_uid_key", + 'pull_request_flags', + ["uid"] + ) diff --git a/alembic/versions/2b626a16542e_commit_flag.py b/alembic/versions/2b626a16542e_commit_flag.py index 20ab714..ccdbc22 100644 --- a/alembic/versions/2b626a16542e_commit_flag.py +++ b/alembic/versions/2b626a16542e_commit_flag.py @@ -48,6 +48,12 @@ def upgrade(): default=datetime.datetime.utcnow), ) + op.create_unique_constraint( + "commit_flags_uid_commit_hash_key", + 'commit_flags', + ["uid", "commit_hash"] + ) + def downgrade(): ''' Drop the commit_flags table. ''' diff --git a/doc/usage/flags.rst b/doc/usage/flags.rst index 8304706..fc0441c 100644 --- a/doc/usage/flags.rst +++ b/doc/usage/flags.rst @@ -21,8 +21,8 @@ with the titles: ``Flag a commit`` or ``Flag a pull-request``. - **uid**: the API endpoints to add flag have an optional UID argument. It - is a unique identifier (of maximum 32 characters) that is unique for the - entire pagure instance and allows to edit a flag. + is a unique identifier (of maximum 32 characters) that is unique the commit + or pull-request that is being/has been flagged. If it is not specified by the user/tool adding the flag, it will be automatically generated and in either case, will be returned in the JSON data returned by the API endpoints. Note that this is the only time you diff --git a/pagure/api/fork.py b/pagure/api/fork.py index 332f1e0..13e1aae 100644 --- a/pagure/api/fork.py +++ b/pagure/api/fork.py @@ -750,7 +750,8 @@ def api_pull_request_add_flag(repo, requestid, username=None, namespace=None): requestfolder=APP.config['REQUESTS_FOLDER'], ) SESSION.commit() - pr_flag = pagure.lib.get_pull_request_flag_by_uid(SESSION, uid) + pr_flag = pagure.lib.get_pull_request_flag_by_uid( + SESSION, request, uid) output['message'] = message output['uid'] = uid output['flag'] = pr_flag.to_json() diff --git a/pagure/api/project.py b/pagure/api/project.py index f527a75..924dbf4 100644 --- a/pagure/api/project.py +++ b/pagure/api/project.py @@ -1430,7 +1430,8 @@ def api_commit_add_flag(repo, commit_hash, username=None, namespace=None): token=flask.g.token.id, ) SESSION.commit() - c_flag = pagure.lib.get_commit_flag_by_uid(SESSION, uid) + c_flag = pagure.lib.get_commit_flag_by_uid( + SESSION, commit_hash, uid) output['message'] = message output['uid'] = uid output['flag'] = c_flag.to_json() diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 703d313..3588415 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -1281,7 +1281,7 @@ def add_pull_request_flag(session, request, username, percent, comment, url, user_obj = get_user(session, user) action = 'added' - pr_flag = get_pull_request_flag_by_uid(session, uid) + pr_flag = get_pull_request_flag_by_uid(session, request, uid) if pr_flag: action = 'updated' pr_flag.comment = comment @@ -1328,7 +1328,7 @@ def add_commit_flag( user_obj = get_user(session, user) action = 'added' - c_flag = get_commit_flag_by_uid(session, uid) + c_flag = get_commit_flag_by_uid(session, commit_hash, uid) if c_flag: action = 'updated' c_flag.comment = comment @@ -2859,10 +2859,11 @@ def get_request_by_uid(session, request_uid): return query.first() -def get_pull_request_flag_by_uid(session, flag_uid): +def get_pull_request_flag_by_uid(session, request, flag_uid): ''' Return the flag corresponding to the specified unique identifier. :arg session: the session to use to connect to the database. + :arg request: the pull-request that was flagged :arg flag_uid: the unique identifier of a request. This identifier is unique accross all flags on this pagure instance and should be unique accross multiple pagure instances as well @@ -2875,15 +2876,18 @@ def get_pull_request_flag_by_uid(session, flag_uid): query = session.query( model.PullRequestFlag ).filter( - model.PullRequestFlag.uid == flag_uid.strip() if flag_uid else None + model.PullRequestFlag.pull_request_uid == request.uid + ).filter( + model.PullRequestFlag.uid == flag_uid.strip() ) return query.first() -def get_commit_flag_by_uid(session, flag_uid): +def get_commit_flag_by_uid(session, commit_hash, flag_uid): ''' Return the flag corresponding to the specified unique identifier. :arg session: the session to use to connect to the database. + :arg commit_hash: the hash of the commit that got flagged :arg flag_uid: the unique identifier of a request. This identifier is unique accross all flags on this pagure instance and should be unique accross multiple pagure instances as well @@ -2896,6 +2900,8 @@ def get_commit_flag_by_uid(session, flag_uid): query = session.query( model.CommitFlag ).filter( + model.CommitFlag.commit_hash == commit_hash + ).filter( model.CommitFlag.uid == flag_uid.strip() if flag_uid else None ) return query.first() diff --git a/pagure/lib/model.py b/pagure/lib/model.py index 59aafc5..807a52a 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -1947,7 +1947,7 @@ class PullRequestFlag(BASE): __tablename__ = 'pull_request_flags' id = sa.Column(sa.Integer, primary_key=True) - uid = sa.Column(sa.String(32), unique=True, nullable=False) + uid = sa.Column(sa.String(32), nullable=False) pull_request_uid = sa.Column( sa.String(32), sa.ForeignKey( @@ -1985,6 +1985,8 @@ class PullRequestFlag(BASE): date_created = sa.Column(sa.DateTime, nullable=False, default=datetime.datetime.utcnow) + __table_args__ = (sa.UniqueConstraint('uid', 'pull_request_uid'),) + user = relation('User', foreign_keys=[user_id], remote_side=[User.id], backref=backref( @@ -2045,7 +2047,7 @@ class CommitFlag(BASE): ), nullable=False, index=True) - uid = sa.Column(sa.String(32), unique=True, nullable=False) + uid = sa.Column(sa.String(32), nullable=False) status = sa.Column( sa.String(32), nullable=False) @@ -2065,6 +2067,8 @@ class CommitFlag(BASE): date_created = sa.Column(sa.DateTime, nullable=False, default=datetime.datetime.utcnow) + __table_args__ = (sa.UniqueConstraint('commit_hash', 'uid'),) + user = relation('User', foreign_keys=[user_id], remote_side=[User.id], backref=backref( From a2e0304326349eee4ade34509340aad7fcbe936d Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 22 2017 16:45:23 +0000 Subject: [PATCH 7/8] Fix unit-tests Signed-off-by: Pierre-Yves Chibon --- diff --git a/tests/test_pagure_flask_api_ui_private_repo.py b/tests/test_pagure_flask_api_ui_private_repo.py index 0346a74..f5ad6c9 100644 --- a/tests/test_pagure_flask_api_ui_private_repo.py +++ b/tests/test_pagure_flask_api_ui_private_repo.py @@ -1663,6 +1663,7 @@ class PagurePrivateRepotest(tests.Modeltests): u'date_created': u'1510742565', u'percent': 0, u'pull_request_uid': u'62b49f00d489452994de5010565fab81', + u'status': u'failure', u'url': u'http://jenkins.cloud.fedoraproject.org/', u'user': { u'default_email': u'bar@pingou.com', @@ -1705,6 +1706,7 @@ class PagurePrivateRepotest(tests.Modeltests): u'date_created': u'1510742565', u'percent': 100, u'pull_request_uid': u'62b49f00d489452994de5010565fab81', + u'status': u'success', u'url': u'http://jenkins.cloud.fedoraproject.org/', u'user': { u'default_email': u'bar@pingou.com', diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py index 0247854..57409b5 100644 --- a/tests/test_pagure_lib.py +++ b/tests/test_pagure_lib.py @@ -2595,6 +2595,7 @@ class PagureLibtests(tests.Modeltests): username="jenkins", percent=100, comment="Build passes", + status='success', url="http://jenkins.cloud.fedoraproject.org", uid="jenkins_build_pagure_34", user='foo', From 066d895fdad142fb87a27f179c8bfb8e98ec93b2 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 22 2017 16:45:23 +0000 Subject: [PATCH 8/8] Fix multiple instance of the same typo: dictionnary -> dictionary Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/lib/model.py b/pagure/lib/model.py index 807a52a..cb78982 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -272,7 +272,7 @@ class User(BASE): return 'User: %s - name %s' % (self.id, self.user) def to_json(self, public=False): - ''' Return a representation of the User in a dictionnary. ''' + ''' Return a representation of the User in a dictionary. ''' output = { 'name': self.user, 'fullname': self.fullname, @@ -1792,7 +1792,7 @@ class PullRequest(BASE): if not comment.notification] def to_json(self, public=False, api=False, with_comments=True): - ''' Returns a dictionnary representation of the pull-request. + ''' Returns a dictionary representation of the pull-request. ''' output = { @@ -2002,7 +2002,7 @@ class PullRequestFlag(BASE): remote_side=[PullRequest.uid]) def to_json(self, public=False): - ''' Returns a dictionnary representation of the pull-request. + ''' Returns a dictionary representation of the pull-request. ''' output = { @@ -2076,7 +2076,7 @@ class CommitFlag(BASE): order_by="CommitFlag.date_created")) def to_json(self, public=False): - ''' Returns a dictionnary representation of the commit flag. + ''' Returns a dictionary representation of the commit flag. ''' output = { @@ -2155,7 +2155,7 @@ class PagureGroup(BASE): return 'Group: %s - name %s' % (self.id, self.group_name) def to_json(self, public=False): - ''' Returns a dictionnary representation of the pull-request. + ''' Returns a dictionary representation of the pull-request. ''' output = {