From 853b2023a063ca2956fc8a522938ad681fbb2e48 Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: Feb 20 2017 13:51:25 +0000 Subject: [PATCH 1/9] Add functionality for dealing with issues with no milestone We need a way find issues that do not have a milestone set yet, and we need a way to find tickets that have specific milestones as well as no milestone. For example: When new tickets are created they don't have a milestone, but we also have a NEEDS_TRIAGE milestone. So to find all the issues that need to triaged(aka have a milestone set) we need to find both types. This patch adds a new "No milestone" button to the roadmap page, and it allows you to search issues by using multiple milestones(including "none"): http://localhost.localdomain:5000/TEST_PROJ/issues?milestone=none&milestone=0.0 We can save this as a custom report for finding all the issues that need to be triaged. --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 52599bd..040f042 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -2028,7 +2028,7 @@ def search_issues( closed=False, tags=None, assignee=None, author=None, private=None, priority=None, milestones=None, count=False, offset=None, limit=None, search_pattern=None, custom_search=None, - updated_after=None): + updated_after=None, no_milestones=None): ''' Retrieve one or more issues associated to a project with the given criterias. @@ -2083,6 +2083,8 @@ def search_issues( :kwarg updated_after: datetime's date format (e.g. 2016-11-15) used to filter issues updated after that date :type updated_after: str or None + :kwarg no_milestones: Request issues that do not have a milestone set yet + :type None, True, or False :return: A single Issue object if issueid is specified, a list of Project objects otherwise. @@ -2127,6 +2129,7 @@ def search_issues( query = query.filter( model.Issue.priority == priority ) + if tags is not None and tags != []: if isinstance(tags, basestring): tags = [tags] @@ -2225,10 +2228,23 @@ def search_issues( ) ) - if milestones is not None and milestones != []: + if no_milestones and milestones is not None and milestones != []: + # Asking for issues with no milestone or a specific milestone + if isinstance(milestones, basestring): + milestones = [milestones] + query = query.filter( + (model.Issue.milestone == None) | + (model.Issue.milestone.in_(milestones)) + ) + elif no_milestones: + # Asking for issues without a milestone + query = query.filter( + model.Issue.milestone == None + ) + elif milestones is not None and milestones != []: + # Asking for a single specific milestone if isinstance(milestones, basestring): milestones = [milestones] - query = query.filter( model.Issue.milestone.in_(milestones) ) diff --git a/pagure/templates/roadmap.html b/pagure/templates/roadmap.html index 12d54fe..81669e6 100644 --- a/pagure/templates/roadmap.html +++ b/pagure/templates/roadmap.html @@ -79,7 +79,7 @@ All Milestones + No Milestone @@ -141,6 +151,109 @@ {% endif %} + +{% if no_stones %} +
+
+ + + + + + {% if status and status|lower == 'closed' %} + + {% else %} + + {% endif %} + + + + + + + + {% for issue in issues |sort(attribute='priority') %} + {% if status is none or status|lower == 'all' or issue.status == status %} + + + + + + + + + {% endif %} + {% else %} + + + + {% endfor %} + +
No Milestone + OpenedClosedModified + Priority + + Assignee + Status
+ #{{ issue.id }} + {% if issue.private %} + + {% endif %} + + {{ issue.title | noJS("img") | safe }} + +    + {% if issue.comments|count > 0 %} + + + {{issue.comments|count}} + + {% endif %} + {% for tag in issue.tags%} + {{tag.tag}} + {% endfor%} + + {{ + issue.date_created | humanize}} + + {% if status and status|lower == 'closed' %} + {{ + issue.closed_at | humanize}} + {% else %} + {{ + issue.last_updated | humanize}} + {% endif %} + + {% if issue.priority %} + {{ repo.priorities[issue.priority | string] }} + {% endif %} + + {% if issue.assignee %} + {{ issue.assignee.default_email | avatar(16) | safe }} + {{ issue.assignee.user }} + {% else %} + unassigned + {% endif %} + + {{ issue.status }} +
No issues found
+
+
+{% endif %} + {% for milestone in milestones %} {% if issues[milestone] %}
diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index 25b633b..b00aa29 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -590,6 +590,7 @@ def view_issues(repo, username=None, namespace=None): assignee = flask.request.args.get('assignee', None) author = flask.request.args.get('author', None) search_pattern = flask.request.args.get('search_pattern', None) + milestones = flask.request.args.getlist('milestone', None) # Custom fields custom_keys = flask.request.args.getlist('ckeys') @@ -599,6 +600,11 @@ def view_issues(repo, username=None, namespace=None): for idx, key in enumerate(custom_keys): custom_search[key] = custom_values[idx] + if "none" in milestones: + no_stone = True + milestones.remove("none") + else: + no_stone = False search_string = search_pattern extra_fields, search_pattern = pagure.lib.tokenize_search_string( search_pattern) @@ -646,6 +652,8 @@ def view_issues(repo, username=None, namespace=None): limit=flask.g.limit, search_pattern=search_pattern, custom_search=custom_search, + milestones=milestones, + no_milestones=no_stone ) issues_cnt = pagure.lib.search_issues( SESSION, @@ -730,6 +738,7 @@ def view_roadmap(repo, username=None, namespace=None): milestones = flask.request.args.getlist('milestone', None) tags = flask.request.args.getlist('tag', None) all_stones = flask.request.args.get('all_stones') + no_stones = flask.request.args.get('no_stones') repo = flask.g.repo @@ -746,6 +755,25 @@ def view_roadmap(repo, username=None, namespace=None): if flask.g.repo_committer: private = None + if no_stones: + # Return only issues that do not have a milestone set + issues = pagure.lib.search_issues( + SESSION, + repo, + no_milestones=True, + status=status if status.lower() != 'all' else None, + ) + return flask.render_template( + 'roadmap.html', + select='issues', + repo=repo, + username=username, + status=status, + no_stones=True, + issues=issues, + tags=tags, + ) + all_milestones = sorted(list(repo.milestones.keys())) active_milestones = pagure.lib.get_active_milestones(SESSION, repo) From c4f9a5689a371eebbc6f652b4b88e303d381f5b2 Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: Feb 20 2017 13:51:25 +0000 Subject: [PATCH 2/9] Add unit tests --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 040f042..10b86db 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -2129,7 +2129,6 @@ def search_issues( query = query.filter( model.Issue.priority == priority ) - if tags is not None and tags != []: if isinstance(tags, basestring): tags = [tags] diff --git a/tests/test_pagure_flask_ui_issues.py b/tests/test_pagure_flask_ui_issues.py index 06b38f5..01694e6 100644 --- a/tests/test_pagure_flask_ui_issues.py +++ b/tests/test_pagure_flask_ui_issues.py @@ -337,6 +337,18 @@ class PagureFlaskIssuestests(tests.Modeltests): msg = pagure.lib.new_issue( session=self.session, repo=repo, + title='Test issue with milestone', + content='Testing search', + user='pingou', + milestone='1.1', + ticketfolder=None + ) + self.session.commit() + self.assertEqual(msg.title, 'Test issue with milestone') + + msg = pagure.lib.new_issue( + session=self.session, + repo=repo, title='Test invalid issue', content='This really is not related', user='pingou', @@ -360,7 +372,7 @@ class PagureFlaskIssuestests(tests.Modeltests): self.assertEqual(output.status_code, 200) self.assertIn('Issues - test - Pagure', output.data) self.assertTrue( - '

\n 1 Open Issues' in output.data) + '

\n 2 Open Issues' in output.data) # Status = closed (all but open) output = self.app.get('/test/issues?status=cloSED') @@ -385,11 +397,10 @@ class PagureFlaskIssuestests(tests.Modeltests): # All tickets output = self.app.get('/test/issues?status=all') - self.assertEqual(output.status_code, 200) self.assertIn('Issues - test - Pagure', output.data) self.assertTrue( - '

\n 2 Issues' in output.data) + '

\n 3 Issues' in output.data) # Custom key searching output = self.app.get( @@ -411,9 +422,9 @@ class PagureFlaskIssuestests(tests.Modeltests): output = self.app.get('/test/issues?status=all') self.assertEqual(output.status_code, 200) self.assertIn('Issues - test - Pagure', output.data) - self.assertIn('

\n 1 Issues (of 2)', output.data) + self.assertIn('

\n 1 Issues (of 3)', output.data) self.assertIn( - '
  • page 1 of 2
    • ', output.data) + '
  • page 1 of 3
    • ', output.data) # All tickets - filtered for 1 - checking the pagination output = self.app.get( @@ -425,6 +436,20 @@ class PagureFlaskIssuestests(tests.Modeltests): '
  • page 1 of 1
    • ', output.data) pagure.APP.config['ITEM_PER_PAGE'] = before + # Search for issues with no milestone MARK + output = self.app.get( + '/test/issues?milestone=none') + self.assertEqual(output.status_code, 200) + self.assertIn('Issues - test - Pagure', output.data) + self.assertIn('1 Open Issues (of 2)', output.data) + + # Search for issues with no milestone and milestone 1.1 + output = self.app.get( + '/test/issues?milestone=none&milestone=1.1') + self.assertEqual(output.status_code, 200) + self.assertIn('Issues - test - Pagure', output.data) + self.assertIn('2 Open Issues (of 2)', output.data) + # New issue button is shown user = tests.FakeUser() with tests.user_set(pagure.APP, user): From 1cde0ffe9eaa7703f0c0029e6cc0a04e01790ffa Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 20 2017 13:51:25 +0000 Subject: [PATCH 3/9] Do not show the milestone glyph if there are no milestone known --- diff --git a/pagure/templates/roadmap.html b/pagure/templates/roadmap.html index 81669e6..266b25e 100644 --- a/pagure/templates/roadmap.html +++ b/pagure/templates/roadmap.html @@ -109,6 +109,7 @@ status=status) }}" title="Display issues with no milestone set">No Milestone + {% if milestones %} {% for stone in milestones %} {% for tag in tag_list %} From 33c52f113dc8f454c2c23d08c57223864fdf0151 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 20 2017 13:51:25 +0000 Subject: [PATCH 4/9] When showing all the tickets without milestone, offer filtering by tags --- diff --git a/pagure/templates/roadmap.html b/pagure/templates/roadmap.html index 266b25e..c92b69a 100644 --- a/pagure/templates/roadmap.html +++ b/pagure/templates/roadmap.html @@ -122,6 +122,7 @@ milestone=stone | add_or_remove(requested_stones[:]), tag=tags, all_stones=all_stones, + no_stones=no_stones, status=status) }}" title="Filter issues by milestone"> {{ stone }} @@ -139,6 +140,7 @@ status=status, tag=tag | add_or_remove(tags[:]), all_stones=all_stones, + no_stones=no_stones, milestone=requested_stones) }}" title="Filter issues by tag"> {{ tag }} diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index b00aa29..f7b854a 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -605,6 +605,7 @@ def view_issues(repo, username=None, namespace=None): milestones.remove("none") else: no_stone = False + search_string = search_pattern extra_fields, search_pattern = pagure.lib.tokenize_search_string( search_pattern) @@ -755,12 +756,31 @@ def view_roadmap(repo, username=None, namespace=None): if flask.g.repo_committer: private = None + tag_list = [ + tag.tag + for tag in pagure.lib.get_tags_of_project(SESSION, repo) + ] + + all_milestones = sorted(list(repo.milestones.keys())) + active_milestones = pagure.lib.get_active_milestones(SESSION, repo) + + milestones_list = active_milestones + if all_stones: + milestones_list = all_milestones + + if 'unplanned' in all_milestones: + index = all_milestones.index('unplanned') + cnt = len(all_milestones) + all_milestones.insert(cnt, all_milestones.pop(index)) + if no_stones: # Return only issues that do not have a milestone set issues = pagure.lib.search_issues( SESSION, repo, no_milestones=True, + tags=tags, + private=private, status=status if status.lower() != 'all' else None, ) return flask.render_template( @@ -768,15 +788,15 @@ def view_roadmap(repo, username=None, namespace=None): select='issues', repo=repo, username=username, + tag_list=tag_list, status=status, no_stones=True, issues=issues, tags=tags, + all_stones=all_stones, + requested_stones=milestones, ) - all_milestones = sorted(list(repo.milestones.keys())) - active_milestones = pagure.lib.get_active_milestones(SESSION, repo) - issues = pagure.lib.search_issues( SESSION, repo, @@ -808,20 +828,6 @@ def view_roadmap(repo, username=None, namespace=None): if not active: del milestone_issues[key] - tag_list = [ - tag.tag - for tag in pagure.lib.get_tags_of_project(SESSION, repo) - ] - - if 'unplanned' in all_milestones: - index = all_milestones.index('unplanned') - cnt = len(all_milestones) - all_milestones.insert(cnt, all_milestones.pop(index)) - - milestones_list = active_milestones - if all_stones: - milestones_list = all_milestones - return flask.render_template( 'roadmap.html', select='issues', From f18d48087a8cbb45f0b45d6d832a0fdd2e919d09 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 20 2017 14:12:17 +0000 Subject: [PATCH 5/9] Use a macro to render the issue table in the roadmap --- diff --git a/pagure/templates/roadmap.html b/pagure/templates/roadmap.html index c92b69a..6cdec80 100644 --- a/pagure/templates/roadmap.html +++ b/pagure/templates/roadmap.html @@ -4,9 +4,113 @@ repo.namespace + '/' if repo.namespace }}{{ repo.name }}{% endblock %} {% set tag = "home"%} - {% block repo %} +{% macro render_issue_list(issues, title, id, milestone) %} +
    +
    + + + + + + {% if status and status|lower == 'closed' %} + + {% else %} + + {% endif %} + + + + + + + + {% for issue in issues |sort(attribute='priority') %} + {% if status is none or status|lower == 'all' or issue.status == status %} + + + + + + + + + {% endif %} + {% else %} + + + + {% endfor %} + +
    {{ title }} + {% if milestone and repo.milestones[milestone] %} +   (Due: {{ repo.milestones[milestone] }}) + {% endif %} + OpenedClosedModified + Priority + + Assignee + Status
    + #{{ issue.id }} + {% if issue.private %} + + {% endif %} + + {{ issue.title | noJS("img") | safe }} + +    + {% if issue.comments | count > 0 %} + + + {{ issue.comments | count }} + + {% endif %} + {% for tag in issue.tags %} + {{ tag.tag }} + {% endfor %} + + {{ + issue.date_created | humanize }} + + {% if status and status|lower == 'closed' %} + {{ + issue.closed_at | humanize }} + {% else %} + {{ + issue.last_updated | humanize }} + {% endif %} + + {% if issue.priority %} + {{ repo.priorities[issue.priority | string] }} + {% endif %} + + {% if issue.assignee %} + {{ issue.assignee.default_email | avatar(16) | safe }} + {{ issue.assignee.user }} + {% else %} + unassigned + {% endif %} + + {{ issue.status }} +
    No issues found
    +
    +
    +{% endmacro %} +

    Milestone Roadmap @@ -150,218 +254,23 @@ {% if not issues %}
    - - No issues found - + + No issues found +
    {% endif %} {% if no_stones %} -
    -
    - - - - - - {% if status and status|lower == 'closed' %} - - {% else %} - - {% endif %} - - - - - - - - {% for issue in issues |sort(attribute='priority') %} - {% if status is none or status|lower == 'all' or issue.status == status %} - - - - - - - - - {% endif %} - {% else %} - - - - {% endfor %} - -
    No Milestone - OpenedClosedModified - Priority - - Assignee - Status
    - #{{ issue.id }} - {% if issue.private %} - - {% endif %} - - {{ issue.title | noJS("img") | safe }} - -    - {% if issue.comments|count > 0 %} - - - {{issue.comments|count}} - - {% endif %} - {% for tag in issue.tags%} - {{tag.tag}} - {% endfor%} - - {{ - issue.date_created | humanize}} - - {% if status and status|lower == 'closed' %} - {{ - issue.closed_at | humanize}} - {% else %} - {{ - issue.last_updated | humanize}} - {% endif %} - - {% if issue.priority %} - {{ repo.priorities[issue.priority | string] }} - {% endif %} - - {% if issue.assignee %} - {{ issue.assignee.default_email | avatar(16) | safe }} - {{ issue.assignee.user }} - {% else %} - unassigned - {% endif %} - - {{ issue.status }} -
    No issues found
    -
    -
    + {{ render_issue_list( + issues, title='No Milestone', + id='no_stones', milestone=None) }} {% endif %} {% for milestone in milestones %} {% if issues[milestone] %} -
    -
    - - - - - - {% if status and status|lower == 'closed' %} - - {% else %} - - {% endif %} - - - - - - - - {% for issue in issues[milestone] |sort(attribute='priority') %} - {% if status is none or status|lower == 'all' or issue.status == status %} - - - - - - - - - {% endif %} - {% else %} - - - - {% endfor %} - -
    {{ milestone }} - {% if repo.milestones[milestone] %} -   (Due: {{ repo.milestones[milestone] }}) - {% endif %} - OpenedClosedModified - Priority - - Assignee - Status
    - #{{ issue.id }} - {% if issue.private %} - - {% endif %} - - {{ issue.title | noJS("img") | safe }} - -    - {% if issue.comments|count > 0 %} - - - {{issue.comments|count}} - - {% endif %} - {% for tag in issue.tags%} - {{tag.tag}} - {% endfor%} - - {{ - issue.date_created | humanize}} - - {% if status and status|lower == 'closed' %} - {{ - issue.closed_at | humanize}} - {% else %} - {{ - issue.last_updated | humanize}} - {% endif %} - - {% if issue.priority %} - {{ repo.priorities[issue.priority | string] }} - {% endif %} - - {% if issue.assignee %} - {{ issue.assignee.default_email | avatar(16) | safe }} - {{ issue.assignee.user }} - {% else %} - unassigned - {% endif %} - - {{ issue.status }} -
    No issues found
    -
    -
    + {{ render_issue_list( + issues[milestone], title=milestone, + id=loop.index, milestone=milestone) }} {% endif %} {% endfor %} {% endblock %} From 0bc2af7818bfbad8041f9d9f7bbef521b99404e2 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 20 2017 14:41:41 +0000 Subject: [PATCH 6/9] Fix the pagination when filtering for the issues with no milestone assigned --- diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index f7b854a..eda0554 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -668,6 +668,7 @@ def view_issues(repo, username=None, namespace=None): priority=priority, search_pattern=search_pattern, custom_search=custom_search, + no_milestones=no_stone, count=True ) oth_issues = pagure.lib.search_issues( @@ -681,6 +682,7 @@ def view_issues(repo, username=None, namespace=None): priority=priority, search_pattern=search_pattern, custom_search=custom_search, + no_milestones=no_stone, count=True, ) oth_issues_cnt = total_issues_cnt - issues_cnt From bc789adfc5a9f14cc67af0daaa7766ce18c021ea Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 20 2017 15:19:17 +0000 Subject: [PATCH 7/9] Adjust the combine_url jinja filter to account for argument list Basically when we had multiple argument with the same name (ie a list) we were moving it into a single element instead of a list, in other words we were going from: ?foo=bar&foo=baz to ?foo=baz thus loosing half of the information. This commit fixes this. --- diff --git a/pagure/ui/filters.py b/pagure/ui/filters.py index dbcb611..5ad6c74 100644 --- a/pagure/ui/filters.py +++ b/pagure/ui/filters.py @@ -568,10 +568,25 @@ def combine_url(url, page, pagetitle, **kwargs): """ url_obj = urlparse.urlparse(url) url = url_obj.geturl().replace(url_obj.query, '').rstrip('?') - query = dict(urlparse.parse_qsl(url_obj.query)) + query = {} + for k, v in urlparse.parse_qsl(url_obj.query): + if k in query: + if isinstance(query[k], list): + query[k].append(v) + else: + query[k] = [query[k], v] + else: + query[k] = v query[pagetitle] = page query.update(kwargs) - return url + '?' + '&'.join(['%s=%s' % (k, query[k]) for k in query]) + args = '' + for key in query: + if isinstance(query[key], list): + for val in query[key]: + args += '&%s=%s' % (key, val) + else: + args += '&%s=%s' % (key, query[key]) + return url + '?' + args[1:] @APP.template_filter('add_or_remove') From 5d49b71e5ee24f48e063ef276dbdf6a187d2a1ad Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 20 2017 15:21:23 +0000 Subject: [PATCH 8/9] Drop the variable oth_issues in favor of oth_issues_cnt --- diff --git a/pagure/templates/issues.html b/pagure/templates/issues.html index 77e9fd4..b9cff13 100644 --- a/pagure/templates/issues.html +++ b/pagure/templates/issues.html @@ -48,7 +48,7 @@ {% endif %}

    - {% if oth_issues %} + {% if oth_issues_cnt %}
    - {% if (issues | length + oth_issues) %} + {% if (issues | length + oth_issues_cnt) %}