From c2b10bc3a2e36a7734a87a52479585bb96548597 Mon Sep 17 00:00:00 2001 From: alonisser Date: Sat, 1 Oct 2016 21:30:58 +0300 Subject: [PATCH 01/11] pep8 and deprecated expection syntax --- committees/models.py | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/committees/models.py b/committees/models.py index 352d6d95..063098af 100644 --- a/committees/models.py +++ b/committees/models.py @@ -101,7 +101,7 @@ def annotations(self): committee_tn = Committee._meta.db_table annotation_tn = Annotation._meta.db_table protocol_part_ct = ContentType.objects.get_for_model(ProtocolPart) - ret = Annotation.objects.filter(content_type=protocol_part_ct) + ret = Annotation.objects.select_related().filter(content_type=protocol_part_ct) return ret.extra(tables=[protocol_part_tn, meeting_tn, committee_tn], where=["%s.object_id=%s.id" % (annotation_tn, protocol_part_tn), @@ -277,9 +277,9 @@ def __unicode__(self): @models.permalink def get_absolute_url(self): if self.committee.type == 'plenum': - return ('plenum-meeting', [str(self.id)]) + return 'plenum-meeting', [str(self.id)] else: - return ('committee-meeting', [str(self.id)]) + return 'committee-meeting', [str(self.id)] def _get_tags(self): tags = Tag.objects.get_for_object(self) @@ -300,7 +300,7 @@ def create_protocol_parts(self, delete_existing=False, mks=None, mk_names=None): delete them, a ValidationError will be thrown, because it doesn't make sense to create the parts again. """ - logger.debug('create_protocol_parts %s'%delete_existing) + logger.debug('create_protocol_parts %s' % delete_existing) if delete_existing: ppct = ContentType.objects.get_for_model(ProtocolPart) annotations = Annotation.objects.filter(content_type=ppct, object_id__in=self.parts.all) @@ -320,18 +320,19 @@ def create_protocol_parts(self, delete_existing=False, mks=None, mk_names=None): return else: def get_protocol_part(i, part): - logger.debug('creating protocol part %s'%i) + logger.debug('creating protocol part %s' % i) return ProtocolPart(meeting=self, order=i, header=part.header, body=part.body) + with KnessetDataCommitteeMeetingProtocol.get_from_text(self.protocol_text) as protocol: # TODO: use bulk_create (I had a strange error when using it) # ProtocolPart.objects.bulk_create( # for testing, you could just save one part: # get_protocol_part(1, protocol.parts[0]).save() list([ - get_protocol_part(i, part).save() - for i, part - in zip(range(1, len(protocol.parts)+1), protocol.parts) - ]) + get_protocol_part(i, part).save() + for i, part + in zip(range(1, len(protocol.parts) + 1), protocol.parts) + ]) self.protocol_parts_update_date = datetime.now() self.save() @@ -347,7 +348,7 @@ def redownload_protocol(self): self.protocol_text = protocol.text self.protocol_text_update_date = datetime.now() self.save() - except AntiwordException, e: + except AntiwordException as e: logger.error( e.message, exc_info=True, @@ -373,14 +374,16 @@ def update_from_dataservice(self, dataservice_object=None): if dataservice_object is None: ds_meetings = [ ds_meeting for ds_meeting - in DataserviceCommitteeMeeting.get(self.committee.knesset_id, self.date - timedelta(days=1), self.date + timedelta(days=1)) + in DataserviceCommitteeMeeting.get(self.committee.knesset_id, self.date - timedelta(days=1), + self.date + timedelta(days=1)) if str(ds_meeting.id) == str(self.knesset_id) - ] + ] if len(ds_meetings) != 1: raise Exception('could not found corresponding dataservice meeting') dataservice_object = ds_meetings[0] - meeting_transformed = ScrapeCommitteeMeetingCommand().get_committee_meeting_fields_from_dataservice(dataservice_object) - [setattr(self, k, v) for k,v in meeting_transformed.iteritems()] + meeting_transformed = ScrapeCommitteeMeetingCommand().get_committee_meeting_fields_from_dataservice( + dataservice_object) + [setattr(self, k, v) for k, v in meeting_transformed.iteritems()] self.save() @property From f57675a8d573ccdf6098088bda9c807f6c592e54 Mon Sep 17 00:00:00 2001 From: alonisser Date: Sat, 1 Oct 2016 21:31:49 +0300 Subject: [PATCH 02/11] Handling missing data related obj --- lobbyists/admin.py | 7 +++--- lobbyists/models.py | 26 ++++++++++++++-------- lobbyists/tests/test_lobbyist_view.py | 31 +++++++++++++++++++++++++++ lobbyists/views.py | 6 +++--- 4 files changed, 55 insertions(+), 15 deletions(-) create mode 100644 lobbyists/tests/test_lobbyist_view.py diff --git a/lobbyists/admin.py b/lobbyists/admin.py index 4af8648f..2d1f7715 100644 --- a/lobbyists/admin.py +++ b/lobbyists/admin.py @@ -31,9 +31,10 @@ class LobbyistCorporationAdmin(ImportExportModelAdmin): list_display = ('name', 'alias_corporations') def lobbyists(self, obj): - return mark_safe(', '.join([ - u''+unicode(lobbyist)+u'' for lobbyist in obj.latest_data.lobbyists.all() - ])) + if obj.latest_data: + return mark_safe(', '.join([ + u''+unicode(lobbyist)+u'' for lobbyist in obj.latest_data.lobbyists.all() + ])) class LobbyistsChangeAdmin(admin.ModelAdmin): diff --git a/lobbyists/models.py b/lobbyists/models.py index 9933c97a..52867902 100644 --- a/lobbyists/models.py +++ b/lobbyists/models.py @@ -74,7 +74,9 @@ class Lobbyist(models.Model): @cached_property def latest_data(self): - return self.data.filter(scrape_time__isnull=False).latest('scrape_time') + lobbyist_data = self.data.filter(scrape_time__isnull=False) + if lobbyist_data.exists(): + return lobbyist_data.latest('scrape_time') @cached_property def latest_corporation(self): @@ -92,14 +94,19 @@ def cached_data(self): data = { 'id': self.id, 'display_name': unicode(self.person), - 'latest_data': { - 'profession': self.latest_data.profession, - 'faction_member': self.latest_data.faction_member, - 'faction_name': self.latest_data.faction_name, - 'permit_type': self.latest_data.permit_type, - 'scrape_time': self.latest_data.scrape_time, - }, } + latest_data = self.latest_data + if latest_data: + data.update({ + + 'latest_data': { + 'profession': self.latest_data.profession, + 'faction_member': self.latest_data.faction_member, + 'faction_name': self.latest_data.faction_name, + 'permit_type': self.latest_data.permit_type, + 'scrape_time': self.latest_data.scrape_time, + } + }) if self.latest_corporation: data.update( {'latest_corporation': { @@ -121,7 +128,8 @@ def latest_lobbyist_corporation(self, corporation_id): def get_corporation_lobbyists(self, corporation_id): lobbyists = [] for lobbyist in LobbyistHistory.objects.latest().lobbyists.all(): - lobbyists.append(lobbyist) if lobbyist.latest_data.corporation_id == corporation_id else None + if lobbyist.latest_data: + lobbyists.append(lobbyist) if lobbyist.latest_data.corporation_id == corporation_id else None return lobbyists diff --git a/lobbyists/tests/test_lobbyist_view.py b/lobbyists/tests/test_lobbyist_view.py new file mode 100644 index 00000000..8033da06 --- /dev/null +++ b/lobbyists/tests/test_lobbyist_view.py @@ -0,0 +1,31 @@ +# -*- coding: utf-8 -* +from django.core.urlresolvers import reverse +from django.test import TestCase + +from lobbyists.models import Lobbyist +from persons.models import Person + + +class LobbyistDetailViewTestCase(TestCase): + def setUp(self): + return super(LobbyistDetailViewTestCase, self).setUp() + + def tearDown(self): + return super(LobbyistDetailViewTestCase, self).tearDown() + + def given_lobbyist_exists(self, name='kressni'): + person = Person.objects.create(name=name) + return Lobbyist.objects.create(person=person) + + def test_endpoint_returns_lobbyist(self): + kresni = self.given_lobbyist_exists() + res = self.client.get(reverse('lobbyist-detail', args=[kresni.pk])) + + self.assertEqual(res.status_code, 200) + + def test_endpoint_returns_lobbyist_with_missing_data(self): + kresni = self.given_lobbyist_exists() + kresni.data.all().delete() + res = self.client.get(reverse('lobbyist-detail', args=[kresni.pk])) + + self.assertEqual(res.status_code, 200) diff --git a/lobbyists/views.py b/lobbyists/views.py index 2912581e..1c99cdc9 100644 --- a/lobbyists/views.py +++ b/lobbyists/views.py @@ -87,9 +87,9 @@ class LobbyistDetailView(DetailView): def get_context_data(self, **kwargs): context = super(LobbyistDetailView, self).get_context_data(**kwargs) lobbyist = context['object'] - context['represents'] = lobbyist.latest_data.represents.all() - context['corporation'] = lobbyist.latest_corporation - context['data'] = lobbyist.latest_data + context['represents'] = lobbyist.latest_data.represents.all() if lobbyist.latest_data else [] + context['corporation'] = lobbyist.latest_corporation if lobbyist.latest_data else None + context['data'] = lobbyist.latest_data if lobbyist.latest_data else {} context['links'] = Link.objects.for_model(context['object']) return context From cb36d2fe18d4377b9ec1a851fe68156796aaa5ed Mon Sep 17 00:00:00 2001 From: alonisser Date: Sat, 1 Oct 2016 21:32:24 +0300 Subject: [PATCH 03/11] Check presence only for currently active mk --- simple/management/commands/syncdata.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/simple/management/commands/syncdata.py b/simple/management/commands/syncdata.py index f6a4da94..9a60ee6e 100644 --- a/simple/management/commands/syncdata.py +++ b/simple/management/commands/syncdata.py @@ -1132,9 +1132,8 @@ def update_presence(self): c.sort() min_timestamp = c[0] c = None - # TODO: should we also parse of current knesset parties not serving members? - # currently we still do but this check is better the previous - for member in Member.current_knesset.all(): + + for member in Member.current_members.all(): if member.id not in presence: logger.error('member %s (id=%d) not found in presence data', member.name, member.id) continue From 8592fe53c5b087c1b87862a37614a2c8a1455f5f Mon Sep 17 00:00:00 2001 From: alonisser Date: Sat, 1 Oct 2016 21:32:49 +0300 Subject: [PATCH 04/11] pep8 --- committees/urls.py | 86 ++++++++++++++++++++++++++-------------------- 1 file changed, 48 insertions(+), 38 deletions(-) diff --git a/committees/urls.py b/committees/urls.py index ec7c4254..f38eb50d 100644 --- a/committees/urls.py +++ b/committees/urls.py @@ -1,4 +1,4 @@ -#encoding: UTF-8 +# encoding: UTF-8 from django.conf.urls import url, patterns from djangoratings.views import AddRatingFromModel from views import ( @@ -7,45 +7,55 @@ TopicDetailView, delete_topic, delete_topic_rating, meeting_list_by_date, edit_topic, CommitteeMMMDocuments, UnpublishedProtocolslistView, FutureMeetingslistView) - meetings_list = MeetingsListView.as_view() unpublished_protocols_list = UnpublishedProtocolslistView.as_view() future_meetings_list = FutureMeetingslistView.as_view() committeesurlpatterns = patterns('', - url(r'^committee/$', CommitteeListView.as_view(), name='committee-list'), - url(r'^committee/more-topics/$', TopicsMoreView.as_view(), name='committee-topics-more'), - url(r'^committee/(?P\d+)/$', CommitteeDetailView.as_view(), name='committee-detail'), - url(r'^committee/all_meetings/$', meetings_list, name='committee-all-meetings'), - url(r'^committee/(?P\d+)/all_meetings/$', meetings_list, name='committee-all-meetings'), - url(r'^committee/(?P\d+)/all_unpublished_protocols/$', unpublished_protocols_list, name='committee-all-unpublished-protocols'), - url(r'^committee/(?P\d+)/all_future_meetings/$', future_meetings_list, name='committee-all-future-meetings'), - url(r'^committee/date/(?P[\d\-]+)/$', meeting_list_by_date, name='committee-meetings-by-date'), - url(r'^committee/(?P\d+)/date/(?P[\d\-]+)/$', meeting_list_by_date, name='committee-meetings-by-date'), - url(r'^committee/(?P\d+)/date/$', meeting_list_by_date, name='committee-meetings-by-date'), - url(r'^committee/(?P\d+)/topic/$', TopicListView.as_view(), name='committee-topic-list'), - url(r'^committee/(?P\d+)/topic/add/$', - edit_topic, - name='edit-committee-topic'), - url(r'^committee/(?P\d+)/topic/edit/(?P\d+)/$', - edit_topic, - name='edit-committee-topic'), - url(r'^committee/meeting/(?P\d+)/$', MeetingDetailView.as_view(), name='committee-meeting'), - url(r'^committee/meeting/tag/(?P.*)/$', MeetingTagListView.as_view(), - name='committeemeeting-tag'), - url(r'^committee/topic/$', TopicListView.as_view(), name='topic-list'), - url(r'^committee/topic/(?P\d+)/delete/$', delete_topic, - name='delete-committee-topic'), - url(r'^committee/topic/(?P\d+)/$', TopicDetailView.as_view(), name='topic-detail'), - url(r'^committee/topic/(?P\d+)/(?P\d+)/$', - AddRatingFromModel(), - {'app_label': 'committees', 'model': 'topic', 'field_name': 'rating'}, - name='rate-topic'), - url(r'^committee/topic/(?P\d+)/null/$', delete_topic_rating, - name='delete-topic-rating'), - url(r'^committee/(?P\d+)/mmm_documents/$', CommitteeMMMDocuments.as_view(), - name='committee-mmm-documents-list'), - url(r'^committee/(?P\d+)/mmm_documents/date/(?P[\d\-]+)/$', - CommitteeMMMDocuments.as_view(), - name='committee-mmm-documents-list-by-date'), -) + url(r'^committee/$', CommitteeListView.as_view(), name='committee-list'), + url(r'^committee/more-topics/$', TopicsMoreView.as_view(), + name='committee-topics-more'), + url(r'^committee/(?P\d+)/$', CommitteeDetailView.as_view(), + name='committee-detail'), + url(r'^committee/all_meetings/$', meetings_list, name='committee-all-meetings'), + url(r'^committee/(?P\d+)/all_meetings/$', meetings_list, + name='committee-all-meetings'), + url(r'^committee/(?P\d+)/all_unpublished_protocols/$', + unpublished_protocols_list, name='committee-all-unpublished-protocols'), + url(r'^committee/(?P\d+)/all_future_meetings/$', future_meetings_list, + name='committee-all-future-meetings'), + url(r'^committee/date/(?P[\d\-]+)/$', meeting_list_by_date, + name='committee-meetings-by-date'), + url(r'^committee/(?P\d+)/date/(?P[\d\-]+)/$', meeting_list_by_date, + name='committee-meetings-by-date'), + url(r'^committee/(?P\d+)/date/$', meeting_list_by_date, + name='committee-meetings-by-date'), + url(r'^committee/(?P\d+)/topic/$', TopicListView.as_view(), + name='committee-topic-list'), + url(r'^committee/(?P\d+)/topic/add/$', + edit_topic, + name='edit-committee-topic'), + url(r'^committee/(?P\d+)/topic/edit/(?P\d+)/$', + edit_topic, + name='edit-committee-topic'), + url(r'^committee/meeting/(?P\d+)/$', MeetingDetailView.as_view(), + name='committee-meeting'), + url(r'^committee/meeting/tag/(?P.*)/$', MeetingTagListView.as_view(), + name='committeemeeting-tag'), + url(r'^committee/topic/$', TopicListView.as_view(), name='topic-list'), + url(r'^committee/topic/(?P\d+)/delete/$', delete_topic, + name='delete-committee-topic'), + url(r'^committee/topic/(?P\d+)/$', TopicDetailView.as_view(), name='topic-detail'), + url(r'^committee/topic/(?P\d+)/(?P\d+)/$', + AddRatingFromModel(), + {'app_label': 'committees', 'model': 'topic', 'field_name': 'rating'}, + name='rate-topic'), + url(r'^committee/topic/(?P\d+)/null/$', delete_topic_rating, + name='delete-topic-rating'), + url(r'^committee/(?P\d+)/mmm_documents/$', + CommitteeMMMDocuments.as_view(), + name='committee-mmm-documents-list'), + url(r'^committee/(?P\d+)/mmm_documents/date/(?P[\d\-]+)/$', + CommitteeMMMDocuments.as_view(), + name='committee-mmm-documents-list-by-date'), + ) From e430110242f47d9b9db9b51570f1182633715d96 Mon Sep 17 00:00:00 2001 From: alonisser Date: Sat, 1 Oct 2016 21:34:53 +0300 Subject: [PATCH 05/11] Adding custom manager for current knesset active member only --- mks/managers.py | 13 +++++++++++-- mks/models.py | 3 ++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/mks/managers.py b/mks/managers.py index 38958cd6..6795a146 100644 --- a/mks/managers.py +++ b/mks/managers.py @@ -3,6 +3,7 @@ from django.db import models, connection from django.db.models import Q + # from agendas.models import Agenda @@ -84,17 +85,25 @@ def current_parties(self): class CurrentKnessetMembersManager(models.Manager): - "Adds the ability to filter on current knesset" + """ + Adds the ability to filter on current knesset + """ def get_queryset(self): from mks.models import Knesset qs = super(CurrentKnessetMembersManager, self).get_queryset() - # TODO: Strange? why should we filter by knesset and not other options? + current_knesset = Knesset.objects.current_knesset() qs = qs.filter(current_party__knesset=current_knesset) return qs +class CurrentKnessetActiveMembersManager(CurrentKnessetMembersManager): + def get_queryset(self): + qs = super(CurrentKnessetActiveMembersManager, self).get_queryset() + return qs.filter(is_current=True) + + class MembershipManager(models.Manager): def membership_in_range(self, ranges=None, only_current_mks=False): if only_current_mks: diff --git a/mks/models.py b/mks/models.py index 2f22ed0d..e7e08d1d 100644 --- a/mks/models.py +++ b/mks/models.py @@ -21,7 +21,7 @@ from mks.managers import ( BetterManager, PartyManager, KnessetManager, CurrentKnessetMembersManager, - CurrentKnessetPartyManager, MembershipManager) + CurrentKnessetPartyManager, MembershipManager, CurrentKnessetActiveMembersManager) GENDER_CHOICES = ( (u'M', _('Male')), @@ -250,6 +250,7 @@ class Member(models.Model): objects = BetterManager() current_knesset = CurrentKnessetMembersManager() + current_members = CurrentKnessetActiveMembersManager() class Meta: ordering = ['name'] From 29833a85d9d80c9f23c920e2cea155f5348589f1 Mon Sep 17 00:00:00 2001 From: alonisser Date: Sat, 1 Oct 2016 22:02:46 +0300 Subject: [PATCH 06/11] Get only active mks in committee --- .../test_committee_meeting_detail_view.py | 52 +++++++++++++++---- committees/views.py | 18 +++++-- 2 files changed, 57 insertions(+), 13 deletions(-) diff --git a/committees/tests/test_committee_meeting_detail_view.py b/committees/tests/test_committee_meeting_detail_view.py index cb11c262..a9c55580 100644 --- a/committees/tests/test_committee_meeting_detail_view.py +++ b/committees/tests/test_committee_meeting_detail_view.py @@ -18,9 +18,9 @@ APP = 'committees' -class CommitteeMeetingDetailViewTest(TestCase): +class CommitteeMeetingDetailViewTestCase(TestCase): def setUp(self): - super(CommitteeMeetingDetailViewTest, self).setUp() + super(CommitteeMeetingDetailViewTestCase, self).setUp() self.knesset = Knesset.objects.create(number=1, start_date=datetime.today() - timedelta(days=1)) self.committee_1 = Committee.objects.create(name='c1') @@ -54,10 +54,13 @@ def setUp(self): self.topic = self.committee_1.topic_set.create(creator=self.jacob, title="hello", description="hello world") self.tag_1 = Tag.objects.create(name='tag1') - self.meeting_1.mks_attended.add(self.mk_1) + self.given_mk_attended_meeting(self.meeting_1, self.mk_1) + + def given_mk_attended_meeting(self, meeting, mk): + meeting.mks_attended.add(mk) def tearDown(self): - super(CommitteeMeetingDetailViewTest, self).tearDown() + super(CommitteeMeetingDetailViewTestCase, self).tearDown() self.client.logout() self.meeting_1.delete() self.meeting_2.delete() @@ -158,12 +161,18 @@ def test_committee_meeting_returns_correct_members(self): self.assertEqual(res.status_code, 200) self.assertTemplateUsed(res, 'committees/committeemeeting_detail.html') - members = res.context['members'] - self.assertEqual(map(just_id, members), - [self.mk_1.id], - 'members has wrong objects: %s' % members) + 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) + 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]) - def test_user_can_add_a_bill_to_meetings_if_not_login(self): + def test_user_can_not_add_a_bill_to_meetings_if_not_login(self): res = self.client.post(reverse('committee-meeting', kwargs={'pk': self.meeting_1.id})) self._verify_bill_not_in_meeting(self.bill_1, self.meeting_1) @@ -171,6 +180,16 @@ def test_user_can_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 @@ -197,7 +216,6 @@ def test_post_removing_and_adding_mk_without_params_get_404_response(self): res = self._given_mk_added_to_meeting(meeting, mk_1) self.assertEqual(res.status_code, 404) - def test_post_adds_bill_to_committee_meeting(self): bill_1 = self.bill_1 meeting = self.meeting_1 @@ -238,6 +256,20 @@ def test_adding_non_existent_lobbyist_returns_404(self): res = self._given_lobbyist_added_to_meeting(meeting, lobbyist_name='non existing') self.assertEqual(res.status_code, 404) + def test_committee_meetings_handles_missing_lobbyist_data(self): + lobbyist = self._setup_lobbyist() + + meeting = self.meeting_1 + + self.assertTrue(self.client.login(username='jacob', password='JKM')) + self._given_lobbyist_added_to_meeting(meeting, lobbyist=lobbyist) + res = self.client.get(self.meeting_1.get_absolute_url()) + self.assertEqual(res.status_code, 200) + + lobbyist.data.all().delete() + + res = self.client.get(self.meeting_1.get_absolute_url()) + self.assertEqual(res.status_code, 200) def test_add_tag_committee_login_required(self): url = reverse('add-tag-to-object', diff --git a/committees/views.py b/committees/views.py index f877f8d8..e1a9df0c 100644 --- a/committees/views.py +++ b/committees/views.py @@ -1,3 +1,4 @@ +# -*- coding: utf-8 -* import datetime import json import re @@ -6,7 +7,9 @@ import difflib import logging +import itertools import tagging +from django.core.exceptions import ObjectDoesNotExist from django.db.models import Q import auxiliary.tag_suggestions @@ -149,13 +152,22 @@ def get_context_data(self, *args, **kwargs): context['paginate_by'] = models.COMMITTEE_PROTOCOL_PAGINATE_BY if cm.committee.type == 'plenum': - context['members'] = cm.mks_attended.order_by('name') + members = cm.mks_attended.order_by('name') + context['hide_member_presence'] = True else: # get meeting members with presence calculation - meeting_members_ids = set(m.id for m in cm.mks_attended.all()) - context['members'] = cm.committee.members_by_presence(ids=meeting_members_ids) + meeting_members_ids = set(member.id for member in cm.mks_attended.filter(is_current=True)) + members = cm.committee.members_by_presence(ids=meeting_members_ids) + context['hide_member_presence'] = False + links = Link.objects.for_model(Member).values() + links_by_member = {} + for k, g in itertools.groupby(links, lambda x: x['object_pk']): + links_by_member[str(k)] = list(g) + for member in members: + member.links = links_by_member.get(str(member.pk), []) + context['members'] = members meeting_text = [cm.topics] + [part.body for part in cm.parts.all()] context['tag_suggestions'] = auxiliary.tag_suggestions.extract_suggested_tags(cm.tags, From 86230ef542544fb0e536a23ab3deff80db7d0382 Mon Sep 17 00:00:00 2001 From: alonisser Date: Sat, 1 Oct 2016 22:04:54 +0300 Subject: [PATCH 07/11] Indenting --- templates/links/_object_icon_links.html | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/templates/links/_object_icon_links.html b/templates/links/_object_icon_links.html index 8c323863..2b826ff6 100644 --- a/templates/links/_object_icon_links.html +++ b/templates/links/_object_icon_links.html @@ -1,9 +1,10 @@ {% load i18n %} {% for link in links %} -{% if link.active %} -
  • - {{ link.title }} -
    • -{% endif %} + {% if link.active %} +
  • + {{ link.title }} +
    • + {% endif %} {% endfor %} From b42ec1cee1f80c0e4b1dbf0f4ac67f5828a28090 Mon Sep 17 00:00:00 2001 From: alonisser Date: Sat, 1 Oct 2016 22:06:22 +0300 Subject: [PATCH 08/11] Optional reusing of provided links on object instead of hitting the DB again --- links/templatetags/links_tags.py | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/links/templatetags/links_tags.py b/links/templatetags/links_tags.py index 115c1f15..aea6de33 100644 --- a/links/templatetags/links_tags.py +++ b/links/templatetags/links_tags.py @@ -8,16 +8,22 @@ @register.inclusion_tag('links/_object_links.html') def object_links(obj): - l = Link.objects.for_model(obj) - return {'links': l, 'MEDIA_URL': settings.MEDIA_URL} + if hasattr(obj, 'links'): + obj_links = obj.links + else: + obj_links = Link.objects.for_model(obj) + return {'links': obj_links, 'MEDIA_URL': settings.MEDIA_URL} @register.inclusion_tag('links/_object_icon_links.html') def object_icon_links(obj): "Display links as icons, to match the new design" key = "%s.%s.%s" % (obj._meta.app_label, obj._meta.module_name, obj.pk) - l = cache.get(key, None) # look in the cache first - if l is None: # if not found in cache - l = Link.objects.for_model(obj) # get it from db - cache.set(key, l, settings.LONG_CACHE_TIME) # and save to cache - return {'links': l} + obj_links = cache.get(key, None) # look in the cache first + if obj_links is None: # if not found in cache + if hasattr(obj, 'links'): + obj_links = obj.links + else: + obj_links = Link.objects.for_model(obj) # get it from db + cache.set(key, obj_links, settings.LONG_CACHE_TIME) # and save to cache + return {'links': obj_links} From 57d16255c562cfeb8f064fff537f6457259ba8ad Mon Sep 17 00:00:00 2001 From: alonisser Date: Sat, 1 Oct 2016 22:06:48 +0300 Subject: [PATCH 09/11] Represent is watching of user in user model --- user/models.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/user/models.py b/user/models.py index 5c7a87ee..be4bcf67 100644 --- a/user/models.py +++ b/user/models.py @@ -74,6 +74,9 @@ def get_actors(self, model, *related): def members(self): return self.get_actors(Member, 'actor') + def is_watching_member(self, a_member): + return a_member in self.members + @property def bills(self): return self.get_actors(Bill, 'actor') From 686191c38efead5be999cf786de7f30025942ca3 Mon Sep 17 00:00:00 2001 From: alonisser Date: Sat, 1 Oct 2016 22:07:25 +0300 Subject: [PATCH 10/11] Represent is watching model in user model and not awkard view codE --- mks/views.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/mks/views.py b/mks/views.py index d856a4fe..cc095420 100644 --- a/mks/views.py +++ b/mks/views.py @@ -267,9 +267,12 @@ class MemberDetailView(DetailView): .select_related('current_party', 'current_party__knesset', 'voting_statistics', - 'awards', - 'awards__award_type') \ - .prefetch_related('parties') + ) \ + .prefetch_related('parties', + 'mmm_documents', + 'awards_and_convictions', + 'person', + 'awards_and_convictions__award_type') MEMBER_INITIAL_DATA = 2 @@ -340,8 +343,8 @@ def get_context_data(self, **kwargs): member = context['object'] current_knesset_start_date = Knesset.objects.current_knesset().start_date if self.request.user.is_authenticated(): - p = self.request.user.profiles.get() - watched = member in p.members + profile = self.request.user.profiles.get() + watched = profile.is_watching_member(member) cached_context = None else: watched = False @@ -418,6 +421,7 @@ def get_context_data(self, **kwargs): for action in actor_stream(member).filter(verb='attended'): i = i + 1 if i == 20: + # JESUS what language are we writing here? and is this a way to do a "limit"? break committee_type = (action and action.target and action.target.committee and From 22438b9f4a4304b20f9b7e3656729910cd3ea785 Mon Sep 17 00:00:00 2001 From: alonisser Date: Sat, 1 Oct 2016 22:08:57 +0300 Subject: [PATCH 11/11] Some optimization in commitee and committee meeting details view, including links handling, Better prefetch and seperating some helper methods --- committees/views.py | 82 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 71 insertions(+), 11 deletions(-) diff --git a/committees/views.py b/committees/views.py index e1a9df0c..ceab6990 100644 --- a/committees/views.py +++ b/committees/views.py @@ -35,7 +35,7 @@ from auxiliary.mixins import GetMoreView from forms import EditTopicForm, LinksFormset from hashnav import method_decorator as hashnav_method_decorator -from knesset.utils import clean_string +from knesset.utils import clean_string, clean_string_no_quotes from laws.models import Bill, PrivateProposal from links.models import Link from mks.models import Member @@ -74,6 +74,14 @@ def get_queryset(self): 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 @@ -88,14 +96,20 @@ def get_context_data(self, *args, **kwargs): # cache.set('committee_detail_%d' % cm.id, cached_context, # settings.LONG_CACHE_TIME) context.update(cached_context) - context['annotations'] = cm.annotations.order_by('-timestamp') - context['topics'] = cm.topic_set.summary()[:5] + return context def _build_context_data(self, cached_context, cm): cached_context['chairpersons'] = cm.chairpersons.all() cached_context['replacements'] = cm.replacements.all() - cached_context['members'] = cm.members_by_presence() + members = cm.members_by_presence() + links = Link.objects.for_model(Member).values() + links_by_member = {} + for k, g in itertools.groupby(links, lambda x: x['object_pk']): + links_by_member[str(k)] = list(g) + for member in members: + member.links = links_by_member.get(str(member.pk), []) + cached_context['members'] = members recent_meetings, more_meetings_available = cm.recent_meetings(limit=self.SEE_ALL_THRESHOLD) cached_context['meetings_list'] = recent_meetings cached_context['more_meetings_available'] = more_meetings_available @@ -107,6 +121,8 @@ def _build_context_data(self, cached_context, cm): end_date=cur_date, limit=self.SEE_ALL_THRESHOLD) cached_context['protocol_not_yet_published_list'] = not_yet_published_meetings cached_context['more_unpublished_available'] = more_unpublished_available + cached_context['annotations'] = cm.annotations.order_by('-timestamp') + cached_context['topics'] = cm.topic_set.summary()[:5] class MeetingDetailView(DetailView): @@ -118,10 +134,51 @@ class MeetingDetailView(DetailView): 'remove-mk': '_handle_remove_mk', 'add-lobbyist': '_handle_add_lobbyist', 'remove-lobbyist': '_handle_remove_lobbyist', - 'protocol': '_handle_protocol', + 'protocol': '_handle_add_protocol', } + def get_object(self, queryset=None): + """ + Returns the object the view is displaying. + + By default this requires `self.queryset` and a `pk` or `slug` argument + in the URLconf, but subclasses can override this to return any object. + """ + # Use a custom queryset if provided; this is required for subclasses + # like DateDetailView + if queryset is None: + queryset = self.get_queryset() + + # Next, try looking up by primary key. + pk = self.kwargs.get(self.pk_url_kwarg, None) + slug = self.kwargs.get(self.slug_url_kwarg, None) + if pk is not None: + #Double prefetch since prefetch does not work over filter + queryset = queryset.filter(pk=pk).prefetch_related('parts', 'parts__speaker__mk', 'lobbyists_mentioned', + 'mks_attended', 'mks_attended__current_party', + 'committee__members', 'committee__chairpersons', + 'committee__replacements') + + # Next, try looking up by slug. + elif slug is not None: + slug_field = self.get_slug_field() + queryset = queryset.filter(**{slug_field: slug}) + + # If none of those are defined, it's an error. + else: + raise AttributeError("Generic detail view %s must be called with " + "either an object pk or a slug." + % self.__class__.__name__) + + try: + # Get the single item from the filtered queryset + obj = queryset.get() + except ObjectDoesNotExist: + raise Http404(_("No %(verbose_name)s found matching the query") % + {'verbose_name': queryset.model._meta.verbose_name}) + return obj + def get_queryset(self): return super(MeetingDetailView, self).get_queryset().select_related('committee') @@ -136,11 +193,7 @@ def get_context_data(self, *args, **kwargs): colors[p] = 'rgb(%i, %i, %i)' % (r, g, b) context['title'] = _('%(committee)s meeting on %(date)s') % {'committee': cm.committee.name, 'date': cm.date_string} - context['description'] = _('%(committee)s meeting on %(date)s on topic %(topic)s') \ - % {'committee': cm.committee.name, - 'date': cm.date_string, - 'topic': cm.topics} - context['description'] = clean_string(context['description']).replace('"', '') + context['description'] = self._resolve_committee_meeting_description(cm) page = self.request.GET.get('page', None) if page: context['description'] += _(' page %(page)s') % {'page': page} @@ -178,6 +231,13 @@ def get_context_data(self, *args, **kwargs): return context + def _resolve_committee_meeting_description(self, committee_meeting): + description = _('%(committee)s meeting on %(date)s on topic %(topic)s') \ + % {'committee': committee_meeting.committee.name, + 'date': committee_meeting.date_string, + 'topic': committee_meeting.topics} + return clean_string_no_quotes(description) + def _resolve_handler_by_user_input_type(self, user_input_type): handler = self._action_handlers.get(user_input_type) return getattr(self, handler) @@ -193,7 +253,7 @@ def post(self, request, **kwargs): return HttpResponseRedirect(".") - def _handle_protocol(self, cm, request): + def _handle_add_protocol(self, cm, request): if not cm.protocol_text: # don't override existing protocols cm.protocol_text = request.POST.get('protocol_text') cm.save()