Skip to content

Add submission type filter to proposal review recap screen#4546

Merged
marcoacierno merged 2 commits intomainfrom
claude/issue-4537-20260130-1551
Jan 30, 2026
Merged

Add submission type filter to proposal review recap screen#4546
marcoacierno merged 2 commits intomainfrom
claude/issue-4537-20260130-1551

Conversation

@marcoacierno
Copy link
Member

Add the ability to filter proposals by submission type (e.g. Talk, Workshop) in the proposal review recap screen.

Closes #4537

Generated with Claude Code

Allow filtering proposals by type (e.g. talk, workshop) in the recap
screen using checkboxes, similar to the existing status filter.

Also refactors the filtering logic to apply all filters (reviews count,
status, and type) together rather than independently, preventing
filters from overriding each other.

Closes #4537

Co-authored-by: Marco Acierno <marcoacierno@users.noreply.github.com>
@vercel
Copy link

vercel bot commented Jan 30, 2026

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

Project Deployment Actions Updated (UTC)
pycon Ready Ready Preview Jan 30, 2026 3:59pm

@claude

This comment has been minimized.

@claude
Copy link
Contributor

claude bot commented Jan 30, 2026

Summary

This PR adds submission type filtering (e.g., Talk, Workshop) to the proposal review recap screen. The implementation refactors the existing filtering logic to consolidate all filters (reviews count, status, and the new submission type) into a unified applyFilters() function, improving code maintainability.

Review Feedback

Architecture & Design ✅

Strengths:

  • Good refactoring: Consolidating three separate event handlers into a single applyFilters() function reduces code duplication and makes the filtering logic more maintainable
  • Consistent pattern: The new submission type filter follows the same checkbox pattern as the existing status filter
  • Proper separation: Backend adapter provides the data, template handles rendering and client-side filtering

Testing & Coverage ⚠️

Issues:

  • Missing test coverage: The existing test test_proposals_review_get_recap_context (backend/reviews/tests/test_admin.py:809) needs to be updated to verify that submission_types is included in the context
  • Current test checks for audience_levels but doesn't check for the new submission_types field

Recommendation:
Add this assertion to the existing test:

assert "submission_types" in context

Error Handling ✅

No concerns. The filtering is client-side only and degrades gracefully if submission types are missing (checkboxes simply won't render).

Performance ✅

Strengths:

  • The refactored applyFilters() function is actually more efficient than the previous implementation, which had the reviews filter and status filter working independently
  • Minimal overhead: All filtering happens client-side on already-loaded data
  • submission_types uses the same pattern as audience_levels with .all() which is fine for this use case (small dataset)

Code Quality

Minor issues:

  • Whitespace-only changes (trailing space removal on lines 197, 206, etc.) are present but harmless
  • These whitespace fixes actually improve code quality

Suggestions

  1. Add test coverage (required before merge):

    # In backend/reviews/tests/test_admin.py:809
    def test_proposals_review_get_recap_context(rf):
        # ... existing test code ...
        assert "submission_types" in context
        # Optionally verify it contains the conference's submission types
  2. Consider whether submission types should default to all checked or all unchecked. Currently all are checked, which means users see all types by default (good for discoverability).

Verdict

Overall this is a clean implementation that improves upon the existing code. The refactoring to applyFilters() is a nice touch. Please add test coverage for the new submission_types context variable before merging.

@marcoacierno marcoacierno merged commit f015f9e into main Jan 30, 2026
8 checks passed
@marcoacierno marcoacierno deleted the claude/issue-4537-20260130-1551 branch January 30, 2026 15:59
@codecov
Copy link

codecov bot commented Jan 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.15%. Comparing base (aa24626) to head (980ab9b).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4546   +/-   ##
=======================================
  Coverage   94.15%   94.15%           
=======================================
  Files         353      353           
  Lines       10342    10342           
  Branches      761      761           
=======================================
  Hits         9738     9738           
  Misses        502      502           
  Partials      102      102           
🚀 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.

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.

Ability to filter by talk and proposal in the proposal review recap screen

1 participant