Fix interim edit clobbering Meetecho remote_instructions (#10689)#2
Open
Fix interim edit clobbering Meetecho remote_instructions (#10689)#2
Conversation
…10689) Editing an interim meeting backed by an auto-created Meetecho conference used to lose the Remote instructions URL whenever the subsequent Meetecho API call failed (and silently orphan the old conference when it succeeded). The WG chair was left with an interim that pointed nowhere and had to escalate to the secretariat to manually recover — losing uploaded slides in the process. Root cause: `InterimSessionModelForm.clean()` unconditionally blanked `remote_instructions` whenever the form was submitted with `remote_participation=='meetecho'`, on the assumption that the post-save helper would then fill in a fresh Meetecho URL. That assumption held for the create flow but falls apart on edit: if the helper's API call to Meetecho fails for any reason (network, auth, rate limit), the URL is already gone from the in-memory form data *and* has been persisted by `ModelForm.save()`, leaving nothing to fall back on. Fixes: - `forms.py`: stop blanking `remote_instructions` in `clean()`. The helper layer decides whether to overwrite the URL and only does so on successful Meetecho creation. Also initialize the `remote_participation` radio from the stored URL on edit so a no-op resubmit doesn't flip modes. - `helpers.py`: only (re)create a Meetecho conference when one of the scheduling fields (`date`, `time`, `requested_duration`, `end_time`) actually changed. Merely editing the agenda now leaves the existing conference alone. On a successful recreate, the old conference is fetched and deleted so it is not orphaned on the Meetecho side. - Tests in `tests_forms.py` and `tests_helpers.py` cover: - form-level: clean() preserves the URL across a Meetecho-mode edit; - helper-level: failure path (exception and empty result) preserves the URL; success path deletes the previous conference; - post-save filter: skip when only non-timing fields changed, recreate when any timing field changed, always create when no URL exists yet. Note: the "is this a Meetecho URL" check currently uses a substring match on "meetecho"; a dedicated flag on Session would be cleaner but requires a migration and is out of scope for this bugfix. Fixes ietf-tools#10689. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The best-effort delete of the replaced Meetecho conference catches any exception from fetch/delete and logs it, to ensure a successful recreate is never undone by a cleanup hiccup. Add a test that forces fetch() to raise and asserts the new URL still lands, delete_conference is not invoked, and nothing propagates out. Closes the remaining uncovered lines flagged by codecov on ietf-tools#10703. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
remote_instructionson edit.InterimSessionModelForm.clean()no longer wipes the stored Meetecho URL wheneverremote_participation=='meetecho'. The helper layer decides whether to overwrite, and only does so on successful Meetecho creation — so a failed API call now leaves the existing URL intact instead of destroying it.sessions_post_savenow filters to sessions whosedate/time/requested_duration/end_timechanged. Editing the agenda on a Meetecho-backed interim leaves the existing conference alone.Bug
Reported in ietf-tools#10689. WG chair edited an interim's agenda; the Meetecho API call in
sessions_post_savefailed; the session'sremote_instructionsURL ended up empty in the database. The secretariat had to delete and recreate the session to recover, losing uploaded slides in the process.Root cause:
clean()was blankingremote_instructionsin memory on every submit withremote_participation=='meetecho', thenModelForm.save()persisted the empty value, thencreate_interim_session_conferencesfailed before it could write a replacement. That blanking was fine for the create flow (no URL to lose) but fatal on edit.Test plan
test_edit_preserves_existing_meetecho_remote_instructionsintests_forms.py— exercisesclean()directly; fails onmainwithAssertionError: '' != 'https://meetings.conf.meetecho.com/...', passes with this change.tests_helpers.pytests:test_create_interim_session_conferences_preserves_url_on_failure— both exception and empty-result paths preserve the URL.test_create_interim_session_conferences_deletes_previous_conference— old conference is fetched and deleted on successful recreate.test_sessions_post_save_skips_meetecho_when_only_agenda_changed— helper gets an empty list.test_sessions_post_save_recreates_meetecho_when_timing_changed— all four timing fields trigger recreate.test_sessions_post_save_creates_meetecho_when_no_existing_url— always create when no existing Meetecho URL.ietf.meetingtest suite (384 tests) passes locally againstsettings_test.MEETECHO_API_CONFIG(127.0.0.1:9) confirmed the original buggy behavior onmainand confirmed the fix prevents it.Known limitations
The "is this a Meetecho URL" check uses a substring match on
"meetecho"in both the form__init__and the post-save filter. A dedicated flag onSessionwould be cleaner but needs a migration and is out of scope for this bugfix.There is a separate pre-existing oddity in
sessions_post_savewhere thecreate_interim_session_conferencescall is made inside thefor form in formsloop, so for formsets with N>1 forms the helper runs N times. Not introduced or worsened by this change; worth a follow-up.🤖 Generated with Claude Code