Merge pull request #5955 from jennifer-richards/rfc-community-lists

fix: Fix tracking of RFCs in CommunityLists

ietf/community/forms.py

@@ -30,6 +30,8 @@ class SearchRuleForm(forms.ModelForm):
         super(SearchRuleForm, self).__init__(*args, **kwargs)
 
         def restrict_state(state_type, slug=None):
+            if "state" not in self.fields:
+                raise RuntimeError(f"Rule type {rule_type} cannot include state filtering")
             f = self.fields['state']
             f.queryset = f.queryset.filter(used=True).filter(type=state_type)
             if slug:
@@ -38,11 +40,15 @@ class SearchRuleForm(forms.ModelForm):
                 f.initial = f.queryset[0].pk
                 f.widget = forms.HiddenInput()
 
+        if rule_type.endswith("_rfc"):
+            del self.fields["state"]  # rfc rules must not look at document states
+
         if rule_type in ["group", "group_rfc", "area", "area_rfc", "group_exp"]:
             if rule_type == "group_exp":
                 restrict_state("draft", "expired")
             else:
-                restrict_state("draft", "rfc" if rule_type.endswith("rfc") else "active")
+                if not rule_type.endswith("_rfc"):
+                    restrict_state("draft", "active")
             
             if rule_type.startswith("area"):
                 self.fields["group"].label = "Area"
@@ -70,7 +76,8 @@ class SearchRuleForm(forms.ModelForm):
             del self.fields["text"]
 
         elif rule_type in ["author", "author_rfc", "shepherd", "ad"]:
-            restrict_state("draft", "rfc" if rule_type.endswith("rfc") else "active")
+            if not rule_type.endswith("_rfc"):
+                restrict_state("draft", "active")
 
             if rule_type.startswith("author"):
                 self.fields["person"].label = "Author"
@@ -84,7 +91,8 @@ class SearchRuleForm(forms.ModelForm):
             del self.fields["text"]
 
         elif rule_type == "name_contains":
-            restrict_state("draft", "rfc" if rule_type.endswith("rfc") else "active")
+            if not rule_type.endswith("_rfc"):
+                restrict_state("draft", "active")
 
             del self.fields["person"]
             del self.fields["group"]

ietf/community/migrations/0003_track_rfcs.py

@@ -0,0 +1,50 @@
+# Generated by Django 4.2.3 on 2023-07-07 18:33
+
+from django.db import migrations
+
+
+def forward(apps, schema_editor):
+    """Track any RFCs that were created from tracked drafts"""
+    CommunityList = apps.get_model("community", "CommunityList")
+    RelatedDocument = apps.get_model("doc", "RelatedDocument")
+
+    # Handle individually tracked documents
+    for cl in CommunityList.objects.all():
+        for rfc in set(
+            RelatedDocument.objects.filter(
+                source__in=cl.added_docs.all(),
+                relationship__slug="became_rfc",
+            ).values_list("target__docs", flat=True)
+        ):
+            cl.added_docs.add(rfc)
+
+    # Handle rules - rules ending with _rfc should no longer filter by state.
+    # There are 9 CommunityLists with invalid author_rfc rules that are filtering
+    # by (draft, active) instead of (draft, rfc) state before migration. All but one
+    # also includes an author rule for (draft, active), so these will start following
+    # RFCs as well. The one exception will start tracking RFCs instead of I-Ds, which
+    # is probably what was intended, but will be a change in their user experience.
+    SearchRule = apps.get_model("community", "SearchRule")
+    rfc_rules = SearchRule.objects.filter(rule_type__endswith="_rfc")
+    rfc_rules.update(state=None)
+
+def reverse(apps, schema_editor):
+    Document = apps.get_model("doc", "Document")
+    for rfc in Document.objects.filter(type__slug="rfc"):
+        rfc.communitylist_set.clear()
+
+    # See the comment above regarding author_rfc
+    SearchRule = apps.get_model("community", "SearchRule")
+    State = apps.get_model("doc", "State")
+    SearchRule.objects.filter(rule_type__endswith="_rfc").update(
+        state=State.objects.get(type_id="draft", slug="rfc")
+    )
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("community", "0002_auto_20230320_1222"),
+        ("doc", "0010_move_rfc_docaliases"),
+    ]
+
+    operations = [migrations.RunPython(forward, reverse)]

ietf/community/tests.py

@@ -151,7 +151,7 @@ class CommunityListTests(WebTest):
             "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="draft", slug="rfc").pk,
+            "author_rfc-state": State.objects.get(type="rfc", slug="published").pk,
         })
         self.assertEqual(r.status_code, 302)
         clist = CommunityList.objects.get(user__username="plain")
@@ -408,4 +408,4 @@ class CommunityListTests(WebTest):
         self.assertEqual(len(outbox), mailbox_before + 1)
         self.assertTrue(draft.name in outbox[-1]["Subject"])
         
