From e043c85dc218da5802ee63adec634cba5c30cb4e Mon Sep 17 00:00:00 2001 From: Kesara Rathnayake Date: Tue, 22 Jul 2025 04:11:50 +1200 Subject: [PATCH] fix: Community feed (#9184) * style: Apply black / ruff formatting * fix: Community feed --- ietf/community/tests.py | 416 ++++++++++++++++++++++-------- ietf/community/views.py | 259 ++++++++++++------- ietf/templates/community/atom.xml | 8 +- 3 files changed, 477 insertions(+), 206 deletions(-) diff --git a/ietf/community/tests.py b/ietf/community/tests.py index 9bd7789958..1255ba46eb 100644 --- a/ietf/community/tests.py +++ b/ietf/community/tests.py @@ -6,13 +6,20 @@ from django.test.utils import override_settings from django.urls import reverse as urlreverse +from lxml import etree -import debug # pyflakes:ignore +import debug # pyflakes:ignore from ietf.community.models import CommunityList, SearchRule, EmailSubscription from ietf.community.signals import notify_of_event -from ietf.community.utils import docs_matching_community_list_rule, community_list_rules_matching_doc -from ietf.community.utils import reset_name_contains_index_for_rule, notify_event_to_subscribers +from ietf.community.utils import ( + docs_matching_community_list_rule, + community_list_rules_matching_doc, +) +from ietf.community.utils import ( + reset_name_contains_index_for_rule, + notify_event_to_subscribers, +) from ietf.community.tasks import notify_event_to_subscribers_task import ietf.community.views from ietf.group.models import Group @@ -26,35 +33,80 @@ from ietf.group.factories import GroupFactory, RoleFactory from ietf.person.factories import PersonFactory, EmailFactory, AliasFactory + class CommunityListTests(TestCase): def test_rule_matching(self): - plain = PersonFactory(user__username='plain') - ad = Person.objects.get(user__username='ad') + plain = PersonFactory(user__username="plain") + ad = Person.objects.get(user__username="ad") draft = WgDraftFactory( - group__parent=Group.objects.get(acronym='farfut' ), + group__parent=Group.objects.get(acronym="farfut"), authors=[ad], ad=ad, shepherd=plain.email(), - states=[('draft-iesg','lc'),('draft','active')], + states=[("draft-iesg", "lc"), ("draft", "active")], ) clist = CommunityList.objects.create(person=plain) - rule_group = SearchRule.objects.create(rule_type="group", group=draft.group, state=State.objects.get(type="draft", slug="active"), community_list=clist) - rule_group_rfc = SearchRule.objects.create(rule_type="group_rfc", group=draft.group, state=State.objects.get(type="rfc", slug="published"), community_list=clist) - rule_area = SearchRule.objects.create(rule_type="area", group=draft.group.parent, state=State.objects.get(type="draft", slug="active"), community_list=clist) + rule_group = SearchRule.objects.create( + rule_type="group", + group=draft.group, + state=State.objects.get(type="draft", slug="active"), + community_list=clist, + ) + rule_group_rfc = SearchRule.objects.create( + rule_type="group_rfc", + group=draft.group, + state=State.objects.get(type="rfc", slug="published"), + community_list=clist, + ) + rule_area = SearchRule.objects.create( + rule_type="area", + group=draft.group.parent, + state=State.objects.get(type="draft", slug="active"), + community_list=clist, + ) - rule_state_iesg = SearchRule.objects.create(rule_type="state_iesg", state=State.objects.get(type="draft-iesg", slug="lc"), community_list=clist) + rule_state_iesg = SearchRule.objects.create( + rule_type="state_iesg", + state=State.objects.get(type="draft-iesg", slug="lc"), + community_list=clist, + ) - rule_author = SearchRule.objects.create(rule_type="author", state=State.objects.get(type="draft", slug="active"), person=Person.objects.filter(documentauthor__document=draft).first(), community_list=clist) + rule_author = SearchRule.objects.create( + rule_type="author", + state=State.objects.get(type="draft", slug="active"), + person=Person.objects.filter(documentauthor__document=draft).first(), + community_list=clist, + ) - rule_ad = SearchRule.objects.create(rule_type="ad", state=State.objects.get(type="draft", slug="active"), person=draft.ad, community_list=clist) + rule_ad = SearchRule.objects.create( + rule_type="ad", + state=State.objects.get(type="draft", slug="active"), + person=draft.ad, + community_list=clist, + ) - rule_shepherd = SearchRule.objects.create(rule_type="shepherd", state=State.objects.get(type="draft", slug="active"), person=draft.shepherd.person, community_list=clist) + rule_shepherd = SearchRule.objects.create( + rule_type="shepherd", + state=State.objects.get(type="draft", slug="active"), + person=draft.shepherd.person, + community_list=clist, + ) - rule_group_exp = SearchRule.objects.create(rule_type="group_exp", group=draft.group, state=State.objects.get(type="draft", slug="expired"), community_list=clist) + rule_group_exp = SearchRule.objects.create( + rule_type="group_exp", + group=draft.group, + state=State.objects.get(type="draft", slug="expired"), + community_list=clist, + ) - rule_name_contains = SearchRule.objects.create(rule_type="name_contains", state=State.objects.get(type="draft", slug="active"), text="draft-.*" + "-".join(draft.name.split("-")[2:]), community_list=clist) + rule_name_contains = SearchRule.objects.create( + rule_type="name_contains", + state=State.objects.get(type="draft", slug="active"), + text="draft-.*" + "-".join(draft.name.split("-")[2:]), + community_list=clist, + ) reset_name_contains_index_for_rule(rule_name_contains) # doc -> rules @@ -71,29 +123,44 @@ def test_rule_matching(self): # rule -> docs self.assertTrue(draft in list(docs_matching_community_list_rule(rule_group))) - self.assertTrue(draft not in list(docs_matching_community_list_rule(rule_group_rfc))) + self.assertTrue( + draft not in list(docs_matching_community_list_rule(rule_group_rfc)) + ) self.assertTrue(draft in list(docs_matching_community_list_rule(rule_area))) - self.assertTrue(draft in list(docs_matching_community_list_rule(rule_state_iesg))) + self.assertTrue( + draft in list(docs_matching_community_list_rule(rule_state_iesg)) + ) self.assertTrue(draft in list(docs_matching_community_list_rule(rule_author))) self.assertTrue(draft in list(docs_matching_community_list_rule(rule_ad))) self.assertTrue(draft in list(docs_matching_community_list_rule(rule_shepherd))) - self.assertTrue(draft in list(docs_matching_community_list_rule(rule_name_contains))) - self.assertTrue(draft not in list(docs_matching_community_list_rule(rule_group_exp))) + self.assertTrue( + draft in list(docs_matching_community_list_rule(rule_name_contains)) + ) + self.assertTrue( + draft not in list(docs_matching_community_list_rule(rule_group_exp)) + ) - draft.set_state(State.objects.get(type='draft', slug='expired')) + draft.set_state(State.objects.get(type="draft", slug="expired")) # doc -> rules matching_rules = list(community_list_rules_matching_doc(draft)) self.assertTrue(rule_group_exp in matching_rules) # rule -> docs - self.assertTrue(draft in list(docs_matching_community_list_rule(rule_group_exp))) + self.assertTrue( + draft in list(docs_matching_community_list_rule(rule_group_exp)) + ) def test_view_list_duplicates(self): - person = PersonFactory(name="John Q. Public", user__username="bazquux@example.com") + person = PersonFactory( + name="John Q. Public", user__username="bazquux@example.com" + ) PersonFactory(name="John Q. Public", user__username="foobar@example.com") - url = urlreverse(ietf.community.views.view_list, kwargs={ "email_or_name": person.plain_name()}) + url = urlreverse( + ietf.community.views.view_list, + kwargs={"email_or_name": person.plain_name()}, + ) r = self.client.get(url) self.assertEqual(r.status_code, 404) @@ -104,20 +171,23 @@ def complex_person(self, *args, **kwargs): return person def email_or_name_set(self, person): - return [e for e in Email.objects.filter(person=person)] + \ - [a for a in Alias.objects.filter(person=person)] + return [e for e in Email.objects.filter(person=person)] + [ + a for a in Alias.objects.filter(person=person) + ] def do_view_list_test(self, person): draft = WgDraftFactory() # without list for id in self.email_or_name_set(person): - url = urlreverse(ietf.community.views.view_list, kwargs={ "email_or_name": id }) + url = urlreverse( + ietf.community.views.view_list, kwargs={"email_or_name": id} + ) r = self.client.get(url) self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") # with list clist = CommunityList.objects.create(person=person) - if not draft in clist.added_docs.all(): + if draft not in clist.added_docs.all(): clist.added_docs.add(draft) SearchRule.objects.create( community_list=clist, @@ -126,31 +196,37 @@ def do_view_list_test(self, person): text="test", ) for id in self.email_or_name_set(person): - url = urlreverse(ietf.community.views.view_list, kwargs={ "email_or_name": id }) + url = urlreverse( + ietf.community.views.view_list, kwargs={"email_or_name": id} + ) r = self.client.get(url) self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") self.assertContains(r, draft.name) def test_view_list(self): - person = self.complex_person(user__username='plain') + person = self.complex_person(user__username="plain") self.do_view_list_test(person) - + def test_view_list_without_active_email(self): - person = self.complex_person(user__username='plain') + person = self.complex_person(user__username="plain") person.email_set.update(active=False) self.do_view_list_test(person) def test_manage_personal_list(self): - person = self.complex_person(user__username='plain') - ad = Person.objects.get(user__username='ad') + person = self.complex_person(user__username="plain") + ad = Person.objects.get(user__username="ad") draft = WgDraftFactory(authors=[ad]) - url = urlreverse(ietf.community.views.manage_list, kwargs={ "email_or_name": person.email() }) + url = urlreverse( + ietf.community.views.manage_list, kwargs={"email_or_name": person.email()} + ) login_testing_unauthorized(self, "plain", url) for id in self.email_or_name_set(person): - url = urlreverse(ietf.community.views.manage_list, kwargs={ "email_or_name": id }) - r = self.client.get(url, user='plain') + url = urlreverse( + ietf.community.views.manage_list, kwargs={"email_or_name": id} + ) + r = self.client.get(url, user="plain") self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") # We can't call post() with follow=True because that 404's if @@ -158,11 +234,13 @@ def test_manage_personal_list(self): # apparently re-encodes the already-encoded url. def follow(r): redirect_url = r.url or url - return self.client.get(redirect_url, user='plain') + return self.client.get(redirect_url, user="plain") # add document - self.assertContains(r, 'add_document') - r = self.client.post(url, {'action': 'add_documents', 'documents': draft.pk}) + self.assertContains(r, "add_document") + r = self.client.post( + url, {"action": "add_documents", "documents": draft.pk} + ) self.assertEqual(r.status_code, 302, msg=f"id='{id}', url='{url}'") clist = CommunityList.objects.get(person__user__username="plain") self.assertTrue(clist.added_docs.filter(pk=draft.pk)) @@ -170,8 +248,10 @@ def follow(r): self.assertContains(r, draft.name, status_code=200) # remove document - self.assertContains(r, 'remove_document_%s' % draft.pk) - r = self.client.post(url, {'action': 'remove_document', 'document': draft.pk}) + self.assertContains(r, "remove_document_%s" % draft.pk) + r = self.client.post( + url, {"action": "remove_document", "document": draft.pk} + ) self.assertEqual(r.status_code, 302, msg=f"id='{id}', url='{url}'") clist = CommunityList.objects.get(person__user__username="plain") self.assertTrue(not clist.added_docs.filter(pk=draft.pk)) @@ -179,23 +259,37 @@ def follow(r): self.assertNotContains(r, draft.name, status_code=200) # add rule - r = self.client.post(url, { - "action": "add_rule", - "rule_type": "author_rfc", - "author_rfc-person": Person.objects.filter(documentauthor__document=draft).first().pk, - "author_rfc-state": State.objects.get(type="rfc", slug="published").pk, - }) + r = self.client.post( + url, + { + "action": "add_rule", + "rule_type": "author_rfc", + "author_rfc-person": Person.objects.filter( + documentauthor__document=draft + ) + .first() + .pk, + "author_rfc-state": State.objects.get( + type="rfc", slug="published" + ).pk, + }, + ) self.assertEqual(r.status_code, 302, msg=f"id='{id}', url='{url}'") clist = CommunityList.objects.get(person__user__username="plain") self.assertTrue(clist.searchrule_set.filter(rule_type="author_rfc")) # add name_contains rule - r = self.client.post(url, { - "action": "add_rule", - "rule_type": "name_contains", - "name_contains-text": "draft.*mars", - "name_contains-state": State.objects.get(type="draft", slug="active").pk, - }) + r = self.client.post( + url, + { + "action": "add_rule", + "rule_type": "name_contains", + "name_contains-text": "draft.*mars", + "name_contains-state": State.objects.get( + type="draft", slug="active" + ).pk, + }, + ) self.assertEqual(r.status_code, 302, msg=f"id='{id}', url='{url}'") clist = CommunityList.objects.get(person__user__username="plain") self.assertTrue(clist.searchrule_set.filter(rule_type="name_contains")) @@ -205,22 +299,31 @@ def follow(r): self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") rule = clist.searchrule_set.filter(rule_type="author_rfc").first() q = PyQuery(r.content) - self.assertEqual(len(q('#r%s' % rule.pk)), 1) + self.assertEqual(len(q("#r%s" % rule.pk)), 1) # remove rule - r = self.client.post(url, { - "action": "remove_rule", - "rule": rule.pk, - }) + r = self.client.post( + url, + { + "action": "remove_rule", + "rule": rule.pk, + }, + ) clist = CommunityList.objects.get(person__user__username="plain") self.assertTrue(not clist.searchrule_set.filter(rule_type="author_rfc")) def test_manage_group_list(self): - draft = WgDraftFactory(group__acronym='mars') - RoleFactory(group__acronym='mars',name_id='chair',person=PersonFactory(user__username='marschairman')) + draft = WgDraftFactory(group__acronym="mars") + RoleFactory( + group__acronym="mars", + name_id="chair", + person=PersonFactory(user__username="marschairman"), + ) - url = urlreverse(ietf.community.views.manage_list, kwargs={ "acronym": draft.group.acronym }) + url = urlreverse( + ietf.community.views.manage_list, kwargs={"acronym": draft.group.acronym} + ) setup_default_community_list_for_group(draft.group) login_testing_unauthorized(self, "marschairman", url) @@ -229,27 +332,41 @@ def test_manage_group_list(self): self.assertEqual(r.status_code, 200) # Verify GET also works with non-WG and RG groups - for gtype in ['area','program']: + for gtype in ["area", "program"]: g = GroupFactory.create(type_id=gtype) # make sure the group's features have been initialized to improve coverage - _ = g.features # pyflakes:ignore + _ = g.features # pyflakes:ignore p = PersonFactory() - g.role_set.create(name_id={'area':'ad','program':'lead'}[gtype],person=p, email=p.email()) - url = urlreverse(ietf.community.views.manage_list, kwargs={ "acronym": g.acronym }) + g.role_set.create( + name_id={"area": "ad", "program": "lead"}[gtype], + person=p, + email=p.email(), + ) + url = urlreverse( + ietf.community.views.manage_list, kwargs={"acronym": g.acronym} + ) setup_default_community_list_for_group(g) - self.client.login(username=p.user.username,password=p.user.username+"+password") + self.client.login( + username=p.user.username, password=p.user.username + "+password" + ) r = self.client.get(url) self.assertEqual(r.status_code, 200) def test_track_untrack_document(self): - person = self.complex_person(user__username='plain') + person = self.complex_person(user__username="plain") draft = WgDraftFactory() - url = urlreverse(ietf.community.views.track_document, kwargs={ "email_or_name": person.email(), "name": draft.name }) + url = urlreverse( + ietf.community.views.track_document, + kwargs={"email_or_name": person.email(), "name": draft.name}, + ) login_testing_unauthorized(self, "plain", url) for id in self.email_or_name_set(person): - url = urlreverse(ietf.community.views.track_document, kwargs={ "email_or_name": id, "name": draft.name }) + url = urlreverse( + ietf.community.views.track_document, + kwargs={"email_or_name": id, "name": draft.name}, + ) # track r = self.client.get(url) @@ -261,7 +378,10 @@ def test_track_untrack_document(self): self.assertEqual(list(clist.added_docs.all()), [draft]) # untrack - url = urlreverse(ietf.community.views.untrack_document, kwargs={ "email_or_name": id, "name": draft.name }) + url = urlreverse( + ietf.community.views.untrack_document, + kwargs={"email_or_name": id, "name": draft.name}, + ) r = self.client.get(url) self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") @@ -271,36 +391,47 @@ def test_track_untrack_document(self): self.assertEqual(list(clist.added_docs.all()), []) def test_track_untrack_document_through_ajax(self): - person = self.complex_person(user__username='plain') + person = self.complex_person(user__username="plain") draft = WgDraftFactory() - url = urlreverse(ietf.community.views.track_document, kwargs={ "email_or_name": person.email(), "name": draft.name }) + url = urlreverse( + ietf.community.views.track_document, + kwargs={"email_or_name": person.email(), "name": draft.name}, + ) login_testing_unauthorized(self, "plain", url) for id in self.email_or_name_set(person): - url = urlreverse(ietf.community.views.track_document, kwargs={ "email_or_name": id, "name": draft.name }) + url = urlreverse( + ietf.community.views.track_document, + kwargs={"email_or_name": id, "name": draft.name}, + ) # track - r = self.client.post(url, HTTP_X_REQUESTED_WITH='XMLHttpRequest') + r = self.client.post(url, HTTP_X_REQUESTED_WITH="XMLHttpRequest") self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") self.assertEqual(r.json()["success"], True) clist = CommunityList.objects.get(person__user__username="plain") self.assertEqual(list(clist.added_docs.all()), [draft]) # untrack - url = urlreverse(ietf.community.views.untrack_document, kwargs={ "email_or_name": id, "name": draft.name }) - r = self.client.post(url, HTTP_X_REQUESTED_WITH='XMLHttpRequest') + url = urlreverse( + ietf.community.views.untrack_document, + kwargs={"email_or_name": id, "name": draft.name}, + ) + r = self.client.post(url, HTTP_X_REQUESTED_WITH="XMLHttpRequest") self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") self.assertEqual(r.json()["success"], True) clist = CommunityList.objects.get(person__user__username="plain") self.assertEqual(list(clist.added_docs.all()), []) def test_csv(self): - person = self.complex_person(user__username='plain') + person = self.complex_person(user__username="plain") draft = WgDraftFactory() for id in self.email_or_name_set(person): - url = urlreverse(ietf.community.views.export_to_csv, kwargs={ "email_or_name": id }) + url = urlreverse( + ietf.community.views.export_to_csv, kwargs={"email_or_name": id} + ) # without list r = self.client.get(url) @@ -308,7 +439,7 @@ def test_csv(self): # with list clist = CommunityList.objects.create(person=person) - if not draft in clist.added_docs.all(): + if draft not in clist.added_docs.all(): clist.added_docs.add(draft) SearchRule.objects.create( community_list=clist, @@ -324,7 +455,9 @@ def test_csv(self): def test_csv_for_group(self): draft = WgDraftFactory() - url = urlreverse(ietf.community.views.export_to_csv, kwargs={ "acronym": draft.group.acronym }) + url = urlreverse( + ietf.community.views.export_to_csv, kwargs={"acronym": draft.group.acronym} + ) setup_default_community_list_for_group(draft.group) @@ -333,11 +466,11 @@ def test_csv_for_group(self): self.assertEqual(r.status_code, 200) def test_feed(self): - person = self.complex_person(user__username='plain') + person = self.complex_person(user__username="plain") draft = WgDraftFactory() for id in self.email_or_name_set(person): - url = urlreverse(ietf.community.views.feed, kwargs={ "email_or_name": id }) + url = urlreverse(ietf.community.views.feed, kwargs={"email_or_name": id}) # without list r = self.client.get(url) @@ -345,7 +478,7 @@ def test_feed(self): # with list clist = CommunityList.objects.create(person=person) - if not draft in clist.added_docs.all(): + if draft not in clist.added_docs.all(): clist.added_docs.add(draft) SearchRule.objects.create( community_list=clist, @@ -357,31 +490,53 @@ def test_feed(self): self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") self.assertContains(r, draft.name) + # test atom xml + xml = etree.fromstring(r.content) + ns = {"atom": "http://www.w3.org/2005/Atom"} + updated = xml.xpath("/atom:feed/atom:updated", namespaces=ns)[0].text + entries = xml.xpath("/atom:feed/atom:entry", namespaces=ns) + self.assertIn("+00:00", updated) # RFC 3339 compatible UTC TZ + for entry in entries: + updated = entry.xpath("atom:updated", namespaces=ns)[0].text + published = entry.xpath("atom:published", namespaces=ns)[0].text + entry_id = entry.xpath("atom:id", namespaces=ns)[0].text + self.assertIn("+00:00", updated) + self.assertIn("+00:00", published) + self.assertIn( + "urn:datatracker-ietf-org:event:", entry_id + ) # atom:entry:id must be a valid URN + # only significant r = self.client.get(url + "?significant=1") self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") - self.assertNotContains(r, '') + self.assertNotContains(r, "") def test_feed_for_group(self): draft = WgDraftFactory() - url = urlreverse(ietf.community.views.feed, kwargs={ "acronym": draft.group.acronym }) + url = urlreverse( + ietf.community.views.feed, kwargs={"acronym": draft.group.acronym} + ) setup_default_community_list_for_group(draft.group) # test GET, rest is tested with personal list r = self.client.get(url) self.assertEqual(r.status_code, 200) - + def test_subscription(self): - person = self.complex_person(user__username='plain') + person = self.complex_person(user__username="plain") draft = WgDraftFactory() - url = urlreverse(ietf.community.views.subscription, kwargs={ "email_or_name": person.email() }) + url = urlreverse( + ietf.community.views.subscription, kwargs={"email_or_name": person.email()} + ) login_testing_unauthorized(self, "plain", url) for id in self.email_or_name_set(person): - url = urlreverse(ietf.community.views.subscription, kwargs={ "email_or_name": id }) + url = urlreverse( + ietf.community.views.subscription, kwargs={"email_or_name": id} + ) # subscription without list r = self.client.get(url) @@ -389,7 +544,7 @@ def test_subscription(self): # subscription with list clist = CommunityList.objects.create(person=person) - if not draft in clist.added_docs.all(): + if draft not in clist.added_docs.all(): clist.added_docs.add(draft) SearchRule.objects.create( community_list=clist, @@ -399,29 +554,49 @@ def test_subscription(self): ) for email in Email.objects.filter(person=person): - url = urlreverse(ietf.community.views.subscription, kwargs={ "email_or_name": email }) + url = urlreverse( + ietf.community.views.subscription, kwargs={"email_or_name": email} + ) r = self.client.get(url) self.assertEqual(r.status_code, 200) # subscribe - r = self.client.post(url, { "email": email.pk, "notify_on": "significant", "action": "subscribe" }) + r = self.client.post( + url, + {"email": email.pk, "notify_on": "significant", "action": "subscribe"}, + ) self.assertEqual(r.status_code, 302) - subscription = EmailSubscription.objects.filter(community_list=clist, email=email, notify_on="significant").first() + subscription = EmailSubscription.objects.filter( + community_list=clist, email=email, notify_on="significant" + ).first() self.assertTrue(subscription) # delete subscription - r = self.client.post(url, { "subscription_id": subscription.pk, "action": "unsubscribe" }) + r = self.client.post( + url, {"subscription_id": subscription.pk, "action": "unsubscribe"} + ) self.assertEqual(r.status_code, 302) - self.assertEqual(EmailSubscription.objects.filter(community_list=clist, email=email, notify_on="significant").count(), 0) + self.assertEqual( + EmailSubscription.objects.filter( + community_list=clist, email=email, notify_on="significant" + ).count(), + 0, + ) def test_subscription_for_group(self): - draft = WgDraftFactory(group__acronym='mars') - RoleFactory(group__acronym='mars',name_id='chair',person=PersonFactory(user__username='marschairman')) + draft = WgDraftFactory(group__acronym="mars") + RoleFactory( + group__acronym="mars", + name_id="chair", + person=PersonFactory(user__username="marschairman"), + ) - url = urlreverse(ietf.community.views.subscription, kwargs={ "acronym": draft.group.acronym }) + url = urlreverse( + ietf.community.views.subscription, kwargs={"acronym": draft.group.acronym} + ) setup_default_community_list_for_group(draft.group) @@ -434,7 +609,7 @@ def test_subscription_for_group(self): @mock.patch("ietf.community.signals.notify_of_event") def test_notification_signal_receiver(self, mock_notify_of_event): """Saving a newly created DocEvent should notify subscribers - + This implicitly tests that notify_of_event_receiver is hooked up to the post_save signal. """ # Arbitrary model that's not a DocEvent @@ -442,19 +617,26 @@ def test_notification_signal_receiver(self, mock_notify_of_event): mock_notify_of_event.reset_mock() # clear any calls that resulted from the factories person.save() self.assertFalse(mock_notify_of_event.called) - + # build a DocEvent that is not yet persisted doc = DocumentFactory() event = DocEventFactory.build(by=person, doc=doc) # builds but does not save... mock_notify_of_event.reset_mock() # clear any calls that resulted from the factories event.save() - self.assertEqual(mock_notify_of_event.call_count, 1, "notify_task should be run on creation of DocEvent") + self.assertEqual( + mock_notify_of_event.call_count, + 1, + "notify_task should be run on creation of DocEvent", + ) self.assertEqual(mock_notify_of_event.call_args, mock.call(event)) - # save the existing DocEvent and see that no notification is sent + # save the existing DocEvent and see that no notification is sent mock_notify_of_event.reset_mock() event.save() - self.assertFalse(mock_notify_of_event.called, "notify_task should not be run save of on existing DocEvent") + self.assertFalse( + mock_notify_of_event.called, + "notify_task should not be run save of on existing DocEvent", + ) # Mock out the on_commit call so we can tell whether the task was actually queued @mock.patch("ietf.submit.views.transaction.on_commit", side_effect=lambda x: x()) @@ -468,7 +650,10 @@ def test_notify_of_event(self, mock_notify_task, mock_on_commit): # under test does not make this call when in "test" mode with override_settings(SERVER_MODE="not-test"): notify_of_event(event) - self.assertTrue(mock_notify_task.delay.called, "notify_task should run for a DocEvent on a draft") + self.assertTrue( + mock_notify_task.delay.called, + "notify_task should run for a DocEvent on a draft", + ) mock_notify_task.reset_mock() event.skip_community_list_notification = True @@ -476,22 +661,28 @@ def test_notify_of_event(self, mock_notify_task, mock_on_commit): # under test does not make this call when in "test" mode with override_settings(SERVER_MODE="not-test"): notify_of_event(event) - self.assertFalse(mock_notify_task.delay.called, "notify_task should not run when skip_community_list_notification is set") + self.assertFalse( + mock_notify_task.delay.called, + "notify_task should not run when skip_community_list_notification is set", + ) event = DocEventFactory.build(by=person, doc=DocumentFactory(type_id="rfc")) # be careful overriding SERVER_MODE - we do it here because the method # under test does not make this call when in "test" mode with override_settings(SERVER_MODE="not-test"): notify_of_event(event) - self.assertFalse(mock_notify_task.delay.called, "notify_task should not run on a document with type 'rfc'") + self.assertFalse( + mock_notify_task.delay.called, + "notify_task should not run on a document with type 'rfc'", + ) @mock.patch("ietf.utils.mail.send_mail_text") def test_notify_event_to_subscribers(self, mock_send_mail_text): - person = PersonFactory(user__username='plain') + person = PersonFactory(user__username="plain") draft = WgDraftFactory() clist = CommunityList.objects.create(person=person) - if not draft in clist.added_docs.all(): + if draft not in clist.added_docs.all(): clist.added_docs.add(draft) sub_to_significant = EmailSubscription.objects.create( @@ -522,11 +713,13 @@ def test_notify_event_to_subscribers(self, mock_send_mail_text): mock_send_mail_text.reset_mock() notify_event_to_subscribers(event) self.assertEqual(mock_send_mail_text.call_count, 2) - addresses = [call_args[0][1] for call_args in mock_send_mail_text.call_args_list] + addresses = [ + call_args[0][1] for call_args in mock_send_mail_text.call_args_list + ] subjects = {call_args[0][3] for call_args in mock_send_mail_text.call_args_list} contents = {call_args[0][4] for call_args in mock_send_mail_text.call_args_list} self.assertCountEqual( - addresses, + addresses, [sub_to_significant.email.address, sub_to_all.email.address], ) self.assertEqual(len(subjects), 1) @@ -545,4 +738,3 @@ def test_notify_event_to_subscribers_task(self, mock_notify): d.delete() notify_event_to_subscribers_task(event_id=d.pk) self.assertFalse(mock_notify.called) - diff --git a/ietf/community/views.py b/ietf/community/views.py index 923ec556f3..08b1c24fe5 100644 --- a/ietf/community/views.py +++ b/ietf/community/views.py @@ -13,13 +13,24 @@ from django.utils import timezone from django.utils.html import strip_tags -import debug # pyflakes:ignore +import debug # pyflakes:ignore from ietf.community.models import CommunityList, EmailSubscription, SearchRule -from ietf.community.forms import SearchRuleTypeForm, SearchRuleForm, AddDocumentsForm, SubscriptionForm +from ietf.community.forms import ( + SearchRuleTypeForm, + SearchRuleForm, + AddDocumentsForm, + SubscriptionForm, +) from ietf.community.utils import can_manage_community_list -from ietf.community.utils import docs_tracked_by_community_list, docs_matching_community_list_rule -from ietf.community.utils import states_of_significant_change, reset_name_contains_index_for_rule +from ietf.community.utils import ( + docs_tracked_by_community_list, + docs_matching_community_list_rule, +) +from ietf.community.utils import ( + states_of_significant_change, + reset_name_contains_index_for_rule, +) from ietf.group.models import Group from ietf.doc.models import DocEvent, Document from ietf.doc.utils_search import prepare_document_table @@ -31,43 +42,61 @@ def lookup_community_list(request, email_or_name=None, acronym=None): """Finds a CommunityList for a person or group - + Instantiates an unsaved CommunityList if one is not found. - + If the person or group cannot be found and uniquely identified, raises an Http404 exception """ assert email_or_name or acronym if acronym: group = get_object_or_404(Group, acronym=acronym) - clist = CommunityList.objects.filter(group=group).first() or CommunityList(group=group) + clist = CommunityList.objects.filter(group=group).first() or CommunityList( + group=group + ) else: persons = lookup_persons(email_or_name) if len(persons) > 1: - if hasattr(request.user, 'person') and request.user.person in persons: + if hasattr(request.user, "person") and request.user.person in persons: person = request.user.person else: - raise Http404(f"Unable to identify the CommunityList for {email_or_name}") + raise Http404( + f"Unable to identify the CommunityList for {email_or_name}" + ) else: person = persons[0] - clist = CommunityList.objects.filter(person=person).first() or CommunityList(person=person) + clist = CommunityList.objects.filter(person=person).first() or CommunityList( + person=person + ) return clist + def view_list(request, email_or_name=None): clist = lookup_community_list(request, email_or_name) # may raise Http404 docs = docs_tracked_by_community_list(clist) docs, meta = prepare_document_table(request, docs, request.GET) - subscribed = request.user.is_authenticated and (EmailSubscription.objects.none() if clist.pk is None else EmailSubscription.objects.filter(community_list=clist, email__person__user=request.user)) + subscribed = request.user.is_authenticated and ( + EmailSubscription.objects.none() + if clist.pk is None + else EmailSubscription.objects.filter( + community_list=clist, email__person__user=request.user + ) + ) + + return render( + request, + "community/view_list.html", + { + "clist": clist, + "docs": docs, + "meta": meta, + "can_manage_list": can_manage_community_list(request.user, clist), + "subscribed": subscribed, + "email_or_name": email_or_name, + }, + ) - return render(request, 'community/view_list.html', { - 'clist': clist, - 'docs': docs, - 'meta': meta, - 'can_manage_list': can_manage_community_list(request.user, clist), - 'subscribed': subscribed, - "email_or_name": email_or_name, - }) @login_required @ignore_view_kwargs("group_type") @@ -79,24 +108,24 @@ def manage_list(request, email_or_name=None, acronym=None): if not can_manage_community_list(request.user, clist): permission_denied(request, "You do not have permission to access this view") - action = request.POST.get('action') + action = request.POST.get("action") - if request.method == 'POST' and action == 'add_documents': + if request.method == "POST" and action == "add_documents": add_doc_form = AddDocumentsForm(request.POST) if add_doc_form.is_valid(): if clist.pk is None: clist.save() - for d in add_doc_form.cleaned_data['documents']: - if not d in clist.added_docs.all(): + for d in add_doc_form.cleaned_data["documents"]: + if d not in clist.added_docs.all(): clist.added_docs.add(d) return HttpResponseRedirect("") else: add_doc_form = AddDocumentsForm() - if request.method == 'POST' and action == 'remove_document': - document_id = request.POST.get('document') + if request.method == "POST" and action == "remove_document": + document_id = request.POST.get("document") if clist.pk is not None and document_id: document = get_object_or_404(clist.added_docs, id=document_id) clist.added_docs.remove(document) @@ -104,16 +133,16 @@ def manage_list(request, email_or_name=None, acronym=None): return HttpResponseRedirect("") rule_form = None - if request.method == 'POST' and action == 'add_rule': + if request.method == "POST" and action == "add_rule": rule_type_form = SearchRuleTypeForm(request.POST) if rule_type_form.is_valid(): - rule_type = rule_type_form.cleaned_data['rule_type'] + rule_type = rule_type_form.cleaned_data["rule_type"] if rule_type: rule_form = SearchRuleForm(clist, rule_type, request.POST) if rule_form.is_valid(): if clist.pk is None: clist.save() - + rule = rule_form.save(commit=False) rule.community_list = clist rule.rule_type = rule_type @@ -125,8 +154,8 @@ def manage_list(request, email_or_name=None, acronym=None): else: rule_type_form = SearchRuleTypeForm() - if request.method == 'POST' and action == 'remove_rule': - rule_pk = request.POST.get('rule') + if request.method == "POST" and action == "remove_rule": + rule_pk = request.POST.get("rule") if clist.pk is not None and rule_pk: rule = get_object_or_404(SearchRule, pk=rule_pk, community_list=clist) rule.delete() @@ -137,23 +166,35 @@ def manage_list(request, email_or_name=None, acronym=None): for r in rules: r.matching_documents_count = docs_matching_community_list_rule(r).count() - empty_rule_forms = { rule_type: SearchRuleForm(clist, rule_type) for rule_type, _ in SearchRule.RULE_TYPES } + empty_rule_forms = { + rule_type: SearchRuleForm(clist, rule_type) + for rule_type, _ in SearchRule.RULE_TYPES + } total_count = docs_tracked_by_community_list(clist).count() - all_forms = [f for f in [rule_type_form, rule_form, add_doc_form, *empty_rule_forms.values()] - if f is not None] - return render(request, 'community/manage_list.html', { - 'clist': clist, - 'rules': rules, - 'individually_added': clist.added_docs.all() if clist.pk is not None else [], - 'rule_type_form': rule_type_form, - 'rule_form': rule_form, - 'empty_rule_forms': empty_rule_forms, - 'total_count': total_count, - 'add_doc_form': add_doc_form, - 'all_forms': all_forms, - }) + all_forms = [ + f + for f in [rule_type_form, rule_form, add_doc_form, *empty_rule_forms.values()] + if f is not None + ] + return render( + request, + "community/manage_list.html", + { + "clist": clist, + "rules": rules, + "individually_added": ( + clist.added_docs.all() if clist.pk is not None else [] + ), + "rule_type_form": rule_type_form, + "rule_form": rule_form, + "empty_rule_forms": empty_rule_forms, + "total_count": total_count, + "add_doc_form": add_doc_form, + "all_forms": all_forms, + }, + ) @login_required @@ -161,24 +202,33 @@ def track_document(request, name, email_or_name=None, acronym=None): doc = get_object_or_404(Document, name=name) if request.method == "POST": - clist = lookup_community_list(request, email_or_name, acronym) # may raise Http404 + clist = lookup_community_list( + request, email_or_name, acronym + ) # may raise Http404 if not can_manage_community_list(request.user, clist): permission_denied(request, "You do not have permission to access this view") if clist.pk is None: clist.save() - if not doc in clist.added_docs.all(): + if doc not in clist.added_docs.all(): clist.added_docs.add(doc) if is_ajax(request): - return HttpResponse(json.dumps({ 'success': True }), content_type='application/json') + return HttpResponse( + json.dumps({"success": True}), content_type="application/json" + ) else: return HttpResponseRedirect(clist.get_absolute_url()) - return render(request, "community/track_document.html", { - "name": doc.name, - }) + return render( + request, + "community/track_document.html", + { + "name": doc.name, + }, + ) + @login_required def untrack_document(request, name, email_or_name=None, acronym=None): @@ -192,28 +242,34 @@ def untrack_document(request, name, email_or_name=None, acronym=None): clist.added_docs.remove(doc) if is_ajax(request): - return HttpResponse(json.dumps({ 'success': True }), content_type='application/json') + return HttpResponse( + json.dumps({"success": True}), content_type="application/json" + ) else: return HttpResponseRedirect(clist.get_absolute_url()) - return render(request, "community/untrack_document.html", { - "name": doc.name, - }) + return render( + request, + "community/untrack_document.html", + { + "name": doc.name, + }, + ) @ignore_view_kwargs("group_type") def export_to_csv(request, email_or_name=None, acronym=None): clist = lookup_community_list(request, email_or_name, acronym) # may raise Http404 - response = HttpResponse(content_type='text/csv') + response = HttpResponse(content_type="text/csv") if clist.group: filename = "%s-draft-list.csv" % clist.group.acronym else: filename = "draft-list.csv" - response['Content-Disposition'] = 'attachment; filename=%s' % filename + response["Content-Disposition"] = "attachment; filename=%s" % filename - writer = csv.writer(response, dialect=csv.excel, delimiter=str(',')) + writer = csv.writer(response, dialect=csv.excel, delimiter=str(",")) header = [ "Name", @@ -226,12 +282,12 @@ def export_to_csv(request, email_or_name=None, acronym=None): ] writer.writerow(header) - docs = docs_tracked_by_community_list(clist).select_related('type', 'group', 'ad') + docs = docs_tracked_by_community_list(clist).select_related("type", "group", "ad") for doc in docs.prefetch_related("states", "tags"): row = [] row.append(doc.name) row.append(doc.title) - e = doc.latest_event(type='new_revision') + e = doc.latest_event(type="new_revision") row.append(e.time.strftime("%Y-%m-%d") if e else "") row.append(strip_tags(doc.friendly_state())) row.append(doc.group.acronym if doc.group else "") @@ -242,39 +298,54 @@ def export_to_csv(request, email_or_name=None, acronym=None): return response + @ignore_view_kwargs("group_type") def feed(request, email_or_name=None, acronym=None): clist = lookup_community_list(request, email_or_name, acronym) # may raise Http404 - significant = request.GET.get('significant', '') == '1' - - documents = docs_tracked_by_community_list(clist).values_list('pk', flat=True) - since = timezone.now() - datetime.timedelta(days=14) - - events = DocEvent.objects.filter( - doc__id__in=documents, - time__gte=since, - ).distinct().order_by('-time', '-id').select_related("doc") + significant = request.GET.get("significant", "") == "1" + + documents = docs_tracked_by_community_list(clist).values_list("pk", flat=True) + updated = timezone.now() + since = updated - datetime.timedelta(days=14) + + events = ( + DocEvent.objects.filter( + doc__id__in=documents, + time__gte=since, + ) + .distinct() + .order_by("-time", "-id") + .select_related("doc") + ) if significant: - events = events.filter(type="changed_state", statedocevent__state__in=list(states_of_significant_change())) + events = events.filter( + type="changed_state", + statedocevent__state__in=list(states_of_significant_change()), + ) host = request.get_host() - feed_url = 'https://%s%s' % (host, request.get_full_path()) + feed_url = "https://%s%s" % (host, request.get_full_path()) feed_id = uuid.uuid5(uuid.NAMESPACE_URL, str(feed_url)) - title = '%s RSS Feed' % clist.long_name() + title = "%s RSS Feed" % clist.long_name() if significant: - subtitle = 'Significant document changes' + subtitle = "Significant document changes" else: - subtitle = 'Document changes' - - return render(request, 'community/atom.xml', { - 'clist': clist, - 'entries': events[:50], - 'title': title, - 'subtitle': subtitle, - 'id': feed_id.urn, - 'updated': timezone.now(), - }, content_type='text/xml') + subtitle = "Document changes" + + return render( + request, + "community/atom.xml", + { + "clist": clist, + "entries": events[:50], + "title": title, + "subtitle": subtitle, + "id": feed_id.urn, + "updated": updated, + }, + content_type="text/xml", + ) @login_required @@ -286,9 +357,11 @@ def subscription(request, email_or_name=None, acronym=None): person = request.user.person - existing_subscriptions = EmailSubscription.objects.filter(community_list=clist, email__person=person) + existing_subscriptions = EmailSubscription.objects.filter( + community_list=clist, email__person=person + ) - if request.method == 'POST': + if request.method == "POST": action = request.POST.get("action") if action == "subscribe": form = SubscriptionForm(person, clist, request.POST) @@ -300,14 +373,20 @@ def subscription(request, email_or_name=None, acronym=None): return HttpResponseRedirect("") elif action == "unsubscribe": - existing_subscriptions.filter(pk=request.POST.get("subscription_id")).delete() + existing_subscriptions.filter( + pk=request.POST.get("subscription_id") + ).delete() return HttpResponseRedirect("") else: form = SubscriptionForm(person, clist) - return render(request, 'community/subscription.html', { - 'clist': clist, - 'form': form, - 'existing_subscriptions': existing_subscriptions, - }) + return render( + request, + "community/subscription.html", + { + "clist": clist, + "form": form, + "existing_subscriptions": existing_subscriptions, + }, + ) diff --git a/ietf/templates/community/atom.xml b/ietf/templates/community/atom.xml index 32e3b00292..01dcdfeee7 100644 --- a/ietf/templates/community/atom.xml +++ b/ietf/templates/community/atom.xml @@ -3,7 +3,7 @@ {{ title }} {{ subtitle }} {{ id }} - {{ updated|date:"Y-m-d\TH:i:sO" }} + {{ updated.isoformat }} @@ -17,11 +17,11 @@ - {{ entry.id }} + urn:datatracker-ietf-org:event:{{ entry.id }} - {{ entry.time|date:"Y-m-d\TH:i:sO" }} + {{ entry.time.isoformat }} - {{ entry.time|date:"Y-m-d\TH:i:sO" }} + {{ entry.time.isoformat }} {{ entry.by }}