From 0619e565ee26d0ed72e14f852c9bdc4a24ad9458 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jan 31 2018 12:38:38 +0000 Subject: Add the possibility to mark milestones as active or inactive By default only the active milestones are shown in the UI but we offer a button to show them all. And tickets can only be assigned to active milestones. Fixes https://pagure.io/pagure/issue/2123 Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/lib/model.py b/pagure/lib/model.py index 837c549..87cd79d 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -550,6 +550,11 @@ class Project(BASE): if self._milestones: milestones = json.loads(self._milestones) + for k in milestones: + if not isinstance(milestones[k], dict): + milestones[k] = {'date': milestones[k], 'active': True} + else: + break return milestones diff --git a/pagure/static/pagure.css b/pagure/static/pagure.css index d6a5931..eabf5b7 100644 --- a/pagure/static/pagure.css +++ b/pagure/static/pagure.css @@ -615,7 +615,7 @@ th[data-sort] { margin-left:2em; } -.hidden{ +.hidden, .milestone_inactive{ display: none; } diff --git a/pagure/templates/roadmap.html b/pagure/templates/roadmap.html index 8ec1ec0..a7ed9b9 100644 --- a/pagure/templates/roadmap.html +++ b/pagure/templates/roadmap.html @@ -13,8 +13,9 @@ {{ title }} - {% if milestone and repo.milestones[milestone] %} -   (Due: {{ repo.milestones[milestone] }}) + {% if milestone and repo.milestones[milestone]['date'] %} +   (Due: {{ + repo.milestones[milestone]['date'] }}) {% endif %} Opened diff --git a/pagure/templates/settings.html b/pagure/templates/settings.html index 73f9010..5d30ac5 100644 --- a/pagure/templates/settings.html +++ b/pagure/templates/settings.html @@ -1249,6 +1249,25 @@ $('.extend-form').click(function(e) { const tgt = $(this).attr('data-target'); let form = $(tgt + ' > div:last-child').clone(); form.find('input[type=text], textarea').val(''); + if (tgt == '#milestones'){ + var _b = $(form.find('.milestone_order_up')); + const idx = parseInt(_b.attr('data-stone')); + + form.removeClass('milestone_inactive'); + form.attr('id', 'milestone_' + (idx + 1 )); + + _b.attr('data-stone', (idx + 1)) + + var _b2 = $(form.find('.milestone_order_bottom')); + _b2.attr('data-stone', (idx + 1)) + + var _d = form.find('input[name=milestone_date_' + idx + ']'); + $(_d).attr('name', 'milestone_date_' + (idx + 1 )); + + var _a = form.find('input[name=active_milestone_' + idx + ']'); + $(_a).attr('name', 'active_milestone_' + (idx + 1 )); + $(_a).prop('checked', true); + } $(tgt).append(form); }); @@ -1268,6 +1287,17 @@ $('.milestone_order_bottom').click(function(e) { } }); +$('#milestone_toggle').click(function(e) { + var _b = $('#milestone_toggle'); + if (_b.text().trim() == 'View all milestones'){ + $('.milestone_inactive').show(); + _b.text('Active milestones only'); + } else { + $('.milestone_inactive').hide(); + _b.text('View all milestones'); + } +}); + {% if config.get('ENABLE_GIVE_PROJECTS', True) and repo.user.user == g.fas_user.username and not repo.is_fork %} diff --git a/pagure/templates/settings_milestones.html b/pagure/templates/settings_milestones.html index 90e42fa..5fc947a 100644 --- a/pagure/templates/settings_milestones.html +++ b/pagure/templates/settings_milestones.html @@ -19,24 +19,28 @@ {{ tag_form.csrf_token }}
-
+
Milestone
-
+
Date (optional)
+
+
{% for milestone in (repo.milestones_keys or repo.milestones or [""]) %} -
-
+
+
-
- +
@@ -47,15 +51,26 @@ data-stone="{{ loop.index }}" data-glyph="arrow-thick-bottom">
+
+ +
{% endfor %}
- + Add new milestone
+
+ + View all milestones + +
diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index 51676b6..46eda3f 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -1041,11 +1041,15 @@ def view_issue(repo, issueid, username=None, namespace=None): flask.abort(404, 'Issue not found') status = pagure.lib.get_issue_statuses(flask.g.session) + milestones = [] + for m in repo.milestones: + if repo.milestones[m]['active']: + milestones.append(m) form = pagure.forms.UpdateIssueForm( status=status, priorities=repo.priorities, - milestones=repo.milestones_keys or repo.milestones or None, + milestones=milestones or None, close_status=repo.close_status, ) form.status.data = issue.status diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index 625eec0..2ac19c7 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -1306,20 +1306,11 @@ def update_milestones(repo, username=None, namespace=None): if form.validate_on_submit(): redirect = flask.request.args.get('from') milestones = [ - w.strip() for w in flask.request.form.getlist('milestones') - ] - - milestone_dates = [ - p.strip() for p in flask.request.form.getlist('milestone_dates') + w.strip() + for w in flask.request.form.getlist('milestones') ] - if len(milestones) != len(milestone_dates): - flask.flash( - 'Milestones and dates are not of the same length', - 'error') - error = True - - for milestone in milestones: + for cnt, milestone in enumerate(milestones): if milestone.strip() and milestones.count(milestone) != 1: flask.flash( 'Milestone %s is present %s times' % ( @@ -1329,25 +1320,24 @@ def update_milestones(repo, username=None, namespace=None): error = True break - for milestone_date in milestone_dates: - if milestone_date.strip() \ - and milestone_dates.count(milestone_date) != 1: - flask.flash( - 'Date %s is present %s times' % ( - milestone_date, milestone_dates.count(milestone_date) - ), - 'error') - error = True - break - + keys = [] if not error: miles = {} for cnt in range(len(milestones)): + active = flask.request.form.get( + 'active_milestone_%s' % (cnt + 1), False) + date = flask.request.form.get( + 'milestone_date_%s' % (cnt + 1), None) + if milestones[cnt].strip(): - miles[milestones[cnt]] = milestone_dates[cnt] + miles[milestones[cnt]] = { + 'date': date.strip() if date else None, + 'active': active, + } + keys.append(milestones[cnt]) try: repo.milestones = miles - repo.milestones_keys = milestones + repo.milestones_keys = keys flask.g.session.add(repo) flask.g.session.commit() flask.flash('Milestones updated') diff --git a/tests/test_pagure_flask_ui_repo.py b/tests/test_pagure_flask_ui_repo.py index 4196664..922b6f2 100644 --- a/tests/test_pagure_flask_ui_repo.py +++ b/tests/test_pagure_flask_ui_repo.py @@ -1316,12 +1316,12 @@ class PagureFlaskRepotests(tests.Modeltests): self.assertIn( '''
-
+
-
- +
@@ -1332,6 +1332,9 @@ class PagureFlaskRepotests(tests.Modeltests): data-stone="1" data-glyph="arrow-thick-bottom">
+
+ +
''', output.data) # Check that the close_status have its empty field diff --git a/tests/test_pagure_flask_ui_repo_milestones.py b/tests/test_pagure_flask_ui_repo_milestones.py new file mode 100644 index 0000000..118bd64 --- /dev/null +++ b/tests/test_pagure_flask_ui_repo_milestones.py @@ -0,0 +1,188 @@ +# -*- coding: utf-8 -*- + +""" + (c) 2018 - Copyright Red Hat Inc + + Authors: + Pierre-Yves Chibon + +""" + +__requires__ = ['SQLAlchemy >= 0.8'] +import pkg_resources + +import sys +import unittest +import os + +from mock import ANY, patch, MagicMock + +sys.path.insert(0, os.path.join(os.path.dirname( + os.path.abspath(__file__)), '..')) + +import pagure +import pagure.lib +import tests + + +class PagureFlaskRepoMilestonestests(tests.Modeltests): + """ Tests for milestones in pagure """ + + @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) + def setUp(self): + """ Set up the environnment, ran before every tests. """ + super(PagureFlaskRepoMilestonestests, self).setUp() + + tests.create_projects(self.session) + tests.create_projects_git(os.path.join(self.path, 'repos')) + + repo = pagure.lib.get_authorized_project(self.session, 'test') + msg = pagure.lib.new_issue( + session=self.session, + repo=repo, + title='Test issue #1', + content='We should work on this for the second time', + user='foo', + status='Open', + ticketfolder=None + ) + self.session.commit() + self.assertEqual(msg.title, 'Test issue #1') + + @patch('pagure.decorators.admin_session_timedout', + MagicMock(return_value=False)) + def test_milestones_settings_empty(self): + """ Test the settings page when no milestones are set. """ + + repo = pagure.lib.get_authorized_project(self.session, 'test') + self.assertEqual(repo.milestones, {}) + + user = tests.FakeUser(username='pingou') + with tests.user_set(self.app.application, user): + output = self.app.get('/test/settings') + self.assertEqual(output.status_code, 200) + self.assertIn( + 'Settings - test - Pagure', output.data) + # Check that the milestones have their empty fields + self.assertIn( + '''
+
+
+ +
+
+ +
+
+ + +
+
+ +
+
''', output.data) + + @patch('pagure.decorators.admin_session_timedout', + MagicMock(return_value=False)) + def test_setting_retrieving_milestones(self): + """ Test setting and retrieving milestones off a project. """ + + repo = pagure.lib.get_authorized_project(self.session, 'test') + + milestones = { + '1.0': None, + '1.1': None, + '1.2': '2018-12-31', + '2.0': '2019', + '3.0': 'future', + '4.0': None, + } + repo.milestones = milestones + self.session.add(repo) + self.session.commit() + + self.assertEqual( + repo.milestones, + { + u'1.0': {'active': True, 'date': None}, + u'1.1': {'active': True, 'date': None}, + u'1.2': {'active': True, 'date': u'2018-12-31'}, + u'2.0': {'active': True, 'date': u'2019'}, + u'3.0': {'active': True, 'date': u'future'}, + u'4.0': {'active': True, 'date': None}, + } + ) + + @patch('pagure.decorators.admin_session_timedout', + MagicMock(return_value=False)) + def test_issue_page_milestone_actives(self): + """ Test viewing tickets on a project having milestones, all active. + """ + + repo = pagure.lib.get_authorized_project(self.session, 'test') + + milestones = { + '1.0': None, + '2.0': '2019', + '3.0': 'future', + } + milestones_keys = ['1.0', '3.0', '2.0'] + repo.milestones = milestones + repo.milestones_keys = milestones_keys + self.session.add(repo) + self.session.commit() + + user = tests.FakeUser(username='pingou') + with tests.user_set(self.app.application, user): + output = self.app.get('/test/issue/1') + self.assertEqual(output.status_code, 200) + self.assertIn( + u'', output.data + ) + + @patch('pagure.decorators.admin_session_timedout', + MagicMock(return_value=False)) + def test_issue_page_milestone_actives(self): + """ Test viewing tickets on a project having milestones, all active. + """ + + repo = pagure.lib.get_authorized_project(self.session, 'test') + + milestones = { + '1.0': {'date': None, 'active': False}, + '2.0': {'date': '2018-01-01', 'active': False}, + '3.0': {'date': '2025-01-01', 'active': True}, + '4.0': {'date': 'future', 'active': True}, + } + milestones_keys = ['1.0', '2.0', '3.0', '4.0'] + repo.milestones = milestones + repo.milestones_keys = milestones_keys + self.session.add(repo) + self.session.commit() + + user = tests.FakeUser(username='pingou') + with tests.user_set(self.app.application, user): + output = self.app.get('/test/issue/1') + self.assertEqual(output.status_code, 200) + self.assertIn( + u'', output.data + ) + + +if __name__ == '__main__': + unittest.main(verbosity=2) diff --git a/tests/test_pagure_flask_ui_roadmap.py b/tests/test_pagure_flask_ui_roadmap.py index 5df563a..4cf796a 100644 --- a/tests/test_pagure_flask_ui_roadmap.py +++ b/tests/test_pagure_flask_ui_roadmap.py @@ -196,11 +196,12 @@ class PagureFlaskRoadmaptests(tests.Modeltests): # Check the result of the action -- Milestones recorded self.session = pagure.lib.create_session(self.dbpath) repo = pagure.lib.get_authorized_project(self.session, 'test') - self.assertEqual(repo.milestones, {u'1': u'Tomorrow'}) + self.assertEqual(repo.milestones, {u'1': {u'active': False, u'date': None}}) data = { 'milestones': ['v1.0', 'v2.0'], - 'milestone_dates': ['Tomorrow', ''], + 'milestone_dates_1': 'Tomorrow', + 'milestone_dates_2': '', 'csrf_token': csrf_token, } output = self.app.post( @@ -215,13 +216,19 @@ class PagureFlaskRoadmaptests(tests.Modeltests): self.session = pagure.lib.create_session(self.dbpath) repo = pagure.lib.get_authorized_project(self.session, 'test') self.assertEqual( - repo.milestones, {u'v1.0': u'Tomorrow', u'v2.0': u''} + repo.milestones, + { + u'v1.0': {u'active': False, u'date': None}, + u'v2.0': {u'active': False, u'date': None} + } ) # Check error - less milestones than dates data = { 'milestones': ['v1.0', 'v2.0'], - 'milestone_dates': ['Tomorrow', 'Next week', 'Next Year'], + 'milestone_date_1': 'Tomorrow', + 'milestone_date_2': 'Next week', + 'milestone_date_3': 'Next Year', 'csrf_token': csrf_token, } output = self.app.post( @@ -231,21 +238,23 @@ class PagureFlaskRoadmaptests(tests.Modeltests): self.assertIn( u'Settings - test - Pagure', output.data) self.assertIn(u'

Settings for test

', output.data) - self.assertIn( - u'\n' - ' Milestones and dates are not of the ' - 'same length', output.data) # Check the result of the action -- Milestones un-changed self.session = pagure.lib.create_session(self.dbpath) repo = pagure.lib.get_authorized_project(self.session, 'test') self.assertEqual( - repo.milestones, {u'v1.0': u'Tomorrow', u'v2.0': u''} + repo.milestones, + { + u'v1.0': {u'active': False, u'date': u'Tomorrow'}, + u'v2.0': {u'active': False, u'date': u'Next week'} + } ) # Check error - Twice the same milestone data = { 'milestones': ['v1.0', 'v2.0', 'v2.0'], - 'milestone_dates': ['Tomorrow', 'Next week', 'Next Year'], + 'milestone_date_1': 'Tomorrow', + 'milestone_date_2': 'Next week', + 'milestone_date_3': 'Next Year', 'csrf_token': csrf_token, } output = self.app.post( @@ -263,13 +272,19 @@ class PagureFlaskRoadmaptests(tests.Modeltests): self.session = pagure.lib.create_session(self.dbpath) repo = pagure.lib.get_authorized_project(self.session, 'test') self.assertEqual( - repo.milestones, {u'v1.0': u'Tomorrow', u'v2.0': u''} + repo.milestones, + { + u'v1.0': {u'active': False, u'date': u'Tomorrow'}, + u'v2.0': {u'active': False, u'date': u'Next week'} + } ) # Check error - Twice the same date data = { 'milestones': ['v1.0', 'v2.0', 'v3.0'], - 'milestone_dates': ['Tomorrow', 'Next week', 'Next week'], + 'milestone_date_1': 'Tomorrow', + 'milestone_date_2': 'Next week', + 'milestone_date_3': 'Next week', 'csrf_token': csrf_token, } output = self.app.post( @@ -281,13 +296,18 @@ class PagureFlaskRoadmaptests(tests.Modeltests): self.assertIn(u'

Settings for test

', output.data) self.assertIn( u'\n' - ' Date Next week is present 2 times', + ' Milestones updated', output.data) - # Check the result of the action -- Milestones un-changed + # Check the result of the action -- Milestones updated self.session = pagure.lib.create_session(self.dbpath) repo = pagure.lib.get_authorized_project(self.session, 'test') self.assertEqual( - repo.milestones, {u'v1.0': u'Tomorrow', u'v2.0': u''} + repo.milestones, + { + u'v1.0': {u'active': False, u'date': u'Tomorrow'}, + u'v2.0': {u'active': False, u'date': u'Next week'}, + u'v3.0': {u'active': False, u'date': u'Next week'}, + } ) # Check for an invalid project @@ -350,7 +370,13 @@ class PagureFlaskRoadmaptests(tests.Modeltests): # Check the result of the action -- Milestones recorded self.session = pagure.lib.create_session(self.dbpath) repo = pagure.lib.get_authorized_project(self.session, 'test') - self.assertEqual(repo.milestones, {u'v1.0': u'', u'v2.0': u''}) + self.assertEqual( + repo.milestones, + { + u'v1.0': {u'active': False, u'date': None}, + u'v2.0': {u'active': False, u'date': None} + } + ) @patch('pagure.lib.git.update_git') @patch('pagure.lib.notify.send_email') @@ -378,7 +404,9 @@ class PagureFlaskRoadmaptests(tests.Modeltests): # Create an unplanned milestone data = { 'milestones': ['v1.0', 'v2.0', 'unplanned'], - 'milestone_dates': ['Tomorrow', '', ''], + 'milestone_date_1': 'Tomorrow', + 'milestone_date_2': '', + 'milestone_date_3': '', 'csrf_token': csrf_token, } output = self.app.post( @@ -395,7 +423,9 @@ class PagureFlaskRoadmaptests(tests.Modeltests): self.assertEqual( repo.milestones, { - u'v1.0': u'Tomorrow', u'v2.0': u'', u'unplanned': u'' + u'unplanned': {u'active': False, u'date': None}, + u'v1.0': {u'active': False, u'date': u'Tomorrow'}, + u'v2.0': {u'active': False, u'date': None} } )