From 04d219d3dfda1ecda1d55e16e8971d5587810cdc Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 28 Nov 2023 09:51:59 -0400 Subject: [PATCH 1/5] fix: Combine Group and GroupHistory when counting --- ietf/meeting/helpers.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/ietf/meeting/helpers.py b/ietf/meeting/helpers.py index e3f4874f4be..5376e088825 100644 --- a/ietf/meeting/helpers.py +++ b/ietf/meeting/helpers.py @@ -317,10 +317,19 @@ def _group_filter_headings(self): groups = set(self._get_group(s) for s in self.sessions if s and self._get_group(s)) - log.assertion('len(groups) == len(set(g.acronym for g in groups))') # no repeated acros + # Verify that we're not using the same acronym for more than one distinct groups. Count using + # id (for Group) or group_id (for GroupHistory) to avoid double-counting if a historical version + # of a Group got into the list. + log.assertion( + "len(set(g.group_id if isinstance(g, GroupHistory) else g.id for g in groups)) " + "== len(set(g.acronym for g in groups))" + ) group_parents = set(self._get_group_parent(g) for g in groups if self._get_group_parent(g)) - log.assertion('len(group_parents) == len(set(gp.acronym for gp in group_parents))') # no repeated acros + log.assertion( + "len(set(g.group_id if isinstance(g, GroupHistory) else g.id for g in group_parents)) " + "== len(set(gp.acronym for gp in group_parents))" + ) all_groups = groups.union(group_parents) all_groups.difference_update([g for g in all_groups if g.acronym in self.exclude_acronyms]) From abfcaf380a0538f820e6584dc395e3fa94963d58 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 28 Nov 2023 09:57:01 -0400 Subject: [PATCH 2/5] refactor: Use same symbol in parallel iterators --- ietf/meeting/helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ietf/meeting/helpers.py b/ietf/meeting/helpers.py index 5376e088825..396b84ff20e 100644 --- a/ietf/meeting/helpers.py +++ b/ietf/meeting/helpers.py @@ -327,7 +327,7 @@ def _group_filter_headings(self): group_parents = set(self._get_group_parent(g) for g in groups if self._get_group_parent(g)) log.assertion( - "len(set(g.group_id if isinstance(g, GroupHistory) else g.id for g in group_parents)) " + "len(set(gp.group_id if isinstance(gp, GroupHistory) else gp.id for gp in group_parents)) " "== len(set(gp.acronym for gp in group_parents))" ) From 16f1df0e70aa6e7f83586123ac93f157294423de Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 28 Nov 2023 10:36:12 -0400 Subject: [PATCH 3/5] fix: Handle group acronym changes in assertion --- ietf/meeting/helpers.py | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/ietf/meeting/helpers.py b/ietf/meeting/helpers.py index 396b84ff20e..1744ec326b3 100644 --- a/ietf/meeting/helpers.py +++ b/ietf/meeting/helpers.py @@ -317,17 +317,29 @@ def _group_filter_headings(self): groups = set(self._get_group(s) for s in self.sessions if s and self._get_group(s)) - # Verify that we're not using the same acronym for more than one distinct groups. Count using - # id (for Group) or group_id (for GroupHistory) to avoid double-counting if a historical version - # of a Group got into the list. + # Verify that we're not using the same acronym for more than one distinct group, accounting for + # the possibility that some groups are GroupHistory instances. The left-hand side counts distinct tuples + # (acronym, id), which will not equal the right-hand side if the same acronym is used for more than + # one group id. log.assertion( - "len(set(g.group_id if isinstance(g, GroupHistory) else g.id for g in groups)) " + "len(" + " set(" + " (g.acronym, g.group_id if isinstance(g, GroupHistory) else g.id)" + " for g in groups" + " )" + ") " "== len(set(g.acronym for g in groups))" ) group_parents = set(self._get_group_parent(g) for g in groups if self._get_group_parent(g)) + # See above for explanation of this assertion log.assertion( - "len(set(gp.group_id if isinstance(gp, GroupHistory) else gp.id for gp in group_parents)) " + "len(" + " set(" + " (gp.acronym, gp.group_id if isinstance(gp, GroupHistory) else gp.id)" + " for gp in groups" + " )" + ") " "== len(set(gp.acronym for gp in group_parents))" ) From 13b63db94d430da27f0d708da40c26eb5c00ef8a Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 28 Nov 2023 11:38:25 -0400 Subject: [PATCH 4/5] refactor: Avoid reference to GroupHistory in assertions --- ietf/meeting/helpers.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ietf/meeting/helpers.py b/ietf/meeting/helpers.py index 1744ec326b3..748beed8218 100644 --- a/ietf/meeting/helpers.py +++ b/ietf/meeting/helpers.py @@ -324,7 +324,7 @@ def _group_filter_headings(self): log.assertion( "len(" " set(" - " (g.acronym, g.group_id if isinstance(g, GroupHistory) else g.id)" + " (g.acronym, getattr(g, 'group_id', g.id))" " for g in groups" " )" ") " @@ -336,8 +336,8 @@ def _group_filter_headings(self): log.assertion( "len(" " set(" - " (gp.acronym, gp.group_id if isinstance(gp, GroupHistory) else gp.id)" - " for gp in groups" + " (gp.acronym, getattr(gp, 'group_id', gp.id))" + " for gp in group_parents" " )" ") " "== len(set(gp.acronym for gp in group_parents))" From ef0f363171914923534ccbe9a0195380063763a6 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 28 Nov 2023 12:07:46 -0400 Subject: [PATCH 5/5] chore: Revert so assertion fails on changed acronym The filtering and meeting displays disagree on how to handle this, so let's get a notification if it comes up in production. --- ietf/meeting/helpers.py | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/ietf/meeting/helpers.py b/ietf/meeting/helpers.py index 748beed8218..14478787fd7 100644 --- a/ietf/meeting/helpers.py +++ b/ietf/meeting/helpers.py @@ -318,28 +318,18 @@ def _group_filter_headings(self): if s and self._get_group(s)) # Verify that we're not using the same acronym for more than one distinct group, accounting for - # the possibility that some groups are GroupHistory instances. The left-hand side counts distinct tuples - # (acronym, id), which will not equal the right-hand side if the same acronym is used for more than - # one group id. + # the possibility that some groups are GroupHistory instances. This assertion will fail if a Group + # and GroupHistory for the same group have a different acronym - in that event, the filter will + # not match the meeting display, so we should be alerted that this has actually occurred. log.assertion( - "len(" - " set(" - " (g.acronym, getattr(g, 'group_id', g.id))" - " for g in groups" - " )" - ") " + "len(set(getattr(g, 'group_id', g.id) for g in groups)) " "== len(set(g.acronym for g in groups))" ) group_parents = set(self._get_group_parent(g) for g in groups if self._get_group_parent(g)) # See above for explanation of this assertion log.assertion( - "len(" - " set(" - " (gp.acronym, getattr(gp, 'group_id', gp.id))" - " for gp in group_parents" - " )" - ") " + "len(set(getattr(gp, 'group_id', gp.id) for gp in group_parents)) " "== len(set(gp.acronym for gp in group_parents))" )