From ac8e5f54029cadc0906680a85b98859554589791 Mon Sep 17 00:00:00 2001 From: Robert Sparks Date: Mon, 22 May 2017 18:40:05 +0000 Subject: [PATCH] Make it possible to close nominations without closing feedback. Fixes #2255. Commit ready for merge. - Legacy-Id: 13399 --- ietf/nomcom/admin.py | 2 +- ietf/nomcom/factories.py | 2 + ietf/nomcom/forms.py | 19 ++-- ietf/nomcom/managers.py | 8 -- ...3_position_nomination_feedback_switches.py | 34 +++++++ .../0014_alter_is_open_help_text.py | 20 +++++ ietf/nomcom/models.py | 4 +- ietf/nomcom/resources.py | 2 + ietf/nomcom/test_data.py | 4 +- ietf/nomcom/tests.py | 90 +++++++++++++++++++ ietf/nomcom/views.py | 18 +++- ietf/templates/nomcom/list_positions.html | 10 +++ 12 files changed, 193 insertions(+), 20 deletions(-) create mode 100644 ietf/nomcom/migrations/0013_position_nomination_feedback_switches.py create mode 100644 ietf/nomcom/migrations/0014_alter_is_open_help_text.py diff --git a/ietf/nomcom/admin.py b/ietf/nomcom/admin.py index d7456d1ba..b830edd6d 100644 --- a/ietf/nomcom/admin.py +++ b/ietf/nomcom/admin.py @@ -24,7 +24,7 @@ class NomineePositionAdmin(admin.ModelAdmin): class PositionAdmin(admin.ModelAdmin): - list_display = ('name', 'nomcom', 'is_open') + list_display = ('name', 'nomcom', 'is_open', 'accepting_nominations', 'accepting_feedback') list_filter = ('nomcom',) diff --git a/ietf/nomcom/factories.py b/ietf/nomcom/factories.py index d4b9c6a00..5c9c523cc 100644 --- a/ietf/nomcom/factories.py +++ b/ietf/nomcom/factories.py @@ -128,6 +128,8 @@ class PositionFactory(factory.DjangoModelFactory): name = factory.Faker('sentence',nb_words=5) is_open = True + accepting_nominations = True + accepting_feedback = True class NomineeFactory(factory.DjangoModelFactory): class Meta: diff --git a/ietf/nomcom/forms.py b/ietf/nomcom/forms.py index 3bdf768ca..6fff204bc 100644 --- a/ietf/nomcom/forms.py +++ b/ietf/nomcom/forms.py @@ -41,7 +41,7 @@ class PositionNomineeField(forms.ChoiceField): def __init__(self, *args, **kwargs): self.nomcom = kwargs.pop('nomcom') - positions = Position.objects.get_by_nomcom(self.nomcom).opened().order_by('name') + positions = Position.objects.get_by_nomcom(self.nomcom).filter(is_open=True).order_by('name') results = [] for position in positions: accepted_nominees = [np.nominee for np in NomineePosition.objects.filter(position=position,state='accepted').exclude(nominee__duplicated__isnull=False)] @@ -62,7 +62,7 @@ class PositionNomineeField(forms.ChoiceField): return nominee (position_id, nominee_id) = nominee.split('_') try: - position = Position.objects.get_by_nomcom(self.nomcom).opened().get(id=position_id) + position = Position.objects.get_by_nomcom(self.nomcom).filter(is_open=True).get(id=position_id) except Position.DoesNotExist: raise forms.ValidationError('Invalid nominee') try: @@ -82,7 +82,7 @@ class MultiplePositionNomineeField(forms.MultipleChoiceField, PositionNomineeFie return nominee (position_id, nominee_id) = nominee.split('_') try: - position = Position.objects.get_by_nomcom(self.nomcom).opened().get(id=position_id) + position = Position.objects.get_by_nomcom(self.nomcom).filter(is_open=True).get(id=position_id) except Position.DoesNotExist: raise forms.ValidationError('Invalid nominee') try: @@ -356,7 +356,9 @@ class NominateForm(forms.ModelForm): self.fields['searched_email'].help_text = 'Search by name or email address. Click here if the search does not find the candidate you want to nominate.' % reverse(new_person_url_name,kwargs={'year':self.nomcom.year()}) self.fields['nominator_email'].label = 'Nominator email' if self.nomcom: - self.fields['position'].queryset = Position.objects.get_by_nomcom(self.nomcom).opened() + self.fields['position'].queryset = Position.objects.get_by_nomcom(self.nomcom).filter(is_open=True) + if self.public: + self.fields['position'].queryset = self.fields['position'].queryset.filter(accepting_nominations=True) self.fields['qualifications'].help_text = self.nomcom.initial_text if not self.public: @@ -454,7 +456,9 @@ class NominateNewPersonForm(forms.ModelForm): self.fields['nominator_email'].label = 'Nominator email' if self.nomcom: - self.fields['position'].queryset = Position.objects.get_by_nomcom(self.nomcom).opened() + self.fields['position'].queryset = Position.objects.get_by_nomcom(self.nomcom).filter(is_open=True) + if self.public: + self.fields['position'].queryset = self.fields['position'].queryset.filter(accepting_nominations=True) self.fields['qualifications'].help_text = self.nomcom.initial_text if not self.public: @@ -680,7 +684,7 @@ class PositionForm(forms.ModelForm): class Meta: model = Position - fields = ('name', 'is_open') + fields = ('name', 'is_open', 'accepting_nominations', 'accepting_feedback') def __init__(self, *args, **kwargs): self.nomcom = kwargs.pop('nomcom', None) @@ -766,7 +770,7 @@ class MutableFeedbackForm(forms.ModelForm): widget=forms.SelectMultiple(attrs={'class':'nominee_multi_select','size':'12'}), help_text='Hold down "Control", or "Command" on a Mac, to select more than one.') else: - self.fields['position'] = forms.ModelChoiceField(queryset=Position.objects.get_by_nomcom(self.nomcom).opened(), label="Position") + self.fields['position'] = forms.ModelChoiceField(queryset=Position.objects.get_by_nomcom(self.nomcom).filter(is_open=True), label="Position") self.fields['searched_email'] = SearchableEmailField(only_users=False,help_text="Try to find the candidate you are classifying with this field first. Only use the name and email fields below if this search does not find the candidate.",label="Candidate",required=False) self.fields['candidate_name'] = forms.CharField(label="Candidate name",help_text="Only fill in this name field if the search doesn't find the person you are classifying",required=False) self.fields['candidate_email'] = forms.EmailField(label="Candidate email",help_text="Only fill in this email field if the search doesn't find the person you are classifying",required=False) @@ -830,7 +834,6 @@ class MutableFeedbackForm(forms.ModelForm): feedback.positions.add(position) return feedback - FullFeedbackFormSet = forms.modelformset_factory( model=Feedback, extra=0, diff --git a/ietf/nomcom/managers.py b/ietf/nomcom/managers.py index 74806959d..435b21fd3 100644 --- a/ietf/nomcom/managers.py +++ b/ietf/nomcom/managers.py @@ -68,14 +68,6 @@ class PositionQuerySet(QuerySet): def get_by_nomcom(self, nomcom): return self.filter(nomcom=nomcom) - def opened(self): - """ only opened positions """ - return self.filter(is_open=True) - - def closed(self): - """ only closed positions """ - return self.filter(is_open=False) - class PositionManager(models.Manager, MixinManager): def get_queryset(self): diff --git a/ietf/nomcom/migrations/0013_position_nomination_feedback_switches.py b/ietf/nomcom/migrations/0013_position_nomination_feedback_switches.py new file mode 100644 index 000000000..dac5f79c3 --- /dev/null +++ b/ietf/nomcom/migrations/0013_position_nomination_feedback_switches.py @@ -0,0 +1,34 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.10.7 on 2017-05-19 07:58 +from __future__ import unicode_literals + +from django.db import migrations, models +from django.db.models import F + +def forward(apps, schema_editor): + Position = apps.get_model('nomcom','Position') + Position.objects.update(accepting_nominations=F('is_open')) + Position.objects.update(accepting_feedback=F('is_open')) + +def reverse(apps, schema_editor): + pass + +class Migration(migrations.Migration): + + dependencies = [ + ('nomcom', '0012_auto_20170210_0205'), + ] + + operations = [ + migrations.AddField( + model_name='position', + name='accepting_feedback', + field=models.BooleanField(default=False, verbose_name=b'Is accepting feedback'), + ), + migrations.AddField( + model_name='position', + name='accepting_nominations', + field=models.BooleanField(default=False, verbose_name=b'Is accepting nominations'), + ), + migrations.RunPython(forward,reverse) + ] diff --git a/ietf/nomcom/migrations/0014_alter_is_open_help_text.py b/ietf/nomcom/migrations/0014_alter_is_open_help_text.py new file mode 100644 index 000000000..16ebf96d2 --- /dev/null +++ b/ietf/nomcom/migrations/0014_alter_is_open_help_text.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.10.7 on 2017-05-22 08:19 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('nomcom', '0013_position_nomination_feedback_switches'), + ] + + operations = [ + migrations.AlterField( + model_name='position', + name='is_open', + field=models.BooleanField(default=False, help_text=b'Set is_open when the nomcom is working on a position. Clear it when an appointment is confirmed.', verbose_name=b'Is open'), + ), + ] diff --git a/ietf/nomcom/models.py b/ietf/nomcom/models.py index 318bd0df2..4a8e2f6b8 100644 --- a/ietf/nomcom/models.py +++ b/ietf/nomcom/models.py @@ -163,7 +163,9 @@ class Position(models.Model): name = models.CharField(verbose_name='Name', max_length=255, help_text='This short description will appear on the Nomination and Feedback pages. Be as descriptive as necessary. Past examples: "Transport AD", "IAB Member"') requirement = models.ForeignKey(DBTemplate, related_name='requirement', null=True, editable=False) questionnaire = models.ForeignKey(DBTemplate, related_name='questionnaire', null=True, editable=False) - is_open = models.BooleanField(verbose_name='Is open', default=False) + is_open = models.BooleanField(verbose_name='Is open', default=False, help_text="Set is_open when the nomcom is working on a position. Clear it when an appointment is confirmed.") + accepting_nominations = models.BooleanField(verbose_name='Is accepting nominations', default=False) + accepting_feedback = models.BooleanField(verbose_name='Is accepting feedback', default=False) objects = PositionManager() diff --git a/ietf/nomcom/resources.py b/ietf/nomcom/resources.py index ec8c98a52..9de9b8249 100644 --- a/ietf/nomcom/resources.py +++ b/ietf/nomcom/resources.py @@ -42,6 +42,8 @@ class PositionResource(ModelResource): "id": ALL, "name": ALL, "is_open": ALL, + "accepting_nominations": ALL, + "accepting_feedback": ALL, "nomcom": ALL_WITH_RELATIONS, "requirement": ALL_WITH_RELATIONS, "questionnaire": ALL_WITH_RELATIONS, diff --git a/ietf/nomcom/test_data.py b/ietf/nomcom/test_data.py index bee016d35..d0cad55aa 100644 --- a/ietf/nomcom/test_data.py +++ b/ietf/nomcom/test_data.py @@ -130,7 +130,9 @@ def nomcom_test_data(): for name in POSITIONS: position, created = Position.objects.get_or_create(nomcom=nomcom, name=name, - is_open=True) + is_open=True, + accepting_nominations=True, + accepting_feedback=True) ChangeStateGroupEvent.objects.get_or_create(group=group, type="changed_state", diff --git a/ietf/nomcom/tests.py b/ietf/nomcom/tests.py index 973208a85..28865f581 100644 --- a/ietf/nomcom/tests.py +++ b/ietf/nomcom/tests.py @@ -1771,3 +1771,93 @@ class MergePersonTests(TestCase): self.assertEqual(self.nc.nominee_set.get(pk=self.nominee2.pk).feedback_set.count(),2) self.assertFalse(self.nc.nominee_set.filter(pk=self.nominee1.pk).exists()) +class AcceptingTests(TestCase): + def setUp(self): + build_test_public_keys_dir(self) + self.nc = NomComFactory(**nomcom_kwargs_for_year()) + self.plain_person = PersonFactory.create() + self.member = self.nc.group.role_set.filter(name='member').first().person + + def tearDown(self): + clean_test_public_keys_dir(self) + + def test_public_accepting_nominations(self): + url = reverse('ietf.nomcom.views.public_nominate',kwargs={'year':self.nc.year()}) + + login_testing_unauthorized(self,self.plain_person.user.username,url) + response = self.client.get(url) + q=PyQuery(response.content) + self.assertEqual( len(q('#id_position option')) , 4 ) + + pos = self.nc.position_set.first() + pos.accepting_nominations=False + pos.save() + + response = self.client.get(url) + q=PyQuery(response.content) + self.assertEqual( len(q('#id_position option')) , 3 ) + + def test_private_accepting_nominations(self): + url = reverse('ietf.nomcom.views.private_nominate',kwargs={'year':self.nc.year()}) + + login_testing_unauthorized(self,self.member.user.username,url) + response = self.client.get(url) + q=PyQuery(response.content) + self.assertEqual( len(q('#id_position option')) , 4 ) + + pos = self.nc.position_set.first() + pos.accepting_nominations=False + pos.save() + + response = self.client.get(url) + q=PyQuery(response.content) + self.assertEqual( len(q('#id_position option')) , 4 ) + + def test_public_accepting_feedback(self): + url = reverse('ietf.nomcom.views.public_feedback',kwargs={'year':self.nc.year()}) + + login_testing_unauthorized(self,self.plain_person.user.username,url) + response = self.client.get(url) + q=PyQuery(response.content) + self.assertEqual( len(q('.badge')) , 3 ) + + pos = self.nc.position_set.first() + pos.accepting_feedback=False + pos.save() + + response = self.client.get(url) + q=PyQuery(response.content) + self.assertEqual( len(q('.badge')) , 2 ) + + url += "?nominee=%d&position=%d" % (pos.nominee_set.first().pk, pos.pk) + response = self.client.get(url) + self.assertTrue('not currently accepting feedback' in unicontent(response)) + + test_data = {'comments': 'junk', + 'position_name': pos.name, + 'nominee_name': pos.nominee_set.first().email.person.name, + 'nominee_email': pos.nominee_set.first().email.address, + 'confirmation': False, + 'nominator_email': self.plain_person.email().address, + 'nominator_name': self.plain_person.plain_name(), + } + response = self.client.post(url, test_data) + self.assertTrue('not currently accepting feedback' in unicontent(response)) + + def test_private_accepting_feedback(self): + url = reverse('ietf.nomcom.views.private_feedback',kwargs={'year':self.nc.year()}) + + login_testing_unauthorized(self,self.member.user.username,url) + response = self.client.get(url) + q=PyQuery(response.content) + self.assertEqual( len(q('.badge')) , 3 ) + + pos = self.nc.position_set.first() + pos.accepting_feedback=False + pos.save() + + response = self.client.get(url) + q=PyQuery(response.content) + self.assertEqual( len(q('.badge')) , 3 ) + + diff --git a/ietf/nomcom/views.py b/ietf/nomcom/views.py index b970a3156..fbb3d85ff 100644 --- a/ietf/nomcom/views.py +++ b/ietf/nomcom/views.py @@ -173,6 +173,7 @@ def private_index(request, year): if selected_state == questionnaire_state: nominee_positions = [np for np in nominee_positions if np.questionnaires] + # TODO- just build this dict using open Positions: see #2254 stats = all_nominee_positions.values('position__name', 'position__id').annotate(total=Count('position')) states = list(NomineePositionStateName.objects.values('slug', 'name')) + [{'slug': questionnaire_state, 'name': u'Questionnaire'}] positions = set([ n.position for n in all_nominee_positions.order_by('position__name') ]) @@ -421,7 +422,10 @@ def feedback(request, year, public): nominee = get_object_or_404(Nominee, id=selected_nominee) position = get_object_or_404(Position, id=selected_position) - positions = Position.objects.get_by_nomcom(nomcom=nomcom).opened() + if public: + positions = Position.objects.get_by_nomcom(nomcom=nomcom).filter(is_open=True,accepting_feedback=True) + else: + positions = Position.objects.get_by_nomcom(nomcom=nomcom).filter(is_open=True) user_comments = Feedback.objects.filter(nomcom=nomcom, type='comment', @@ -445,6 +449,18 @@ def feedback(request, year, public): 'base_template': base_template }) + if public and position and not (position.is_open and position.accepting_feedback): + messages.warning(request, "This Nomcom is not currently accepting feedback for "+position.name) + return render(request, 'nomcom/feedback.html', { + 'form': None, + 'nomcom': nomcom, + 'year': year, + 'selected': 'feedback', + 'positions': positions, + 'counts' : counts, + 'base_template': base_template + }) + if nominee and position and request.method == 'POST': form = FeedbackForm(data=request.POST, nomcom=nomcom, user=request.user, diff --git a/ietf/templates/nomcom/list_positions.html b/ietf/templates/nomcom/list_positions.html index 46810234f..89d10b83d 100644 --- a/ietf/templates/nomcom/list_positions.html +++ b/ietf/templates/nomcom/list_positions.html @@ -21,6 +21,16 @@
{% for position in group.list %}

{{ position.name }}

+ {% if group.grouper %} +
+
Accepting
+
+ {% if position.accepting_nominations %}Nominations{% endif %} + {% if position.accepting_nominations and position.accepting_feedback %}and{% endif %} + {% if position.accepting_feedback %}Feedback{% endif %} +
+
+ {% endif %}
Templates