From 73001bc42ec9064e383d5bc0a71d23730629c148 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 04 2016 17:47:41 +0000 Subject: [PATCH 1/7] Add close_status to issues The list of status is a property of the project and can be used to specify why/how a ticket was closed. --- diff --git a/alembic/versions/644ef887bb6f_add_close_status.py b/alembic/versions/644ef887bb6f_add_close_status.py new file mode 100644 index 0000000..9a20a41 --- /dev/null +++ b/alembic/versions/644ef887bb6f_add_close_status.py @@ -0,0 +1,85 @@ +"""Add close status + +Revision ID: 644ef887bb6f +Revises: 368fd931cf7f +Create Date: 2016-10-04 15:38:41.908679 + +""" + +# revision identifiers, used by Alembic. +revision = '644ef887bb6f' +down_revision = '368fd931cf7f' + +from alembic import op +import sqlalchemy as sa + + +try: + from pagure.lib import model +except ImportError: + import sys + sys.path.insert(0, '.') + from pagure.lib import model + + +def upgrade(): + ''' Add the column _close_status to the table projects. + ''' + op.add_column( + 'projects', + sa.Column('_close_status', sa.Text, nullable=True) + ) + op.add_column( + 'issues', + sa.Column('close_status', sa.Text, nullable=True) + ) + + engine = op.get_bind() + Session = sa.orm.scoped_session(sa.orm.sessionmaker()) + Session.configure(bind=engine) + session = Session() + + # Update all the existing projects + statuses = ['Invalid', 'Insufficient data', 'Fixed', 'Duplicate'] + for project in session.query(model.Project).all(): + project.close_status = statuses + session.add(project) + session.commit() + + # Add the status 'Closed' for issues + ticket_stat = model.StatusIssue(status='Closed') + session.add(ticket_stat) + session.commit() + + # Remove the old status + op.execute('''DELETE FROM "status_issue" WHERE "status" NOT IN ('Open', 'Closed'); ''') + + # Set the close_status for all the closed tickets + op.execute('''UPDATE "issues" SET "close_status"=status where status != 'Open'; ''') + + # Mark all the tickets as closed + op.execute('''UPDATE "issues" SET status='Closed' where status != 'Open'; ''') + + +def downgrade(): + ''' Add the column _close_status to the table projects. + ''' + engine = op.get_bind() + Session = sa.orm.scoped_session(sa.orm.sessionmaker()) + Session.configure(bind=engine) + session = Session() + + statuses = ['Invalid', 'Insufficient data', 'Fixed', 'Duplicate'] + for status in statuses: + ticket_stat = model.StatusIssue(status=status) + session.add(ticket_stat) + session.commit() + + # Set the close_status for all the closed tickets + op.execute('''UPDATE "issues" SET status=close_status where status != 'Open'; ''') + + # Remove the old status + op.execute('''DELETE FROM "status_issue" WHERE status = 'Closed'; ''') + + op.drop_column('projects', '_close_status') + op.drop_column('issues', 'close_status') diff --git a/pagure/lib/model.py b/pagure/lib/model.py index 66bfe0b..7edf9bd 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -102,7 +102,7 @@ def create_default_status(session, acls=None): """ Insert the defaults status in the status tables. """ - statuses = ['Open', 'Invalid', 'Insufficient data', 'Fixed', 'Duplicate'] + statuses = ['Open', 'Closed'] for status in statuses: ticket_stat = StatusIssue(status=status) session.add(ticket_stat) @@ -320,6 +320,7 @@ class Project(BASE): _milestones = sa.Column(sa.Text, nullable=True) _reports = sa.Column(sa.Text, nullable=True) _notifications = sa.Column(sa.Text, nullable=True) + _close_status = sa.Column(sa.Text, nullable=True) date_created = sa.Column(sa.DateTime, nullable=False, default=datetime.datetime.utcnow) @@ -478,6 +479,23 @@ class Project(BASE): self._reports = json.dumps(reports) @property + def close_status(self): + """ Return the dict stored as string in the database as an actual + dict object. + """ + close_status = [] + + if self._close_status: + close_status = json.loads(self._close_status) + + return close_status + + @close_status.setter + def close_status(self, close_status): + ''' Ensures the different close status are properly saved. ''' + self._close_status = json.dumps(close_status) + + @property def open_requests(self): ''' Returns the number of open pull-requests for this project. ''' return BASE.metadata.bind.query( @@ -607,6 +625,7 @@ class Issue(BASE): private = sa.Column(sa.Boolean, nullable=False, default=False) priority = sa.Column(sa.Integer, nullable=True, default=None) milestone = sa.Column(sa.String(255), nullable=True, default=None) + close_status = sa.Column(sa.Text, nullable=True) date_created = sa.Column(sa.DateTime, nullable=False, default=datetime.datetime.utcnow) From 827c774a239fbb670c0cb8167051aecfcb1d5bb8 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 04 2016 17:47:41 +0000 Subject: [PATCH 2/7] Add possibility to set/remove the close status in the settings page of a project --- diff --git a/pagure/templates/settings.html b/pagure/templates/settings.html index f5308c5..1573ccf 100644 --- a/pagure/templates/settings.html +++ b/pagure/templates/settings.html @@ -619,6 +619,60 @@
+ Close status +
+
+