-        
+        

ietf/community/utils.py

@@ -71,70 +71,103 @@ def update_name_contains_indexes_with_new_doc(doc):
         if re.search(r.text, doc.name) and not doc in r.name_contains_index.all():
             r.name_contains_index.add(doc)
 
+
 def docs_matching_community_list_rule(rule):
     docs = Document.objects.all()
+    
+    if rule.rule_type.endswith("_rfc"):
+        docs = docs.filter(type_id="rfc")  # rule.state is ignored for RFCs
+    else:
+        docs = docs.filter(type_id="draft", states=rule.state)
+    
     if rule.rule_type in ['group', 'area', 'group_rfc', 'area_rfc']:
-        return docs.filter(Q(group=rule.group_id) | Q(group__parent=rule.group_id), states=rule.state)
+        return docs.filter(Q(group=rule.group_id) | Q(group__parent=rule.group_id))
     elif rule.rule_type in ['group_exp']:
-        return docs.filter(group=rule.group_id, states=rule.state)
+        return docs.filter(group=rule.group_id)
     elif rule.rule_type.startswith("state_"):
-        return docs.filter(states=rule.state)
+        return docs
     elif rule.rule_type in ["author", "author_rfc"]:
-        return docs.filter(states=rule.state, documentauthor__person=rule.person)
+        return docs.filter(documentauthor__person=rule.person)
     elif rule.rule_type == "ad":
-        return docs.filter(states=rule.state, ad=rule.person)
+        return docs.filter(ad=rule.person)
     elif rule.rule_type == "shepherd":
-        return docs.filter(states=rule.state, shepherd__person=rule.person)
+        return docs.filter(shepherd__person=rule.person)
     elif rule.rule_type == "name_contains":
-        return docs.filter(states=rule.state, searchrule=rule)
+        return docs.filter(searchrule=rule)
 
     raise NotImplementedError
 
+
 def community_list_rules_matching_doc(doc):
+    rules = SearchRule.objects.none()
+    if doc.type_id not in ["draft", "rfc"]:
+        return rules  # none
     states = list(doc.states.values_list("pk", flat=True))
 
-    rules = SearchRule.objects.none()
-
+    # group and area rules
     if doc.group_id:
         groups = [doc.group_id]
         if doc.group.parent_id:
             groups.append(doc.group.parent_id)
+        rules_to_add = SearchRule.objects.filter(group__in=groups)
+        if doc.type_id == "rfc":
+            rules_to_add = rules_to_add.filter(rule_type__in=["group_rfc", "area_rfc"])
+        else:
+            rules_to_add = rules_to_add.filter(
+                rule_type__in=["group", "area", "group_exp"],
+                state__in=states,
+            )
+        rules |= rules_to_add
+
+    # state rules (only relevant for I-Ds)
+    if doc.type_id == "draft":
         rules |= SearchRule.objects.filter(
-            rule_type__in=['group', 'area', 'group_rfc', 'area_rfc', 'group_exp'],
+            rule_type__in=[
+                "state_iab",
+                "state_iana",
+                "state_iesg",
+                "state_irtf",
+                "state_ise",
+                "state_rfceditor",
+                "state_ietf",
+            ],
             state__in=states,
-            group__in=groups
         )
 
-    rules |= SearchRule.objects.filter(
-        rule_type__in=['state_iab', 'state_iana', 'state_iesg', 'state_irtf', 'state_ise', 'state_rfceditor', 'state_ietf'],
-        state__in=states,
-    )
-
-    rules |= SearchRule.objects.filter(
-        rule_type__in=["author", "author_rfc"],
-        state__in=states,
-        person__in=list(Person.objects.filter(documentauthor__document=doc)),
-    )
-
-    if doc.ad_id:
+    # author rules
+    if doc.type_id == "rfc":
         rules |= SearchRule.objects.filter(
-            rule_type="ad",
+            rule_type="author_rfc",
+            person__in=list(Person.objects.filter(documentauthor__document=doc)),
+        )
+    else:
+        rules |= SearchRule.objects.filter(
+            rule_type="author",
             state__in=states,
-            person=doc.ad_id,
+            person__in=list(Person.objects.filter(documentauthor__document=doc)),
         )
 
-    if doc.shepherd_id:
-        rules |= SearchRule.objects.filter(
-            rule_type="shepherd",
-            state__in=states,
-            person__email=doc.shepherd_id,
-        )
+    # Other draft-only rules rules
+    if doc.type_id == "draft":
+        if doc.ad_id:
+            rules |= SearchRule.objects.filter(
+                rule_type="ad",
+                state__in=states,
+                person=doc.ad_id,
+            )
 
