From 9addf4271b29c5c10026db62dcd4cd591281bd77 Mon Sep 17 00:00:00 2001 From: alonisser Date: Sat, 1 Oct 2016 23:42:52 +0300 Subject: [PATCH] resolve #724 non current mk appears in committee page if was replacement or chairperson --- committees/models.py | 4 +- committees/tests/base.py | 95 ++++++++++++++++++ .../tests/test_committee_detail_view.py | 45 ++++++++- .../test_committee_meeting_detail_view.py | 98 +++---------------- committees/views.py | 8 +- 5 files changed, 152 insertions(+), 98 deletions(-) create mode 100644 committees/tests/base.py diff --git a/committees/models.py b/committees/models.py index 063098af..529be137 100644 --- a/committees/models.py +++ b/committees/models.py @@ -132,8 +132,8 @@ def filter_this_year(res_set): members = list(Member.objects.filter(id__in=ids)) else: members = list((self.members.filter(is_current=True) | - self.chairpersons.all() | - self.replacements.all()).distinct()) + self.chairpersons.filter(is_current=True) | + self.replacements.filter(is_current=True)).distinct()) d = Knesset.objects.current_knesset().start_date if from_date is None else from_date meetings_with_mks = self.meetings.filter( diff --git a/committees/tests/base.py b/committees/tests/base.py new file mode 100644 index 00000000..5b908cb1 --- /dev/null +++ b/committees/tests/base.py @@ -0,0 +1,95 @@ +# -*- coding: utf-8 -* +from django.core.urlresolvers import reverse +from django.test import TestCase + +from lobbyists.models import Lobbyist +from persons.models import Person, PersonAlias + +just_id = lambda x: x.id + + +class BaseCommitteeTestCase(TestCase): + def setUp(self): + super(BaseCommitteeTestCase, self).setUp() + + def tearDown(self): + super(BaseCommitteeTestCase, self).tearDown() + + def verify_expected_members_in_context(self, res, expected_mks): + members = res.context['members'] + self.assertEqual(map(just_id, members), + expected_mks, + 'members has wrong objects: %s' % members) + + def verify_unexpected_members_not_in_context(self, res, unexpected_members): + members = res.context['members'] + self.assertNotIn(unexpected_members, map(just_id, members), 'members has wrong objects: %s' % members) + + def _given_mk_added_to_meeting(self, meeting, mk): + return self.client.post(reverse('committee-meeting', + kwargs={'pk': meeting.id}), + {'user_input_type': 'mk', + 'mk_name': mk.name}) + + def _given_mk_removed_from_meeting(self, meeting, mk): + return self.client.post(reverse('committee-meeting', + kwargs={'pk': meeting.id}), + {'user_input_type': 'remove-mk', + 'mk_name_to_remove': mk.name}) + + def _given_lobbyist_added_to_meeting(self, meeting, lobbyist=None, lobbyist_name=None): + lobbyist_name = lobbyist.person.name if lobbyist else lobbyist_name + return self.client.post(reverse('committee-meeting', + kwargs={'pk': meeting.id}), + {'user_input_type': 'add-lobbyist', + 'lobbyist_name': lobbyist_name}) + + def _given_lobbyist_removed_from_meeting(self, meeting, lobbyist=None, lobbyist_name=None): + lobbyist_name = lobbyist.person.name if lobbyist else lobbyist_name + return self.client.post(reverse('committee-meeting', + kwargs={'pk': meeting.id}), + {'user_input_type': 'remove-lobbyist', + 'lobbyist_name': lobbyist_name}) + + def _verify_mk_does_not_have_meeting(self, meeting, mk_1): + self.assertFalse(meeting in mk_1.committee_meetings.all()) + + def _verify_mk_has_meeting(self, meeting, mk_1): + self.assertTrue(meeting in mk_1.committee_meetings.all()) + + def _verify_bill_in_meeting(self, bill_1, meeting): + self.assertTrue(bill_1 in meeting.bills_first.all()) + + def _given_bill_added_to_meeting(self, bill_1, meeting): + return self.client.post(reverse('committee-meeting', + kwargs={'pk': + meeting.id}), + {'user_input_type': 'bill', + 'bill_id': bill_1.id}) + + def _verify_bill_not_in_meeting(self, bill_1, meeting): + self.assertFalse(bill_1 in meeting.bills_first.all()) + + def _verify_lobbyist_mentioned_in_meetings(self, lobbyist, meeting): + self.assertTrue(lobbyist in meeting.lobbyists_mentioned.all()) + + def _verify_lobbyist_not_mentioned_in_meetings(self, lobbyist, meeting): + self.assertFalse(lobbyist in meeting.lobbyists_mentioned.all()) + + def _setup_lobbyist(self, name='kressni'): + person = Person.objects.create(name=name) + return Lobbyist.objects.create(person=person) + + def _setup_alias_for_person(self, person, alias_name='alias_name'): + return PersonAlias.objects.create(person=person, name=alias_name) + + def given_mk_is_added_to_committee(self, committee, mk): + committee.members.add(mk) + + def given_mk_is_added_to_committee_as_replacment(self, committee, mk): + committee.replacements.add(mk) + + def given_mk_is_added_to_committee_as_chairperson(self, committee, mk): + committee.chairpersons.add(mk) + + diff --git a/committees/tests/test_committee_detail_view.py b/committees/tests/test_committee_detail_view.py index 9c6252ae..9d4526b1 100644 --- a/committees/tests/test_committee_detail_view.py +++ b/committees/tests/test_committee_detail_view.py @@ -4,10 +4,10 @@ from django.core.urlresolvers import reverse from django.contrib.auth.models import User, Group, Permission from django.contrib.contenttypes.models import ContentType -import unittest -from annotatetext.models import Annotation -from actstream.models import Action + from tagging.models import Tag, TaggedItem + +from committees.tests.base import BaseCommitteeTestCase from laws.models import Bill from mks.models import Member, Knesset from committees.models import Committee, CommitteeMeeting @@ -16,7 +16,7 @@ APP = 'committees' -class CommitteeDetailViewTest(TestCase): +class CommitteeDetailViewTest(BaseCommitteeTestCase): def setUp(self): super(CommitteeDetailViewTest, self).setUp() self.knesset = Knesset.objects.create(number=1, @@ -67,7 +67,7 @@ def tearDown(self): self.mk_1.delete() self.topic.delete() - def testCommitteeList(self): + def test_committee_list_view(self): res = self.client.get(reverse('committee-list')) self.assertEqual(res.status_code, 200) self.assertTemplateUsed(res, 'committees/committee_list.html') @@ -87,6 +87,41 @@ def test_committee_returns_a_list_of_meetings(self): [self.meeting_1.id, self.meeting_2.id, ], 'object_list has wrong objects: %s' % object_list) + def test_committee_does_not_return_non_active_current_knesset_members(self): + non_current_mk = Member.objects.create(name='mk is no more', is_current=False) + self.given_mk_is_added_to_committee(self.committee_1, non_current_mk) + self.given_mk_is_added_to_committee(self.committee_1, self.mk_1) + + res = self.client.get(self.committee_1.get_absolute_url()) + self.assertEqual(res.status_code, 200) + + self.verify_expected_members_in_context(res, [self.mk_1.pk]) + self.verify_unexpected_members_not_in_context(res, [non_current_mk.pk]) + + def test_committee_does_not_return_non_active_current_knesset_members_who_were_replacements(self): + non_current_mk = Member.objects.create(name='mk is no more', is_current=False) + self.given_mk_is_added_to_committee(self.committee_1, non_current_mk) + self.given_mk_is_added_to_committee_as_replacment(self.committee_1, non_current_mk) + self.given_mk_is_added_to_committee(self.committee_1, self.mk_1) + + res = self.client.get(self.committee_1.get_absolute_url()) + self.assertEqual(res.status_code, 200) + + self.verify_expected_members_in_context(res, [self.mk_1.pk]) + self.verify_unexpected_members_not_in_context(res, [non_current_mk.pk]) + + def test_committee_does_not_return_non_active_current_knesset_members_who_were_person(self): + non_current_mk = Member.objects.create(name='mk is no more', is_current=False) + self.given_mk_is_added_to_committee(self.committee_1, non_current_mk) + self.given_mk_is_added_to_committee_as_chairperson(self.committee_1, non_current_mk) + self.given_mk_is_added_to_committee(self.committee_1, self.mk_1) + + res = self.client.get(self.committee_1.get_absolute_url()) + self.assertEqual(res.status_code, 200) + + self.verify_expected_members_in_context(res, [self.mk_1.pk]) + self.verify_unexpected_members_not_in_context(res, [non_current_mk.pk]) + def test_committeemeeting_by_tag(self): res = self.client.get('%s?tagged=false' % reverse('committee-all-meetings')) self.assertQuerysetEqual(res.context['object_list'], diff --git a/committees/tests/test_committee_meeting_detail_view.py b/committees/tests/test_committee_meeting_detail_view.py index a9c55580..d0bc1972 100644 --- a/committees/tests/test_committee_meeting_detail_view.py +++ b/committees/tests/test_committee_meeting_detail_view.py @@ -1,24 +1,23 @@ +import unittest from datetime import datetime, timedelta -from django.test import TestCase + +from actstream.models import Action +from annotatetext.models import Annotation from django.conf import settings -from django.core.urlresolvers import reverse from django.contrib.auth.models import User, Group, Permission from django.contrib.contenttypes.models import ContentType -import unittest -from annotatetext.models import Annotation -from actstream.models import Action -from tagging.models import Tag, TaggedItem +from django.core.urlresolvers import reverse +from tagging.models import Tag + +from committees.models import Committee +from committees.tests.base import BaseCommitteeTestCase from laws.models import Bill -from lobbyists.models import Lobbyist from mks.models import Member, Knesset -from committees.models import Committee, CommitteeMeeting -from persons.models import Person, PersonAlias -just_id = lambda x: x.id APP = 'committees' -class CommitteeMeetingDetailViewTestCase(TestCase): +class CommitteeMeetingDetailViewTestCase(BaseCommitteeTestCase): def setUp(self): super(CommitteeMeetingDetailViewTestCase, self).setUp() self.knesset = Knesset.objects.create(number=1, @@ -161,16 +160,15 @@ def test_committee_meeting_returns_correct_members(self): self.assertEqual(res.status_code, 200) self.assertTemplateUsed(res, 'committees/committeemeeting_detail.html') - self._verify_expected_members_in_context(res, [self.mk_1.id]) + self.verify_expected_members_in_context(res, [self.mk_1.id]) - def test_mk_from_current_knesset_that_are_not_current_members_are_not_displayed(self): - not_current_mk = Member.objects.create(name='mk 1', is_current=False) + def test_mk_from_current_knesset_that_are_not_current_members_are_also_displayed(self): + not_current_mk = Member.objects.create(name='mk is no more', is_current=False) self.given_mk_attended_meeting(self.meeting_1, not_current_mk) self._given_mk_added_to_meeting(self.meeting_1, not_current_mk) res = self.client.get(self.meeting_1.get_absolute_url()) self.assertEqual(res.status_code, 200) - self._verify_expected_members_in_context(res, [self.mk_1.id]) - self._verify_unexpected_members_not_in_context(res, [not_current_mk.id]) + self.verify_expected_members_in_context(res, [self.mk_1.id, not_current_mk.pk]) def test_user_can_not_add_a_bill_to_meetings_if_not_login(self): res = self.client.post(reverse('committee-meeting', @@ -180,16 +178,6 @@ def test_user_can_not_add_a_bill_to_meetings_if_not_login(self): self.assertTrue(res['location'].startswith('%s%s' % ('http://testserver', settings.LOGIN_URL))) - def _verify_expected_members_in_context(self, res, expected_mks): - members = res.context['members'] - self.assertEqual(map(just_id, members), - expected_mks, - 'members has wrong objects: %s' % members) - - def _verify_unexpected_members_not_in_context(self, res, unexpected_members): - members = res.context['members'] - self.assertNotIn(unexpected_members, map(just_id, members), 'members has wrong objects: %s' % members) - def test_post_removing_and_adding_mk_to_committee_meetings(self): mk_1 = self.mk_1 meeting = self.meeting_1 @@ -314,61 +302,3 @@ def test_create_tag(self): self.assertEqual(res.status_code, 200) self.new_tag = Tag.objects.get(name='new tag') self.assertIn(self.new_tag, self.meeting_1.tags) - - def _given_mk_added_to_meeting(self, meeting, mk): - return self.client.post(reverse('committee-meeting', - kwargs={'pk': meeting.id}), - {'user_input_type': 'mk', - 'mk_name': mk.name}) - - def _given_mk_removed_from_meeting(self, meeting, mk): - return self.client.post(reverse('committee-meeting', - kwargs={'pk': meeting.id}), - {'user_input_type': 'remove-mk', - 'mk_name_to_remove': mk.name}) - - def _given_lobbyist_added_to_meeting(self, meeting, lobbyist=None, lobbyist_name=None): - lobbyist_name = lobbyist.person.name if lobbyist else lobbyist_name - return self.client.post(reverse('committee-meeting', - kwargs={'pk': meeting.id}), - {'user_input_type': 'add-lobbyist', - 'lobbyist_name': lobbyist_name}) - - def _given_lobbyist_removed_from_meeting(self, meeting, lobbyist=None, lobbyist_name=None): - lobbyist_name = lobbyist.person.name if lobbyist else lobbyist_name - return self.client.post(reverse('committee-meeting', - kwargs={'pk': meeting.id}), - {'user_input_type': 'remove-lobbyist', - 'lobbyist_name': lobbyist_name}) - - def _verify_mk_does_not_have_meeting(self, meeting, mk_1): - self.assertFalse(meeting in mk_1.committee_meetings.all()) - - def _verify_mk_has_meeting(self, meeting, mk_1): - self.assertTrue(meeting in mk_1.committee_meetings.all()) - - def _verify_bill_in_meeting(self, bill_1, meeting): - self.assertTrue(bill_1 in meeting.bills_first.all()) - - def _given_bill_added_to_meeting(self, bill_1, meeting): - return self.client.post(reverse('committee-meeting', - kwargs={'pk': - meeting.id}), - {'user_input_type': 'bill', - 'bill_id': bill_1.id}) - - def _verify_bill_not_in_meeting(self, bill_1, meeting): - self.assertFalse(bill_1 in meeting.bills_first.all()) - - def _verify_lobbyist_mentioned_in_meetings(self, lobbyist, meeting): - self.assertTrue(lobbyist in meeting.lobbyists_mentioned.all()) - - def _verify_lobbyist_not_mentioned_in_meetings(self, lobbyist, meeting): - self.assertFalse(lobbyist in meeting.lobbyists_mentioned.all()) - - def _setup_lobbyist(self, name='kressni'): - person = Person.objects.create(name=name) - return Lobbyist.objects.create(person=person) - - def _setup_alias_for_person(self, person, alias_name='alias_name'): - return PersonAlias.objects.create(person=person, name=alias_name) diff --git a/committees/views.py b/committees/views.py index ceab6990..70750e35 100644 --- a/committees/views.py +++ b/committees/views.py @@ -76,12 +76,6 @@ class CommitteeDetailView(DetailView): model = Committee queryset = Committee.objects.prefetch_related('members', 'chairpersons', 'replacements', 'events', 'meetings', ).all() - # 'meetings__mks_attended', - # 'meetings__lobbyists_mentioned', - # 'meetings__lobbyist_corporations_mentioned', - # 'topic_set', - # - # 'mmm_documents').all() view_cache_key = 'committee_detail_%d' SEE_ALL_THRESHOLD = 10 @@ -210,7 +204,7 @@ def get_context_data(self, *args, **kwargs): context['hide_member_presence'] = True else: # get meeting members with presence calculation - meeting_members_ids = set(member.id for member in cm.mks_attended.filter(is_current=True)) + meeting_members_ids = set(member.id for member in cm.mks_attended.all()) members = cm.committee.members_by_presence(ids=meeting_members_ids) context['hide_member_presence'] = False