From 18ed76d93d83f38f870d0ffa0e9a6b4915c7c039 Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: Jan 12 2017 15:36:17 +0000 Subject: [PATCH 1/10] Issue 1711 - Improve Issue roadmap page The roapmap page appears to have been started, but not finished. Certain features did not work, like sorting on tags (tags were previosuly being treated as milestones). This fix now makes it possible to isolate individual milestones, status, and tags. All three can be combined to create a new report. Each criteria is clicked to select and deselect. The layout of the page has also been improved to be more consistent, and uniform https://pagure.io/pagure/issue/1711 --- diff --git a/pagure/templates/roadmap.html b/pagure/templates/roadmap.html index 88d33f5..0825134 100644 --- a/pagure/templates/roadmap.html +++ b/pagure/templates/roadmap.html @@ -9,7 +9,7 @@

- {{ issues|count }} Milestones + Milestone Roadmap +
Milestones:  + + {% for stone in allmilestones|sort %} + {% if stone in requested_stones %} + + {{ stone }} + {% else %} + + {{ stone }} + {% endif %} + {% endfor %} + +
+
Status:  {% if not status %} Open - All {% else %} - Open All {% endif %} - +
+ +
Tags:  {% for tag in tag_list %} - {% if tag.tag in tags %} - + {% if tag in tags %} + {% else %} - + {% endif %} - - {{ tag.tag }} + {{ tag }} {% endfor %} - +
+
{% for milestone in milestones %}
-
-
- Milestone: {{ milestone }} - {% if repo.milestones[milestone] %} - - Due: {{ repo.milestones[milestone] }} - - {% endif %} -
-
-
- - +
+ + + + + {% if status and status|lower != 'open' %} + + {% else %} + + {% endif %} + + {% if not status or status|lower == 'open' %} + + {% endif %} + {% if repo.milestones[milestone] %} + + Due: {{ repo.milestones[milestone] }} + + {% endif %} + + + - {% for issue in issues[milestone] |sort(attribute='priority') %} + {% for issue in issues[milestone] |sort(attribute='priority') %} + {% if status is none or (status and issue.status == 'Open') %} + + + {% endif %} {% else %} @@ -195,6 +266,12 @@
Milestone: {{ milestone }}OpenedClosedModified + Priority (reset) + + Assignee (reset) + Status
#{{ issue.id }} - {% if status and status != 'Open' %} + {% if status is none or status != 'Open' %} {{issue.status}} + issue.close_status|lower == 'invalid' %}label-danger{% + elif issue.close_status|lower == 'fixed' %}label-success{% + elif issue.close_status|lower == 'insufficient data' %}label-warning{% + elif issue.close_status|lower == 'duplicate' %}label-default{% + elif issue.close_status %}label-default{% + endif %}">{{issue.close_status}} {% endif %} {% if issue.private %} @@ -163,6 +230,10 @@ {{ issue.date_created | humanize}} + {{ + issue.last_updated | humanize}} + - {% if issue.status != 'Open' %} - {{ issue.status }} + {% if issue.assignee %} + {{ issue.assignee.default_email | avatar(16) | safe }} + {{ issue.assignee.user }} {% else %} - {% if issue.assignee %} - {{ issue.assignee.default_email | avatar(16) | safe }} - {{ issue.assignee.user }} - {% else %} - unassigned - {% endif %} + unassigned {% endif %} + {{ issue.status }} +
No issues found
+{% else %} +
+ + No issues found + +
{% endfor %} {% endblock %} {% block jscripts %} diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index 4ce8d5d..e192a7e 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -686,6 +686,7 @@ def view_roadmap(repo, username=None, namespace=None): if status.lower() == 'all': status = None milestone = flask.request.args.getlist('milestone', None) + tags = flask.request.args.getlist('tag', None) repo = flask.g.repo @@ -701,12 +702,17 @@ def view_roadmap(repo, username=None, namespace=None): if flask.g.repo_admin: private = None + requested_stones = None + if milestone is not None: + requested_stones = milestone milestones = milestone or list(repo.milestones.keys()) + all_milestones = list(repo.milestones.keys()) issues = pagure.lib.search_issues( SESSION, repo, milestones=milestones, + tags=tags, private=private, ) @@ -732,12 +738,10 @@ def view_roadmap(repo, username=None, namespace=None): if not active: del milestone_issues[key] - if milestone: - for mlstone in milestone: - if mlstone not in milestone_issues: - milestone_issues[mlstone] = [] - - tag_list = pagure.lib.get_tags_of_project(SESSION, repo) + all_tags = pagure.lib.get_tags_of_project(SESSION, repo) + tag_list = [] + for tag in all_tags: + tag_list.append(tag.tag) reponame = pagure.get_repo_path(repo) repo_obj = pygit2.Repository(reponame) @@ -755,8 +759,10 @@ def view_roadmap(repo, username=None, namespace=None): tag_list=tag_list, status=status, milestones=milestones_ordered, + requested_stones=requested_stones, + allmilestones=all_milestones, issues=milestone_issues, - tags=milestone, + tags=tags, ) From e73313d102ffc56e3e70f50901b8295cdcd611c8 Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: Jan 12 2017 15:36:17 +0000 Subject: [PATCH 2/10] Applied recommendations - Removed the milestones variable - Revised the Due section of a mielstone - Removed the old closed status icons - Updated the tests --- diff --git a/pagure/templates/roadmap.html b/pagure/templates/roadmap.html index 0825134..4990cbf 100644 --- a/pagure/templates/roadmap.html +++ b/pagure/templates/roadmap.html @@ -151,10 +151,14 @@ {% for milestone in milestones %}
- +
- + {% if status and status|lower != 'open' %} @@ -179,11 +183,6 @@ status=status) }}">reset) {% endif %} - {% if repo.milestones[milestone] %} - - Due: {{ repo.milestones[milestone] }} - - {% endif %} @@ -194,15 +193,6 @@
Milestone: {{ milestone }}Milestone: {{ milestone }} + {% if repo.milestones[milestone] %} +   (Due: {{ repo.milestones[milestone] }}) + {% endif %} + OpenedClosedStatus
#{{ issue.id }} - {% if status is none or status != 'Open' %} - {{issue.close_status}} - {% endif %} {% if issue.private %} {% endif %} diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index e192a7e..ca1067b 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -249,7 +249,7 @@ def update_issue(repo, issueid, username=None, namespace=None): ticketfolder=APP.config['TICKETS_FOLDER'], ) SESSION.commit() - if message and message != 'Nothing to change': + if message: messages.add(message) # Update priority @@ -705,13 +705,12 @@ def view_roadmap(repo, username=None, namespace=None): requested_stones = None if milestone is not None: requested_stones = milestone - milestones = milestone or list(repo.milestones.keys()) all_milestones = list(repo.milestones.keys()) issues = pagure.lib.search_issues( SESSION, repo, - milestones=milestones, + milestones=milestone or list(repo.milestones.keys()), tags=tags, private=private, ) @@ -720,7 +719,7 @@ def view_roadmap(repo, username=None, namespace=None): milestone_issues = defaultdict(list) for cnt in range(len(issues)): saved = False - for mlstone in sorted(milestones): + for mlstone in sorted(milestone or list(repo.milestones.keys())): if mlstone == issues[cnt].milestone: milestone_issues[mlstone].append(issues[cnt]) saved = True diff --git a/tests/test_pagure_flask_ui_roadmap.py b/tests/test_pagure_flask_ui_roadmap.py index c01da82..8053e8a 100644 --- a/tests/test_pagure_flask_ui_roadmap.py +++ b/tests/test_pagure_flask_ui_roadmap.py @@ -162,7 +162,6 @@ class PagureFlaskRoadmaptests(tests.Modeltests): 'href="/test/issue/1/edit" title="Edit this issue">', output.data) - def test_update_milestones(self): """ Test updating milestones of a repo. """ tests.create_projects(self.session) @@ -482,7 +481,6 @@ class PagureFlaskRoadmaptests(tests.Modeltests): # test the roadmap view output = self.app.get('/test/roadmap') self.assertEqual(output.status_code, 200) - self.assertIn(u'2 Milestones', output.data) self.assertIn(u'Milestone: v2.0', output.data) self.assertIn(u'Milestone: unplanned', output.data) self.assertEqual( @@ -491,7 +489,6 @@ class PagureFlaskRoadmaptests(tests.Modeltests): # test the roadmap view for all milestones output = self.app.get('/test/roadmap?status=All') self.assertEqual(output.status_code, 200) - self.assertIn(u'3 Milestones', output.data) self.assertIn(u'Milestone: v1.0', output.data) self.assertIn(u'Milestone: v2.0', output.data) self.assertIn(u'Milestone: unplanned', output.data) @@ -501,27 +498,31 @@ class PagureFlaskRoadmaptests(tests.Modeltests): # test the roadmap view for a specific milestone output = self.app.get('/test/roadmap?milestone=v2.0') self.assertEqual(output.status_code, 200) - self.assertIn(u'1 Milestones', output.data) self.assertIn(u'Milestone: v2.0', output.data) self.assertEqual( output.data.count(u'#'), 2) - # test the roadmap view for a specific milestone - closed + # test the roadmap view for a specific milestone - open output = self.app.get('/test/roadmap?milestone=v1.0') self.assertEqual(output.status_code, 200) - self.assertIn(u'1 Milestones', output.data) - self.assertIn(u'Milestone: v1.0', output.data) + self.assertIn(u'No issues found', output.data) self.assertEqual( output.data.count(u'#'), 0) # test the roadmap view for a specific milestone - closed output = self.app.get('/test/roadmap?milestone=v1.0&status=All') self.assertEqual(output.status_code, 200) - self.assertIn(u'1 Milestones', output.data) self.assertIn(u'Milestone: v1.0', output.data) self.assertEqual( output.data.count(u'#'), 2) + # test the roadmap view for a specific tag + output = self.app.get('/test/roadmap?milestone=v2.0&tag=unknown') + self.assertEqual(output.status_code, 200) + self.assertIn(u'No issues found', output.data) + self.assertEqual( + output.data.count(u'#'), 0) + # test the roadmap view for errors output = self.app.get('/foo/roadmap') self.assertEqual(output.status_code, 404) From f4c6ff81855f0d2278256631baa975ce694c2a5c Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: Jan 12 2017 15:36:17 +0000 Subject: [PATCH 3/10] Fix table id --- diff --git a/pagure/templates/roadmap.html b/pagure/templates/roadmap.html index 4990cbf..515c743 100644 --- a/pagure/templates/roadmap.html +++ b/pagure/templates/roadmap.html @@ -151,7 +151,7 @@ {% for milestone in milestones %}
- +
+{% endif %} +{% for milestone in milestones %} + {% if issues[milestone] %} +
+
+
Milestone: {{ milestone }} From 3b943e1038a8bf92894997a2268133baf2e22592 Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: Jan 12 2017 15:36:17 +0000 Subject: [PATCH 4/10] Add table milestone class for table indentation Also added an index for the table id --- diff --git a/pagure/static/pagure.css b/pagure/static/pagure.css index a8abce1..4a5936b 100644 --- a/pagure/static/pagure.css +++ b/pagure/static/pagure.css @@ -128,6 +128,14 @@ white-space: nowrap; display:none; } +.milestone-table { + width: 100%; +} + +.milestone-table tr td:first-child { + width: 100%; +} + .bodycontent { min-height: 85vh; diff --git a/pagure/templates/roadmap.html b/pagure/templates/roadmap.html index 515c743..92a165a 100644 --- a/pagure/templates/roadmap.html +++ b/pagure/templates/roadmap.html @@ -148,10 +148,12 @@
+{% set index = 1 %} {% for milestone in milestones %}
- +
+ {% set index = index + 1 %} - {% endif %} {% if not status or status|lower == 'open' %} {% endif %} From 4b89e6638230f68b74be8b9bd134b1dd06b7c596 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jan 12 2017 15:36:17 +0000 Subject: [PATCH 6/10] Rework the roadmap page based on Mark Reynold's work - Make the tags list display in the same way as in the issue list page - Add the milestones using the `target` icon - Add all the status Open|Closed|All as we do in the issue list page - Fix the logic to properly display the Closed issues - Remove some of the redundant variables - Simplify some small bits of code --- diff --git a/pagure/templates/roadmap.html b/pagure/templates/roadmap.html index 15c369a..b06bcd5 100644 --- a/pagure/templates/roadmap.html +++ b/pagure/templates/roadmap.html @@ -8,59 +8,108 @@ {% block repo %} -

- Milestone Roadmap - - - - - {% if g.repo.reports %} -

+
-
Milestones:  +
- {% for stone in allmilestones|sort %} + Open + Closed + All + + + + {% for tag in tag_list %} + {% if tag in tags %} + + {% else %} + + {% endif %} + {{ tag }} + {% endfor %} + + + + {% for stone in milestones %} {% if stone in requested_stones %} - {{ stone }} {% else %} @@ -72,188 +121,126 @@ namespace=repo.namespace, milestone=stone, tag=tags, - status='All' if not status else None) }}" + status=status) }}" title="Filter issues by milestone"> {{ stone }} {% endif %} {% endfor %} - -
-
Status:  - - {% if not status %} - Open - All - {% else %} - Open - All - {% endif %} - -
- -
Tags:  - - {% for tag in tag_list %} - {% if tag in tags %} - - {% else %} - - {% endif %} - {{ tag }} - {% endfor %}
-
-{% set index = 1 %} -{% for milestone in milestones %} -
-
-
Milestone: {{ milestone }} From 40571fc3bee2c6029fd4f44f807509bed0384624 Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: Jan 12 2017 15:36:17 +0000 Subject: [PATCH 5/10] Remove "reset" links from priority and assigned --- diff --git a/pagure/templates/roadmap.html b/pagure/templates/roadmap.html index 92a165a..15c369a 100644 --- a/pagure/templates/roadmap.html +++ b/pagure/templates/roadmap.html @@ -156,7 +156,7 @@ {% set index = index + 1 %}
Milestone: {{ milestone }} + {{ milestone }} {% if repo.milestones[milestone] %}   (Due: {{ repo.milestones[milestone] }}) {% endif %} @@ -168,21 +168,11 @@ Modified - Priority (reset) + Priority - Assignee (reset) + Assignee Status
- {% set index = index + 1 %} - - - - - {% if status and status|lower != 'open' %} - - {% else %} - - {% endif %} - - {% if not status or status|lower == 'open' %} - - {% endif %} - - - - - - {% for issue in issues[milestone] |sort(attribute='priority') %} - {% if status is none or (status and issue.status == 'Open') %} - - - - - - - - - {% 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}} - - {{ - issue.last_updated | humanize}} - - {% 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
-
-
-{% else %} +{% if not issues %}
No issues found
+ + + + + {% 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
+
+
+ {% endif %} {% endfor %} {% endblock %} {% block jscripts %} diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index ca1067b..11a3bee 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -683,9 +683,7 @@ def view_roadmap(repo, username=None, namespace=None): """ List all issues associated to a repo as roadmap """ status = flask.request.args.get('status', 'Open') - if status.lower() == 'all': - status = None - milestone = flask.request.args.getlist('milestone', None) + milestones = flask.request.args.getlist('milestone', None) tags = flask.request.args.getlist('tag', None) repo = flask.g.repo @@ -702,53 +700,48 @@ def view_roadmap(repo, username=None, namespace=None): if flask.g.repo_admin: private = None - requested_stones = None - if milestone is not None: - requested_stones = milestone - all_milestones = list(repo.milestones.keys()) + all_milestones = sorted(list(repo.milestones.keys())) issues = pagure.lib.search_issues( SESSION, repo, - milestones=milestone or list(repo.milestones.keys()), + milestones=milestones or all_milestones, tags=tags, private=private, + status=status if status.lower()!= 'all' else None, ) # Change from a list of issues to a dict of milestone/issues milestone_issues = defaultdict(list) - for cnt in range(len(issues)): + for issue in issues: saved = False - for mlstone in sorted(milestone or list(repo.milestones.keys())): - if mlstone == issues[cnt].milestone: - milestone_issues[mlstone].append(issues[cnt]) + for mlstone in sorted(milestones or all_milestones): + if mlstone == issue.milestone: + milestone_issues[mlstone].append(issue) saved = True break if saved: continue - if status: + if status and status.lower() != 'all': for key in milestone_issues.keys(): active = False for issue in milestone_issues[key]: - if issue.status == 'Open': + if issue.status == status: active = True break if not active: del milestone_issues[key] - all_tags = pagure.lib.get_tags_of_project(SESSION, repo) - tag_list = [] - for tag in all_tags: - tag_list.append(tag.tag) + tag_list = [ + tag.tag + for tag in pagure.lib.get_tags_of_project(SESSION, repo) + ] - reponame = pagure.get_repo_path(repo) - repo_obj = pygit2.Repository(reponame) - milestones_ordered = sorted(list(milestone_issues.keys())) - if 'unplanned' in milestones_ordered: - index = milestones_ordered.index('unplanned') - cnt = len(milestones_ordered) - milestones_ordered.insert(cnt, milestones_ordered.pop(index)) + if 'unplanned' in all_milestones: + index = all_milestones.index('unplanned') + cnt = len(all_milestones) + all_milestones.insert(cnt, all_milestones.pop(index)) return flask.render_template( 'roadmap.html', @@ -757,9 +750,8 @@ def view_roadmap(repo, username=None, namespace=None): username=username, tag_list=tag_list, status=status, - milestones=milestones_ordered, - requested_stones=requested_stones, - allmilestones=all_milestones, + milestones=all_milestones, + requested_stones=milestones, issues=milestone_issues, tags=tags, ) From ab586907bfed09f4e6f4adc2891dc5b52c3f8259 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jan 12 2017 15:36:17 +0000 Subject: [PATCH 7/10] Use loop.index to access the index --- diff --git a/pagure/templates/roadmap.html b/pagure/templates/roadmap.html index b06bcd5..39f279a 100644 --- a/pagure/templates/roadmap.html +++ b/pagure/templates/roadmap.html @@ -140,7 +140,7 @@ {% if issues[milestone] %}
- +
{{ milestone }} From a4e61d2b5e38fe6d4baf475db09444e70193d8a8 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jan 12 2017 15:36:17 +0000 Subject: [PATCH 8/10] We do not want the message if nothing changed --- diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index 11a3bee..136cfa4 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -249,7 +249,7 @@ def update_issue(repo, issueid, username=None, namespace=None): ticketfolder=APP.config['TICKETS_FOLDER'], ) SESSION.commit() - if message: + if message and message != 'Nothing to change': messages.add(message) # Update priority From ce9a97544c75da8b6e323b5ae68af9da0cf04445 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jan 12 2017 15:36:35 +0000 Subject: [PATCH 9/10] Attempt to make all the flags on one line and remain as such --- diff --git a/pagure/static/pagure.css b/pagure/static/pagure.css index 4a5936b..695e345 100644 --- a/pagure/static/pagure.css +++ b/pagure/static/pagure.css @@ -547,11 +547,11 @@ blockquote { } .messageicon{ -float: left; -font-size: 1.5em; -padding-right: 0.5em; -margin-top: -4px; -color: #999; + float: left; + font-size: 1.5em; + padding-right: 0.5em; + margin-top: -4px; + color: #999; } .msg-git-hash{ font-family:monospace; @@ -560,7 +560,7 @@ color: #999; } th[data-sort] { - cursor: pointer; + cursor: pointer; } .pr-changes-description @@ -569,24 +569,30 @@ th[data-sort] { } .readme dd{ - margin-left:2em; + margin-left:2em; } .hidden{ - display: none; + display: none; } .code_table .cell_commit{ - padding-left:0.5em; + padding-left:0.5em; } #cal-heatmap { - padding:0.5em; + padding:0.5em; } .attachment_list p { - margin-bottom: 0; + margin-bottom: 0; +} + +.btn-group-sm .btn-nopad, +.btn-nopad { + padding: 0rem .375rem!important; + margin-left: .5rem !important } /*Our specific responsive overrides*/ diff --git a/pagure/templates/roadmap.html b/pagure/templates/roadmap.html index 39f279a..3bae9a5 100644 --- a/pagure/templates/roadmap.html +++ b/pagure/templates/roadmap.html @@ -40,7 +40,7 @@
-
+
All - + {% for tag in tag_list %} {% if tag in tags %} {% endfor %} - + {% for stone in milestones %} {% if stone in requested_stones %} Date: Jan 12 2017 15:36:35 +0000 Subject: [PATCH 10/10] Add icon to status, and move milestones before tags --- diff --git a/pagure/templates/roadmap.html b/pagure/templates/roadmap.html index 3bae9a5..e691e44 100644 --- a/pagure/templates/roadmap.html +++ b/pagure/templates/roadmap.html @@ -42,6 +42,7 @@
+ All - - {% for tag in tag_list %} - {% if tag in tags %} - - {% else %} - - {% endif %} - {{ tag }} - {% endfor %} - - {% for stone in milestones %} {% if stone in requested_stones %} @@ -127,6 +102,32 @@ {% endif %} {% endfor %} + + + {% for tag in tag_list %} + {% if tag in tags %} + + {% else %} + + {% endif %} + {{ tag }} + {% endfor %} +
{% if not issues %}