Skip to content

fix: prevent slide upload by participants after session (#10582) - #11258

Merged
jennifer-richards merged 9 commits into
ietf-tools:mainfrom
jimfenton:slide_permission
Aug 13, 2026
Merged

fix: prevent slide upload by participants after session (#10582)#11258
jennifer-richards merged 9 commits into
ietf-tools:mainfrom
jimfenton:slide_permission

Conversation

@jimfenton

Copy link
Copy Markdown
Collaborator

Restricts slide submission by other than those that can manage slides after the session has ended.

Does not include tests for this, and breaks two existing tests:

======================================================================
FAIL: test_propose_session_slides (ietf.meeting.tests_views.MaterialsTests.test_propose_session_slides)

Traceback (most recent call last):
File "/workspace/ietf/meeting/tests_views.py", line 7077, in test_propose_session_slides
self.assertTrue(q('.proposeslides'))
AssertionError: [] is not true

======================================================================
FAIL: test_submit_and_approve_multiple_versions (ietf.meeting.tests_views.MaterialsTests.test_submit_and_approve_multiple_versions)

Traceback (most recent call last):
File "/home/dev/.local/lib/python3.12/site-packages/django/test/utils.py", line 461, in inner
return func(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.12/unittest/mock.py", line 1396, in patched
return func(*newargs, **newkeywargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/workspace/ietf/meeting/tests_views.py", line 7271, in test_submit_and_approve_multiple_versions
self.assertEqual(r.status_code, 302)
AssertionError: 403 != 302

Apparently this was because the sessions created for testing did not specify a time. I'm surprised this didn't fail before, because the submission would probably have violated the grace time after which Secretariat action is needed. I tried but didn't manage to succeed in fixing these test cases by adding a session time to the SessionFactory calls.

@jimfenton
jimfenton marked this pull request as draft July 21, 2026 15:07
@jimfenton

Copy link
Copy Markdown
Collaborator Author

I found another "Upload slides" button that isn't covered by this fix yet (materials/views.py line 1937). Will add.

@jimfenton
jimfenton marked this pull request as ready for review July 21, 2026 19:49

@jennifer-richards jennifer-richards left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style nit + a question

Comment thread ietf/meeting/views.py Outdated
Upload new slides
</a>
{% elif request.user.is_authenticated and not session.is_material_submission_cutoff %}
{% elif request.user.is_authenticated and not session.is_past %}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be ...and not session.is_past and not session.is_material_submission_cutoff? I.e., is this going to undesirably start allowing submissions between the material submission cutoff and the end of the session?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was my understanding that the material submission cutoff was after the meeting, to lock down the content for proceedings and such. This was partly based on what I saw in the test cases. So is_past would be a tighter restriction. If my understanding was incorrect, this would need to change.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you're likely right - I was thinking of draft submission deadline.

@jennifer-richards jennifer-richards left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed the failing tests, added missing test coverage, and adjusted to handle a corner case.

@codecov

codecov Bot commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.62%. Comparing base (b5b78cd) to head (971e97d).
⚠️ Report is 17 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11258      +/-   ##
==========================================
+ Coverage   88.58%   88.62%   +0.04%     
==========================================
  Files         333      333              
  Lines       44728    44815      +87     
==========================================
+ Hits        39624    39719      +95     
+ Misses       5104     5096       -8     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jennifer-richards
jennifer-richards merged commit bc04115 into ietf-tools:main Aug 13, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants