Merge pull request #729 from alonisser/bugfix_724_again

resolve #724 non current mk appears in committee page

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(

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

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'],

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)

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