Forbid update of title and abstract when CFP is closed

[estyxx]

What

ToDo

[vercel]

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

Project	Deployment	Review	Updated (UTC)
pycon	Ready	Preview	Jan 13, 2026 11:53am

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.42%. Comparing base (67727a5) to head (25f1a32).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4525      +/-   ##
==========================================
- Coverage   94.42%   94.42%   -0.01%     
==========================================
  Files         352      352              
  Lines       10141    10157      +16     
  Branches      732      738       +6     
==========================================
+ Hits         9576     9591      +15     
  Misses        468      468              
- Partials       97       98       +1     

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

if the submission is accepted they should be able to make changes

[marcoacierno]

Suggested change

	print(LazyI18nString(self.abstract.to_dict()).data)
	print(submission.abstract.data)

[marcoacierno]

ernesto also wanted elevator_pitch

[marcoacierno]

@claude do a code-review

[claude]

Claude finished @marcoacierno's task in 59s —— View job

---

Code Review

• Read modified files
• Analyze implementation logic
• Review test coverage
• Provide comprehensive feedback

---

Summary

The implementation looks solid overall! The PR successfully implements the feature to restrict editing of title, abstract, and elevator_pitch fields after the CFP deadline, with an exception for accepted submissions. The code is well-tested with comprehensive test coverage.

Strengths

1. Good test coverage: Two dedicated tests cover both the accepted and non-accepted scenarios (lines 1310-1425 in test_edit_submission.py)
2. Clear business logic: The validation properly checks both CFP status and submission status
3. User-friendly error messages: Clear error messages explaining why the edit is blocked
4. Proper field comparison: Uses LazyI18nString for multilingual field comparison

Issues Found

1. Potential False Positives in Field Comparison (backend/api/submissions/mutations.py:281)

The current comparison logic may incorrectly flag changes when the field values are actually the same:

if LazyI18nString(input_value.to_dict()) != submission_value:

Issue: The LazyI18nString comparison might not work as expected if the dictionaries have different key orders or if there are subtle differences in how the data is serialized. This could incorrectly block legitimate "no-change" updates.

Recommendation: Consider a more robust comparison that normalizes the data structures before comparing, or extract the comparison into a helper method with explicit testing.

2. Missing Test Case: Unchanged Field Values

There's no test that verifies users CAN update other fields (like notes, tags, etc.) after the CFP deadline as long as they don't change title, abstract, or elevator_pitch.

Recommendation: Add a test case like:

def test_non_accepted_submission_can_edit_non_restricted_fields_after_cfp_deadline():
    # Verify users can update notes, tags, etc. after CFP closes
    # as long as title/abstract/elevator_pitch remain unchanged

3. Code Clarity: Magic Status Comparison (backend/api/submissions/mutations.py:270)

The comparison uses SubmissionModel.STATUS.accepted which is clear, but the logic could be slightly more explicit about what happens with other statuses:

if (
    not conference.is_cfp_open
    and submission.status != SubmissionModel.STATUS.accepted
):

Minor suggestion: Consider adding a comment explaining which statuses are affected (e.g., "proposed", "rejected", "waiting_list", etc.).

Questions for Discussion

1. Edge case: What about submissions in "maybe" or "waiting_list" status? Should they be allowed to edit after being promoted to "accepted"?

2. User experience: If a user submits the form without changing these fields, they'll still get validation errors. Should the comparison happen earlier to avoid confusing error messages?

Test Quality Assessment

The test coverage is excellent:

• ✅ Accepted submissions can edit restricted fields after deadline (lines 1310-1368)
• ✅ Non-accepted submissions cannot edit restricted fields after deadline (lines 1370-1425)
• ✅ Tests verify error messages are clear
• ✅ Tests verify data isn't changed when validation fails
• ⚠️ Missing: Test for editing non-restricted fields when CFP is closed

Minor Observations

1. Line 266-287: The validation logic is well-placed in the UpdateSubmissionInput.validate() method, maintaining good separation of concerns.

2. Line 281: The field comparison creates a new LazyI18nString instance each time. This is fine but consider caching if performance becomes an issue (unlikely given the use case).

3. Error message formatting (line 282-286): Uses replace("_", " ") which works but could be more explicit about field names in user-facing messages (e.g., "elevator_pitch" → "elevator pitch").

Verdict

The implementation is production-ready with excellent test coverage. The identified issues are minor and mostly about edge cases and code robustness. The core functionality works correctly and follows the business requirements from the review comments.

Approved ✅ (matching marcoacierno's approval)

---

[marcoacierno]

@claude can you write the missing test:

⚠️ Missing: Test for editing non-restricted fields when CFP is closed

[claude]

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

---

Writing Missing Test

• Read existing test file to understand patterns
• Read mutations file to identify non-restricted fields
• Write test for editing non-restricted fields when CFP is closed
• Commit and push changes

---

Summary

I've successfully added the missing test case test_non_accepted_submission_can_edit_non_restricted_fields_after_cfp_deadline to the test file.

What the test verifies:

• Non-accepted submissions (status: proposed) CAN edit non-restricted fields after the CFP deadline
• The test specifically updates tags, short_social_summary, and do_not_record while keeping restricted fields (title, abstract, elevator_pitch) unchanged
• The update succeeds and the non-restricted fields are properly modified
• The restricted fields remain unchanged

Test location: backend/api/submissions/tests/test_edit_submission.py:1427

The test follows the same pattern as the existing tests and provides comprehensive coverage for the edge case where users want to update metadata fields without touching the restricted content fields.

Note: I wasn't able to run the test suite locally due to permission restrictions, but the test follows the exact same pattern as the existing tests in the file and should pass in CI.

---

| Branch

[marcoacierno]

/deploy