-    rules |= SearchRule.objects.filter(
-        rule_type="name_contains",
-        state__in=states,
-        name_contains_index=doc, # search our materialized index to avoid full scan
-    )
+        if doc.shepherd_id:
+            rules |= SearchRule.objects.filter(
+                rule_type="shepherd",
+                state__in=states,
+                person__email=doc.shepherd_id,
+            )
+
+        rules |= SearchRule.objects.filter(
+            rule_type="name_contains",
+            state__in=states,
+            name_contains_index=doc,  # search our materialized index to avoid full scan
+        )
 
     return rules
 
@@ -146,7 +179,11 @@ def docs_tracked_by_community_list(clist):
     # in theory, we could use an OR query, but databases seem to have
     # trouble with OR queries and complicated joins so do the OR'ing
     # manually
-    doc_ids = set(clist.added_docs.values_list("pk", flat=True))
+    doc_ids = set()
+    for doc in clist.added_docs.all():
+        doc_ids.add(doc.pk)
+        doc_ids.update(alias.docs.first().pk for alias in doc.related_that_doc("became_rfc"))
+
     for rule in clist.searchrule_set.all():
         doc_ids = doc_ids | set(docs_matching_community_list_rule(rule).values_list("pk", flat=True))
 

ietf/community/views.py

@@ -79,19 +79,18 @@ def manage_list(request, username=None, acronym=None, group_type=None):
         rule_type_form = SearchRuleTypeForm(request.POST)
         if rule_type_form.is_valid():
             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
-                rule.save()
-                if rule.rule_type == "name_contains":
-                    reset_name_contains_index_for_rule(rule)
+            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
+                    rule.save()
+                    if rule.rule_type == "name_contains":
+                        reset_name_contains_index_for_rule(rule)
 
                 return HttpResponseRedirect("")
     else:

ietf/templates/doc/search/search_result_row.html

@@ -49,13 +49,13 @@
         {% if doc.pages %}<small class="float-end text-body-secondary d-none d-sm-block">{{ doc.pages }} page{{ doc.pages|pluralize }}</small>{% endif %}
         <div>
             <a href="{{ doc.get_absolute_url }}">
-                {% if doc.get_state_slug == "rfc" %}
+                {% if doc.type_id == "rfc" %}
                     RFC {{ doc.rfc_number }}
                 {% else %}
                     {{ doc.name }}-{{ doc.rev }}
                 {% endif %}
             </a>
-            {% if doc.get_state_slug == "rfc" and "draft" in doc.name %}<i class="text-body-secondary">(was {{ doc.name }})</i>{% endif %}
+{#            {% if doc.type_id == "rfc" and "draft" in doc.name %}<i class="text-body-secondary">(was {{ doc.name }})</i>{% endif %} TODO drop this or look up the corresponding draft name#}
             <br>
             {% comment %}
             <div class="float-end">
@@ -106,19 +106,19 @@
         {% endif %}
     </td>
     <td>
-            {% if doc.latest_revision_date|timesince_days|new_enough:request and doc.get_state_slug != "rfc" %}
+            {% if doc.latest_revision_date|timesince_days|new_enough:request and doc.type_id != "rfc" %}
                 {% if doc.rev != "00" %}
                     <a href="{{ rfcdiff_base_url }}?url2={{ doc.name }}-{{ doc.rev }}">
                     {% elif doc.replaces %}
                         <a href="{{ rfcdiff_base_url }}?url1={{ doc.replaces_canonical_name }}&amp;url2={{ doc.name }}-{{ doc.rev }}">
                         {% endif %}
                     {% endif %}
-                    {% if doc.get_state_slug == "rfc" %}
+                    {% if doc.type_id == "rfc" %}
                         {{ doc.latest_revision_date|date:"Y-m" }}
                     {% else %}
                         {{ doc.latest_revision_date|date:"Y-m-d" }}
                     {% endif %}
-                    {% if doc.latest_revision_date|timesince_days|new_enough:request and doc.get_state_slug != "rfc" %}
+                    {% if doc.latest_revision_date|timesince_days|new_enough:request and doc.type_id != "rfc" %}
                         {% if doc.rev != "00" or doc.replaces %}</a>{% endif %}
                     {% endif %}
                 {% if doc.latest_revision_date|timesince_days|new_enough:request %}
@@ -127,7 +127,7 @@
                         <span class="badge rounded-pill bg-success">New</span>
                     </div>
                 {% endif %}
-                {% if doc.get_state_slug == "active" and doc.expirable and doc.expires|timesince_days|expires_soon:request %}
+                {% if doc.type_id == "draft" and doc.get_state_slug == "active" and doc.expirable and doc.expires|timesince_days|expires_soon:request %}
                     <br>
                     <span class="badge rounded-pill bg-warning">Expires soon</span>
                 {% endif %}