#2917 Add the possibility to mark milestones as active or inactive
Merged 6 years ago by pingou. Opened 6 years ago by pingou.

file modified
+5
@@ -550,6 +550,11 @@ 

  

          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

  

file modified
+1 -1
@@ -615,7 +615,7 @@ 

    margin-left:2em;

  }

  

- .hidden{

+ .hidden, .milestone_inactive{

    display: none;

  }

  

@@ -13,8 +13,9 @@ 

        <thead class="thead-default">

          <tr>

            <th>{{ title }}

-             {% if milestone and repo.milestones[milestone] %}

-               <span style="font-weight:normal">&nbsp (Due: {{ repo.milestones[milestone] }})</span>

+             {% if milestone and repo.milestones[milestone]['date'] %}

+               <span style="font-weight:normal">&nbsp (Due: {{

+                 repo.milestones[milestone]['date'] }})</span>

              {% endif %}

            </th>

            <th class="open_date">Opened</th>

@@ -1249,6 +1249,25 @@ 

    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_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 %}

@@ -19,24 +19,28 @@ 

            {{ tag_form.csrf_token }}

            <div class="card-block">

              <div class="row">

-               <div class="col-sm-6">

+               <div class="col-sm-4">

                  <strong>Milestone</strong>

                </div>

-               <div class="col-sm-6">

+               <div class="col-sm-4">

                  <strong>Date (optional)</strong>

                </div>

+               <div class="col-sm-4">

+               </div>

              </div>

              <div id="milestones">

            {% for milestone in (repo.milestones_keys or repo.milestones or [""]) %}

-               <div class="row p-t-1 milestone" id="milestone_{{ loop.index }}">

-                 <div class="col-sm-5 p-r-0">

+               <div class="row p-t-1 milestone{%if milestone and

+               not repo.milestones[milestone]['active'] %} milestone_inactive {%

+               endif %}" id="milestone_{{ loop.index }}">

+                 <div class="col-sm-4 p-r-0">

                    <input type="text" name="milestones"

                      value="{{ milestone }}" size="3" class="form-control"/>

                  </div>

-                 <div class="col-sm-5 p-r-0">

-                   <input type="text" name="milestone_dates"

-                     value="{{ repo.milestones[milestone]

-                         if repo.milestones[milestone] is not none

+                 <div class="col-sm-4 p-r-0">

+                   <input type="text" name="milestone_date_{{ loop.index }}"

+                     value="{{ repo.milestones[milestone]['date']

+                         if milestone and repo.milestones[milestone]['date'] is not none

                      }}" class="form-control"/>

                  </div>

                  <div class="col-sm-2 p-r-0" >
@@ -47,15 +51,26 @@ 

                          data-stone="{{ loop.index }}"

                          data-glyph="arrow-thick-bottom"></span>

                  </div>

+                 <div class="col-sm-1 p-r-0" >

+                     <input type="checkbox" name="active_milestone_{{ loop.index

+                     }}"{% if not milestone or repo.milestones[milestone]['active']

+                     %} checked{% endif %} />

+                 </div>

                </div>

            {% endfor %}

            </div>

            <div class="row p-t-1">

              <div class="col-sm-6">

-               <a class="btn btn-secondary btn-sm btn-block extend-form" data-target="#milestones">

+               <a class="btn btn-secondary btn-sm btn-block extend-form"

+                 data-target="#milestones">

                    Add new milestone

                </a>

              </div>

+             <div class="col-sm-6">

+               <a class="btn btn-secondary btn-sm btn-block" id="milestone_toggle">

+                   View all milestones

+               </a>

+             </div>

            </div>

            <div class="row p-t-1">

              <div class="col-sm-12">

file modified
+5 -1
@@ -1041,11 +1041,15 @@ 

              flask.abort(404, 'Issue not found')

  

      status = pagure.lib.get_issue_statuses(flask.g.session)

+     milestones = []

+     for m in repo.milestones_keys or 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

file modified
+15 -25
@@ -1306,20 +1306,11 @@ 

      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 @@ 

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

