fix: apply materials only to a group's scheduled sessions - #11770
Merged
jennifer-richards merged 7 commits intoSep 16, 2026
Merged
jennifer-richards merged 7 commits into
jennifer-richards merged 7 commits into
Conversation
get_sessions() returns every session a group has at a meeting, in any state. Cancelled sessions and the tombstones left behind by rescheduling keep their timeslot assignment, so they are in that list too. Several views need only the sessions that are actually going to happen; notify_meetecho_of_all_slides already filtered for that inline. Give the filter a name so the other views can share it. No behavior change. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
upload_session_slides and approve_proposed_slides built their idea of "all of the group's sessions" from get_sessions(), which includes cancelled sessions and the tombstones left by rescheduling. With "apply to all" checked, which is the default, a deck uploaded to one session was also attached to those, and Meetecho was told about slides for sessions that will not happen. The "Session N" heading on both pages was an index into that same list, so it disagreed with the numbering on the session materials page whenever an unscheduled session sorted earlier. Use scheduled_only() for both the target set and the numbering. A session that is not itself scheduled forms a group of one: it gets no number, no "apply to all" checkbox, and changes to it stay with it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
session_details numbered each panel with forloop.counter, so the "Unscheduled Sessions" panel restarted at "Session 1", and neither panel's numbers matched the "Session N" heading on the upload pages, which indexed the full session list. A reschedule tombstone made this worse: it keeps a timeslot, so the view gave it a time and an empty status, and it appeared under "Unscheduled Sessions" looking like a live session with a full set of upload buttons. Number only scheduled sessions, from the same scheduled_only() list the upload pages now use, so a number means the same thing everywhere. Sessions that are not scheduled show their status name in place of a number, which is what the tombstone case needed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The sessa/sessb token in material document names reads like a session number, and it is easy to conclude it should follow the schedule or skip cancelled sessions. It must not: document names are fixed once created and sessions are reordered, cancelled and moved after that. State the reason at the point where the ordering is chosen. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Move get_sessions() and scheduled_only() to utils.py, next to get_meeting_sessions() and the sort key they are built from, so that utils code can use them without importing views. Add apply_to_all_sessions(), which returns the sessions an "apply to all" material change covers together with the acted-on session's position in that list, and use it in the two slide views. Every material upload view repeats this same computation, so it gets one home before the rest are converted. No behavior change. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The agenda, minutes, narrative minutes, bluesheets, drafts and recordings views had the same problem the slide views did: their "apply to all" set and their "Session N" heading came from the full session list, which includes cancelled sessions and reschedule tombstones. save_session_minutes_revision() went further and used get_meeting_sessions() directly, so an apply-to-all minutes upload deleted and replaced the minutes on every session of the group in any state. Convert all of them to apply_to_all_sessions(). Material already attached to an unscheduled session is now left alone by an apply-to-all upload elsewhere, and the session number on every material page counts the same sessions the materials page does. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #11770 +/- ##
==========================================
+ Coverage 88.81% 88.87% +0.06%
==========================================
Files 337 340 +3
Lines 45547 45694 +147
==========================================
+ Hits 40452 40611 +159
+ Misses 5095 5083 -12 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
rjsparks
commented
Sep 16, 2026
| def docname_token(self): | ||
| # Position in pk order, so the token never changes for the life of the session. It is an | ||
| # identifier, not a sequence number: sessions get reordered, cancelled and moved after | ||
| # documents carrying the token exist, so "sessb" says nothing about when the session meets. |
Member
Author
There was a problem hiding this comment.
I had this comment added so that future analysis of this code doesn't assume that sessa, sessb, etc. implies an ordering (without the comment, Claude made that assumption).
| return [s for s in sessions if s.current_status == "sched"] | ||
|
|
||
|
|
||
| def apply_to_all_sessions(session): |
Member
There was a problem hiding this comment.
nit: naming reads like a verb, implying it will apply
get_meeting_sessions() had a single caller, get_sessions(), whose only job was to annotate the status and sort into schedule order. Do that in get_meeting_sessions() itself. It now returns a list rather than a queryset; no caller chained filters on it. sessions_covered_by_apply_to_all() replaces apply_to_all_sessions(), which read like a command to apply something. Both from review of ietf-tools#11770. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
jennifer-richards
approved these changes
Sep 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a group has several sessions at a meeting and some are not in state
sched, uploading materials to one session could change another.Every material upload view built its idea of "all of the group's sessions"
from the full session list. Cancelled sessions and the tombstones left by
rescheduling keep their timeslot assignment, so they are in that list. With
"apply to all" checked, which is the default, a deck uploaded to one session
was attached to those too, and Meetecho was told about slides for sessions
that will not happen. Minutes were worse:
save_session_minutes_revision()deleted and replaced the minutes on every session of the group in any state.
The same list drove the "Session N" heading on the upload pages, while the
materials page numbered each panel separately with
forloop.counter. So agroup with sessions A (scheduled), B (cancelled) and C (scheduled) called C
"Session 2" on the materials page and "Session 3" on its upload page. A
reschedule tombstone also rendered under "Unscheduled Sessions" with a time,
no status, and a full set of upload buttons, indistinguishable from a live
session.
The fix is one definition of the covered set.
apply_to_all_sessions()returns the group's scheduled sessions in schedule order together with the
acted-on session's position in that list. A session that is not itself
scheduled is covered alone: it gets no number, no "apply to all" checkbox,
and changes to it stay with it. The materials page numbers from the same
list and shows a status name instead of a number for unscheduled sessions.
Out of scope, noted for later: revision uploads default "apply to all" to
checked, so revising a deck on one session adds it to every scheduled
session.
docname_token()is unchanged; its pk ordering is deliberate andnow says so in a comment.
The commits are ordered for review: two pure refactors that introduce the
helpers, three behavior changes each with tests that fail without them,
and the comment.
Fixes #10579
🤖 Generated with Claude Code