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',