feat: show expired WG/RG drafts at WG/RG Documents page #4252
Conversation
|
I think this is taking a very reasonable approach. We need tests of the new concept though. The setup that tests the WG document page should build some docs that are expired (and some that are dead) and make sure the right thing happens when the new rule is both present and absent. |
ietf/community/forms.py
Outdated
| if rule_type in ['group', 'group_rfc', 'area', 'area_rfc']: | ||
| restrict_state("draft", "rfc" if rule_type.endswith("rfc") else "active") | ||
| if rule_type in ['group', 'group_rfc', 'area', 'area_rfc', 'group_exp']: | ||
| restrict_state("draft", "rfc" if rule_type.endswith("rfc") else "expired" if rule_type.endswith("exp") else "active") |
There was a problem hiding this comment.
This will be hard for future contributors to trust (what's the precedence of each if?). Requires a moderate python-steeping to know.
I think it would be better to make this more verbose and split the computation of the wanted state slug to multiple lines.
There was a problem hiding this comment.
OK, will try to split.
There was a problem hiding this comment.
I made the changes you requested - splited the complex condition into two "if" statements and added the tests for both community and group tests (the latter is needed because of some post-filtering, which doesn't show expired drafts if they are dead). I also fixed a logical error - extraneous check whether the "draft" slug is in "auth-rm" or "ietf-rm" (as far as I understand this can never happen when the "draft" is "expired"). Correct me if I'm wrong - the states of the document are cumbersome and I didn't find clear explanation of their relations. I also fixed the last minute error when I renamed auto-generated migration files and forgot to also change "dependencies' in the latter of them. I think the PR is ready for merging.
|
@larseggert, @evyncke This includes a migration to add this community rule to all active rg/wg. Any groups that don't want expired drafts to show on their group/document pages would need to remove the rule. Is that acceptable to the IESG? |
fix dependency on migration file name
Simplify condition statements
|
I don't have a strong feeling here. We got to pick one default, and enabling it seems OK to me. |
Fix - remove erroneous check (never happen)
|
Same opinion as Lars' one: let's display expired I-D by default with this rule. Moreover, the changed behaviour will be visible to the WG, i.e., then chairs / AD can update to their taste. |
This commit addresses issue #3078 (and #3171, which is duplicate).
The idea is that a new community filter rule type is introduced - this rule filters expired drafts for a particular group. Chairs can add this rule or remove it as well as they can add any other type of filtering rule.
The rule is pretty simple, but there are some post-processing in group/view.py. In particular:
Commit includes two migration files - ietf/community/migrations/0008_add_group_exp_rule.py was created automatically by makemigration and ietf/community/migrations/0009_add_group_exp_rule_to_groups.py was written manually. The latter updates database adding new rule to every WG/RG. This file can be dropped if we don't want this rule to silently appear in every WG/RG.
Commit also modifies tests to include testing this type of rule.