@@ -1316,12 +1316,12 @@ 

              self.assertIn(

              '''<div id="milestones">

                <div class="row p-t-1 milestone" id="milestone_1">

-                 <div class="col-sm-5 p-r-0">

+                 <div class="col-sm-4 p-r-0">

                    <input type="text" name="milestones"

                      value="" size="3" class="form-control"/>

                  </div>

-                 <div class="col-sm-5 p-r-0">

-                   <input type="text" name="milestone_dates"

+                 <div class="col-sm-4 p-r-0">

+                   <input type="text" name="milestone_date_1"

                      value="" class="form-control"/>

                  </div>

                  <div class="col-sm-2 p-r-0" >
@@ -1332,6 +1332,9 @@ 

                          data-stone="1"

                          data-glyph="arrow-thick-bottom"></span>

                  </div>

+                 <div class="col-sm-1 p-r-0" >

+                     <input type="checkbox" name="active_milestone_1" checked />

+                 </div>

                </div>''', output.data)

  

              # Check that the close_status have its empty field

@@ -0,0 +1,189 @@ 

+ # -*- coding: utf-8 -*-

+ 

+ """

+  (c) 2018 - Copyright Red Hat Inc

+ 

+  Authors:

+    Pierre-Yves Chibon <pingou@pingoured.fr>

+ 

+ """

+ 

+ __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(

+                 '<title>Settings - test - Pagure</title>', output.data)

+             # Check that the milestones have their empty fields

+             self.assertIn(

+             '''<div id="milestones">

+               <div class="row p-t-1 milestone" id="milestone_1">

+                 <div class="col-sm-4 p-r-0">

+                   <input type="text" name="milestones"

+                     value="" size="3" class="form-control"/>

+                 </div>

+                 <div class="col-sm-4 p-r-0">

+                   <input type="text" name="milestone_date_1"

+                     value="" class="form-control"/>

+                 </div>

+                 <div class="col-sm-2 p-r-0" >

+                     <span class="oi milestone_order_up"

+                         data-stone="1"

+                         data-glyph="arrow-thick-top"></span>

+                     <span class="oi milestone_order_bottom"

+                         data-stone="1"

+                         data-glyph="arrow-thick-bottom"></span>

+                 </div>

+                 <div class="col-sm-1 p-r-0" >

+                     <input type="checkbox" name="active_milestone_1" checked />

+                 </div>

+               </div>''', 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'<select class="form-control c-select" id="milestone" name="milestone">'

+                 u'<option selected value=""></option>'

+                 u'<option value="1.0">1.0</option>'

+                 u'<option value="3.0">3.0</option>'

+                 u'<option value="2.0">2.0</option>'

+                 u'</select>', output.data

+             )

+ 

+     @patch('pagure.decorators.admin_session_timedout',

+            MagicMock(return_value=False))

+     def test_issue_page_milestone_not_allactives(self):

+         """ Test viewing tickets on a project having milestones, not all

+         being 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'<select class="form-control c-select" id="milestone" name="milestone">'

+                 u'<option selected value=""></option>'

+                 u'<option value="3.0">3.0</option>'

+                 u'<option value="4.0">4.0</option>'

+                 u'</select>', output.data

+             )

+ 

+ 

+ if __name__ == '__main__':

+     unittest.main(verbosity=2)

@@ -196,11 +196,12 @@ 

              # 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 @@ 

              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 @@ 

              self.assertIn(

                  u'<title>Settings - test - Pagure</title>', output.data)

              self.assertIn(u'<h3>Settings for test</h3>', output.data)

-             self.assertIn(

-                 u'</button>\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 @@ 

              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 @@ 

              self.assertIn(u'<h3>Settings for test</h3>', output.data)

              self.assertIn(

                  u'</button>\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 @@ 

              # 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 @@ 

              # 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 @@ 

              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}

                  }

              )

  

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 pingou@pingoured.fr

Still needs some more tests but gives us a basis to review

rebased onto 9b1821ea32ba13af7d3b912684cfa1a75d760d93

6 years ago

Alright, ready for review :)

rebased onto 69d6b8f168c0ee56188d14bf6fc7e97ea51979ba

6 years ago

doc string is wrong here, it should be "not all active" or something like that

I would add an "Active" label on top of the tick box in the setting so it is obvious what the tick box is used for.

Also after a quick test it seems possible to assign an inactive milestone to an issue.

rebased onto c63e078fd2c8ef6ece15d89e7c602ae5ea518db4

6 years ago

docstring and function name as well, thanks

rebased onto 9822ab1eee8cc09a38197e565c47e263b5bc4af0

6 years ago

Also after a quick test it seems possible to assign an inactive milestone to an issue.

Via the UI or the API?
In the UI they are not shown, via the API that sounds right indeed.

rebased onto 0619e56

6 years ago

Ok tested again and inactive milestones are not available in the issue metadata form

:thumbsup:

Pull-Request has been merged by pingou

6 years ago