Add BE Support for asking to not record a talk

[marcoacierno]

What

ToDo

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Review	Updated (UTC)
pycon	Error		Dec 14, 2025 2:43pm

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.49%. Comparing base (cf37c8a) to head (c22d033).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4509   +/-   ##
=======================================
  Coverage   94.49%   94.49%           
=======================================
  Files         352      352           
  Lines       10136    10143    +7     
  Branches      735      735           
=======================================
+ Hits         9578     9585    +7     
  Misses        463      463           
  Partials       95       95           

🚀 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.

[marcoacierno]

/deploy

[marcoacierno]

@claude do a code-review

[claude]

Claude finished @marcoacierno's task in 1m 26s —— View job

---

Code Review

• Read and analyze changed files
• Review model changes
• Review GraphQL API changes
• Review test coverage
• Provide feedback

---

Summary

This PR adds backend support for speakers to opt out of having their talks/workshops recorded. The implementation is clean and well-tested. I found one potential issue regarding validation logic and a few minor observations.

Issues Found

🔴 Missing Validation: do_not_record vs SubmissionType.is_recordable

Location: backend/submissions/models.py:125 (in Submission.clean() method) and backend/api/submissions/mutations.py:118 (in BaseSubmissionInput.validate())

Issue: The code allows setting do_not_record=True even when the submission type is not recordable (i.e., SubmissionType.is_recordable=False). This creates a logical inconsistency.

Example scenario:

• A submission type "Lightning Talk" has is_recordable=False (these talks are never recorded)
• A speaker submits a lightning talk and checks do_not_record=True
• This is redundant and potentially confusing

Recommendation: Add validation to prevent do_not_record=True when type.is_recordable=False. You could either:

1. Add validation in Submission.clean():

if self.do_not_record and self.type_id and not self.type.is_recordable:
    raise exceptions.ValidationError(
        {
            "do_not_record": _("This submission type cannot be recorded")
        }
    )

2. Or add it to BaseSubmissionInput.validate() in mutations.py:

submission_type = conference.submission_types.filter(id=self.type).first()
if self.do_not_record and submission_type and not submission_type.is_recordable:
    errors.add_error("do_not_record", "This submission type cannot be recorded")

Alternatively, you could auto-disable the checkbox in the frontend when the submission type is not recordable.

Minor Observations

1. Migration Order

The migrations 0029_submission_do_not_record.py (created at 13:25) and 0030_submissiontype_is_recordable.py (created at 14:29) are in the correct dependency order. Good!

2. Test Coverage

Location: backend/api/submissions/tests/test_edit_submission.py:1265 and backend/api/submissions/tests/test_send_submission.py:1278

The tests cover the happy path well (setting do_not_record=True and verifying it persists). However, consider adding tests for:

• Edge case: Trying to set do_not_record=True on a submission type with is_recordable=False (once validation is added)
• Verifying default value (do_not_record=False) in more scenarios

3. Admin Interface

Location: backend/submissions/admin.py:252

The field do_not_record is added to the admin fieldset, which is good. Consider also adding it to list_display if organizers need to quickly see which submissions opted out of recording:

list_display = (
    "title",
    "speaker_display_name",
    "type",
    "status",
    "do_not_record",  # Add here
    "conference",
    ...
)

And possibly add a filter:

list_filter = ("conference", "status", "pending_status", "type", "tags", "do_not_record")

4. GraphQL Type Exposure

Location: backend/api/submissions/types.py:122

The do_not_record field is correctly marked as a private field (only visible to admins and the submitter). Good security practice!

The is_recordable field on SubmissionType is public (line 42), which is fine since it's conference configuration data that users need to see.

5. Code Style

Location: backend/api/submissions/tests/test_submissions.py:13-15

The unrelated change (removing hardcoded user IDs) is a good cleanup but should ideally be in a separate commit. Not a blocker though.

Positive Observations

✅ Good test coverage for the new functionality
✅ Proper use of private fields for sensitive data
✅ Migration dependencies are correct
✅ Default value (False) is sensible
✅ Field is integrated into both mutations (send and update)
✅ Help text is clear and descriptive

---

Recommendation

Before merging: Add validation to prevent logical inconsistencies between do_not_record and is_recordable. The other observations are nice-to-haves that can be addressed in follow-up PRs if needed.