+ Here is the list of all the status that can be used when closing + an issue. +

+
+
+ {{ tag_form.csrf_token }} +
+
+
+ Status +
+
+
+ {% for status in repo.close_status | sort %} +
+
+ +
+
+ {% endfor %} +
+
+
+ + Add new close status + +
+
+
+
+ +
+
+
+
+
+
+ +
+
+
Issue Tags
@@ -807,5 +861,17 @@ $('#new_milestone').click(function(e) { ); }); +$('#new_close_status').click(function(e) { + console.log('new close status'); + $('#close_sstatus').append( + '
\ +
\ + \ +
\ +
' + ); +}); + {% endblock %} diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index 4d19889..c857a4a 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -2046,3 +2046,53 @@ def update_public_notifications(repo, username=None, namespace=None): return flask.redirect(flask.url_for( 'view_settings', username=username, repo=repo.name, namespace=repo.namespace)) + + +@APP.route('//update/close_status', methods=['POST']) +@APP.route('///update/close_status', methods=['POST']) +@APP.route('/fork///update/close_status', methods=['POST']) +@APP.route( + '/fork////update/close_status', + methods=['POST']) +@login_required +def update_close_status(repo, username=None, namespace=None): + """ Update the close_status of a project. + """ + if admin_session_timedout(): + flask.flash('Action canceled, try it again', 'error') + url = flask.url_for( + 'view_settings', username=username, repo=repo, + namespace=namespace) + return flask.redirect( + flask.url_for('auth_login', next=url)) + + repo = flask.g.repo + + if not repo.settings.get('issue_tracker', True): + flask.abort(404, 'No issue tracker found for this project') + + if not flask.g.repo_admin: + flask.abort( + 403, + 'You are not allowed to change the settings for this project') + + form = pagure.forms.ConfirmationForm() + + error = False + if form.validate_on_submit(): + close_status = [ + w.strip() for w in flask.request.form.getlist('close_status') + if w.strip() + ] + try: + repo.close_status = close_status + SESSION.add(repo) + SESSION.commit() + flask.flash('List of close status updated') + except SQLAlchemyError as err: # pragma: no cover + SESSION.rollback() + flask.flash(str(err), 'error') + + return flask.redirect(flask.url_for( + 'view_settings', username=username, repo=repo.name, + namespace=namespace)) From 05fc3ce3397ea600e5c360e81fcd13f7e6393698 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 04 2016 17:47:41 +0000 Subject: [PATCH 3/7] Adjust the issue view to include the close_status information Fixes https://pagure.io/pagure/issue/1373 --- diff --git a/pagure/forms.py b/pagure/forms.py index b8a2243..d632905 100644 --- a/pagure/forms.py +++ b/pagure/forms.py @@ -290,6 +290,11 @@ class UpdateIssueForm(FlaskForm): 'Private', [wtforms.validators.optional()], ) + close_status = wtforms.SelectField( + 'Closed as', + [wtforms.validators.Optional()], + choices=[] + ) def __init__(self, *args, **kwargs): """ Calls the default constructor with the normal argument but @@ -315,6 +320,12 @@ class UpdateIssueForm(FlaskForm): self.milestone.choices.append((key, key)) self.milestone.choices.insert(0, ('', '')) + self.close_status.choices = [] + if 'close_status' in kwargs: + for key in sorted(kwargs['close_status']): + self.close_status.choices.append((key, key)) + self.close_status.choices.insert(0, ('', '')) + class AddPullRequestCommentForm(FlaskForm): ''' Form to add a comment to a pull-request. ''' diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 0d8d09d..b4f55bb 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -1289,13 +1289,13 @@ def new_pull_request(session, branch_from, def edit_issue(session, issue, ticketfolder, user, - title=None, content=None, status=None, + title=None, content=None, status=None, close_status=None, priority=None, milestone=None, private=False): ''' Edit the specified issue. ''' user_obj = get_user(session, user) - if status == 'Fixed' and issue.parents: + if status != 'Open' and issue.parents: for parent in issue.parents: if parent.status == 'Open': raise pagure.exceptions.PagureException( @@ -1314,6 +1314,9 @@ def edit_issue(session, issue, ticketfolder, user, if status.lower() != 'open': issue.closed_at = datetime.datetime.utcnow() edit.append('status') + if close_status and close_status != issue.close_status: + issue.close_status = close_status + edit.append('close_status') if priority: try: priority = int(priority) diff --git a/pagure/templates/issue.html b/pagure/templates/issue.html index fe8b982..0287100 100644 --- a/pagure/templates/issue.html +++ b/pagure/templates/issue.html @@ -116,18 +116,26 @@

{{ issue.status }} + {% if issue.status == 'Closed' %} + + as: {{ issue.close_status }} + + {% endif %}

{% if authenticated and g.repo_admin %} {{ render_bootstrap_field(form.status, formclass="issue-metadata-form") }} + {{ render_bootstrap_field(form.close_status, + formclass="issue-metadata-form") }} {% endif%}
diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index f63b376..255dbca 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -102,6 +102,7 @@ def update_issue(repo, issueid, username=None, namespace=None): status=status, priorities=repo.priorities, milestones=repo.milestones, + close_status=repo.close_status, ) if form.validate_on_submit(): @@ -160,8 +161,14 @@ def update_issue(repo, issueid, username=None, namespace=None): except ValueError: pass - assignee = form.assignee.data - new_status = form.status.data + assignee = form.assignee.data.strip() or None + new_status = form.status.data.strip() or None + close_status = form.close_status.data.strip() or None + if new_status != 'Closed': + close_status = None + if close_status not in repo.close_status: + close_status = None + new_priority = None try: new_priority = int(form.priority.data) @@ -224,6 +231,7 @@ def update_issue(repo, issueid, username=None, namespace=None): SESSION, issue=issue, status=new_status, + close_status=close_status, private=issue.private, user=flask.g.fas_user.username, ticketfolder=APP.config['TICKETS_FOLDER'], @@ -768,11 +776,13 @@ def view_issue(repo, issueid, username=None, namespace=None): status=status, priorities=repo.priorities, milestones=repo.milestones, + close_status=repo.close_status, ) form.status.data = issue.status form.priority.data = str(issue.priority) form.milestone.data = str(issue.milestone) form.private.data = issue.private + form.close_status.data = issue.close_status tag_list = pagure.lib.get_tags_of_project(SESSION, repo) return flask.render_template( 'issue.html', From fc0a9440c31adf64284ee6d8c37c35a6eb5022ce Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 04 2016 17:47:41 +0000 Subject: [PATCH 4/7] Add support for close_status in the API while keeping the backward compatibility --- diff --git a/pagure/api/issue.py b/pagure/api/issue.py index 2520af6..bdf6295 100644 --- a/pagure/api/issue.py +++ b/pagure/api/issue.py @@ -510,11 +510,16 @@ def api_change_status_issue(repo, issueid, username=None, namespace=None): Input ^^^^^ - +-------------+---------+--------------+------------------------------+ - | Key | Type | Optionality | Description | - +=============+=========+==============+==============================+ - | ``status`` | string | Mandatory | The new status of the issue | - +-------------+---------+--------------+------------------------------+ + +----------------- +---------+--------------+------------------------+ + | Key | Type | Optionality | Description | + +==================+=========+==============+========================+ + | ``close_status`` | string | Optional | The close status of | + | | | | the issue | + +----------------- +---------+--------------+------------------------+ + | ``status`` | string | Mandatory | The new status of the | + | | | | issue, can be 'Open' or| + | | | | 'Closed' | | + +----------------- +---------+--------------+------------------------+ Sample response ^^^^^^^^^^^^^^^ @@ -555,15 +560,27 @@ def api_change_status_issue(repo, issueid, username=None, namespace=None): 403, error_code=APIERROR.EISSUENOTALLOWED) status = pagure.lib.get_issue_statuses(SESSION) - form = pagure.forms.StatusForm(status=status, csrf_enabled=False) + form = pagure.forms.StatusForm( + status=status, + close_status=repo.close_status, + csrf_enabled=False) + + close_status = None + if form.close_status.raw_data: + close_status = form.close_status.data + new_status = form.status.data.strip() + if new_status in repo.close_status and not close_status: + close_status = new_status + form.status.data = 'Closed' + if form.validate_on_submit(): - new_status = form.status.data try: # Update status message = pagure.lib.edit_issue( SESSION, issue=issue, status=new_status, + close_status=close_status, user=flask.g.fas_user.username, ticketfolder=APP.config['TICKETS_FOLDER'], ) diff --git a/pagure/forms.py b/pagure/forms.py index d632905..2a63713 100644 --- a/pagure/forms.py +++ b/pagure/forms.py @@ -216,6 +216,11 @@ class StatusForm(FlaskForm): [wtforms.validators.Required()], choices=[] ) + close_status = wtforms.SelectField( + 'Closed as', + [wtforms.validators.Optional()], + choices=[] + ) def __init__(self, *args, **kwargs): """ Calls the default constructor with the normal argument but @@ -227,6 +232,11 @@ class StatusForm(FlaskForm): self.status.choices = [ (status, status) for status in kwargs['status'] ] + self.close_status.choices = [] + if 'close_status' in kwargs: + for key in sorted(kwargs['close_status']): + self.close_status.choices.append((key, key)) + self.close_status.choices.insert(0, ('', '')) class NewTokenForm(FlaskForm): @@ -249,6 +259,7 @@ class NewTokenForm(FlaskForm): ] + class UpdateIssueForm(FlaskForm): ''' Form to add a comment to an issue. ''' tag = wtforms.TextField( From a836e36c893f1cc7bc8588c8e078a846cb8e4db1 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 04 2016 17:47:41 +0000 Subject: [PATCH 5/7] Fix showing the status of the issue in the issue list Fixes https://pagure.io/pagure/issue/1381 --- diff --git a/pagure/templates/issues.html b/pagure/templates/issues.html index a2708cc..ec07685 100644 --- a/pagure/templates/issues.html +++ b/pagure/templates/issues.html @@ -167,12 +167,14 @@ #{{ issue.id }} {% if status != 'Open' %} - {{issue.status}} + {{issue.close_status if issue.status == 'Closed' else issue.status }} {% endif %} {% if issue.private %} From 2aa887dfb08880dd7c15812fe5018a659d077962 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 04 2016 17:47:42 +0000 Subject: [PATCH 6/7] Adjust unit-tests for the changes related to close_status --- diff --git a/tests/__init__.py b/tests/__init__.py index 0aa4ebb..0d34759 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -257,6 +257,7 @@ def create_projects(session): description='test project #1', hook_token='aaabbbccc', ) + item.close_status = ['Invalid', 'Insufficient data', 'Fixed', 'Duplicate'] session.add(item) item = pagure.lib.model.Project( @@ -265,6 +266,7 @@ def create_projects(session): description='test project #2', hook_token='aaabbbddd', ) + item.close_status = ['Invalid', 'Insufficient data', 'Fixed', 'Duplicate'] session.add(item) session.commit() diff --git a/tests/test_pagure_flask_ui_issues.py b/tests/test_pagure_flask_ui_issues.py index 0cf262f..51261d7 100644 --- a/tests/test_pagure_flask_ui_issues.py +++ b/tests/test_pagure_flask_ui_issues.py @@ -509,7 +509,8 @@ class PagureFlaskIssuestests(tests.Modeltests): 'name="csrf_token" type="hidden" value="')[1].split('">')[0] data = { - 'status': 'fixed' + 'status': 'Closed', + 'close_status': 'fixed' } # Invalid repo @@ -535,7 +536,7 @@ class PagureFlaskIssuestests(tests.Modeltests): in output.data) # Right status, wrong csrf - data['status'] = 'Fixed' + data['close_status'] = 'Fixed' output = self.app.post( '/test/issue/1/update', data=data, follow_redirects=True) self.assertEqual(output.status_code, 200) @@ -570,7 +571,7 @@ class PagureFlaskIssuestests(tests.Modeltests): in output.data) self.assertIn( '

' - '@pingou changed the status to Fixed' + '@pingou changed the status to Closed' '

'.format( app_url=pagure.APP.config['APP_URL'].rstrip('/')), output.data) @@ -578,7 +579,8 @@ class PagureFlaskIssuestests(tests.Modeltests): # Add new comment data = { 'csrf_token': csrf_token, - 'status': 'Fixed', + 'status': 'Closed', + 'close_status': 'Fixed', 'comment': 'Woohoo a second comment !', } output = self.app.post( @@ -607,7 +609,8 @@ class PagureFlaskIssuestests(tests.Modeltests): # Add new tag data = { 'csrf_token': csrf_token, - 'status': 'Fixed', + 'status': 'Closed', + 'close_status': 'Fixed', 'tag': 'tag2', } output = self.app.post( @@ -630,7 +633,8 @@ class PagureFlaskIssuestests(tests.Modeltests): # Assign issue to an non-existent user data = { 'csrf_token': csrf_token, - 'status': 'Fixed', + 'status': 'Closed', + 'close_status': 'Fixed', 'assignee': 'ralph', } output = self.app.post( @@ -656,7 +660,8 @@ class PagureFlaskIssuestests(tests.Modeltests): # Assign issue properly data = { 'csrf_token': csrf_token, - 'status': 'Fixed', + 'status': 'Closed', + 'close_status': 'Fixed', 'assignee': 'pingou', } output = self.app.post( @@ -696,8 +701,9 @@ class PagureFlaskIssuestests(tests.Modeltests): # Reset the status of the first issue parent_issue = pagure.lib.search_issues( - self.session, repo, issueid=2) + self.session, repo, issueid=1) parent_issue.status = 'Open' + self.session.add(parent_issue) # Add the dependency relationship self.session.add(parent_issue) issue = pagure.lib.search_issues(self.session, repo, issueid=2) diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py index e946395..09fb942 100644 --- a/tests/test_pagure_lib.py +++ b/tests/test_pagure_lib.py @@ -1044,9 +1044,7 @@ class PagureLibtests(tests.Modeltests): def test_get_issue_statuses(self): """ Test the get_issue_statuses of pagure.lib. """ statuses = pagure.lib.get_issue_statuses(self.session) - self.assertEqual( - sorted(statuses), - ['Duplicate', 'Fixed', 'Insufficient data', 'Invalid', 'Open']) + self.assertEqual(sorted(statuses), ['Closed', 'Open']) def test_set_up_user(self): """ Test the set_up_user of pagure.lib. """ From 6ea90a178c19cb4d3430614aac411ffa0f408f12 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 05 2016 08:53:29 +0000 Subject: [PATCH 7/7] Allow the returned close_status to be None and not 'None' --- diff --git a/pagure/forms.py b/pagure/forms.py index 2a63713..682beb2 100644 --- a/pagure/forms.py +++ b/pagure/forms.py @@ -304,7 +304,8 @@ class UpdateIssueForm(FlaskForm): close_status = wtforms.SelectField( 'Closed as', [wtforms.validators.Optional()], - choices=[] + choices=[], + coerce=lambda val: unicode(val) if val else None ) def __init__(self, *args, **kwargs):