Skip to content

feat: add granular permission and group system for conference management#60

Merged
JacobCoffee merged 12 commits intomainfrom
feat/granular-permissions
Mar 19, 2026
Merged

feat: add granular permission and group system for conference management#60
JacobCoffee merged 12 commits intomainfrom
feat/granular-permissions

Conversation

@JacobCoffee
Copy link
Copy Markdown
Owner

Summary

  • Add 22 custom permissions to the Conference model and 2 new permissions to TravelGrant for fine-grained access control
  • Replace the monolithic ManagePermissionMixin with ConferencePermissionMixin that checks per-view required_permission attributes, with fallback to superuser and legacy change_conference access
  • Add required_permission to all 70+ management views across 10 view modules (views.py, views_financial.py, views_reports.py, views_analytics.py, views_overrides.py, views_checkin.py, views_terminal.py, views_letters.py, views_vouchers.py, views_bulk_purchases.py)
  • Rewrite setup_groups management command with 10 groups: Conference Organizer, Program Committee, Registration Manager, Finance Team, Travel Grant Reviewer, Sponsor Manager, Check-in Staff, Activity Organizer, Reports Viewer, Read-Only Staff
  • Add permission-based sidebar visibility guards to the base template so users only see sections they can access
  • Update test fixtures to use the new permission-based access model

Test plan

  • All 1965 tests pass (1 pre-existing flaky test due to badge file cleanup race condition)
  • Lint and format checks pass
  • Type checks pass (only pre-existing Django ORM warnings)
  • Verify sidebar hides sections for users without permissions
  • Verify setup_groups creates all 10 groups with correct permission counts
  • Test that legacy superuser and change_conference holders still have full access

Generated with Claude Code

Copilot AI review requested due to automatic review settings March 19, 2026 18:36
@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a fine-grained, permission-based access control system for conference management by adding new custom permissions, updating all management views to declare required permissions, rebuilding staff groups around those permissions, and updating the management sidebar to hide inaccessible sections.

Changes:

  • Add custom permissions to Conference (22) and TravelGrant (2) and ship migrations for both.
  • Replace the legacy “monolithic” management permission mixins with ConferencePermissionMixin + per-view required_permission.
  • Rewrite setup_groups to create 10 role-based groups and update tests/templates to use the new model.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
tests/test_registration/test_bootstrap_registration.py Updates setup_groups tests for the new 10-group model.
tests/test_manage/test_report_views.py Updates report-view fixtures to grant permissions via the new group/perm model.
tests/test_manage/test_financial_views.py Updates finance fixtures to use new finance/report/registration permissions.
tests/test_manage/test_analytics_reports.py Updates analytics fixtures to use the new report permissions model.
src/django_program/programs/models.py Adds new custom TravelGrant permissions.
src/django_program/programs/migrations/0011_alter_travelgrant_options.py Migration to apply updated TravelGrant permissions.
src/django_program/manage/views.py Introduces ConferencePermissionMixin, sidebar permission context, and adds required_permission across many views.
src/django_program/manage/views_vouchers.py Adds required_permission for voucher bulk generation.
src/django_program/manage/views_terminal.py Adds required_permission for Terminal POS.
src/django_program/manage/views_reports.py Switches report views to permission-based mixin and adds per-view permissions.
src/django_program/manage/views_overrides.py Adds per-view override permissions.
src/django_program/manage/views_letters.py Adds per-view registration/letter permissions.
src/django_program/manage/views_financial.py Switches financial views to the new permission-based mixin and adds required permission.
src/django_program/manage/views_checkin.py Adds per-view check-in permissions.
src/django_program/manage/views_bulk_purchases.py Adds per-view bulk purchase permissions.
src/django_program/manage/views_analytics.py Switches analytics views to the new permission-based mixin and adds required permission.
src/django_program/manage/templates/django_program/manage/base.html Adds permission-gated sidebar visibility and collapsible sidebar UI behavior.
src/django_program/conference/models.py Adds new custom Conference permissions.
src/django_program/conference/migrations/0010_alter_conference_options.py Migration to apply updated Conference permissions.
src/django_program/conference/management/commands/setup_groups.py Replaces old groups with 10 new role groups and updates permission assignment logic.
Comments suppressed due to low confidence (3)

src/django_program/manage/views_reports.py:126

  • Report/analytics sidebar highlighting relies on active_nav, but report views no longer set it now that ReportPermissionMixin.get_context_data() was removed. As a result, the Reports/Analytics links won’t get the active class and the new “don’t collapse the active section” logic won’t work correctly. Consider setting context["active_nav"] = "reports" in the shared base for report views (or in ConferencePermissionMixin via a per-view attribute).
class ReportsDashboardView(ConferencePermissionMixin, TemplateView):
    """Landing page for all admin reports with summary statistics."""

    template_name = "django_program/manage/reports_dashboard.html"
    required_permission = "view_reports"

    def get_context_data(self, **kwargs: object) -> dict[str, object]:
        """Build context with summary stats for all report types.

        Args:
            **kwargs: Additional context data.

        Returns:
            Template context with attendee, inventory, voucher, and discount summaries.
        """
        context: dict[str, object] = super().get_context_data(**kwargs)
        conference = self.conference

src/django_program/manage/templates/django_program/manage/base.html:1272

  • The "Sync & Overrides" sidebar section is shown when user_perms.overrides or user_perms.settings, but the only link in the section is Overrides. Users with settings permission but without overrides will still see an Overrides link that will 403. Consider wrapping the Overrides link/subnav in user_perms.overrides (and similarly guard any sync links with user_perms.settings if/when added here).
      {% if user_perms.overrides or user_perms.settings %}
      <hr class="sidebar-utility-separator">
      <div class="sidebar-section" data-section="sync-overrides">
        <div class="sidebar-section-title">Sync & Overrides <svg class="sidebar-collapse-icon" viewBox="0 0 16 16" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round"><path d="M4 6l4 4 4-4"/></svg></div>
        <ul class="sidebar-nav">
          <li>
            <div class="sidebar-nav-expandable">
              <a href="{% url 'manage:override-list' conference.slug %}" class="{% if active_nav == 'overrides' %}active{% endif %}">
                <span class="sidebar-nav-icon"><svg width="16" height="16" fill="none" stroke="currentColor" stroke-width="1.5"><path d="M11 1.5L4.5 8l2 2L13 3.5"/><path d="M2.5 12.5h11M2.5 14.5h7"/></svg></span> Overrides
              </a>

src/django_program/conference/management/commands/setup_groups.py:170

  • setup_groups silently ignores any permission codenames that aren’t found (it just sets whatever matched returns). With many new custom permissions, a typo or missing migration could leave groups under-permissioned without any signal. Consider detecting and reporting/raising on missing permissions (e.g., compute missing = perm_set - found and print a warning or fail the command when missing is non-empty).
            perm_set = set(perm_specs)
            app_labels = {app for app, _ in perm_set}
            permissions = Permission.objects.filter(
                content_type__app_label__in=app_labels,
            )
            matched = [p for p in permissions if (p.content_type.app_label, p.codename) in perm_set]
            group.permissions.set(matched)

            if verbosity > 0:
                self.stdout.write(self.style.SUCCESS(f"  {verb} group '{group_name}' with {len(matched)} permissions"))

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

JacobCoffee and others added 2 commits March 19, 2026 14:11
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 19, 2026 19:16
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a granular permission + group-based access control system for conference management, replacing the previous “staff/change_conference” gating with per-view required_permission checks and permission-aware sidebar rendering.

Changes:

  • Added custom Conference permissions (dashboard/settings/program/registration/commerce/etc.) and expanded TravelGrant permissions.
  • Replaced the legacy ManagePermissionMixin/report/finance mixins with ConferencePermissionMixin and applied required_permission across management views.
  • Reworked setup_groups to create 10 role-based groups and updated tests/fixtures to use the new permission model.

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/test_registration/test_bootstrap_registration.py Updates group name expectations and read-only permission assertions for new groups/permissions.
tests/test_manage/test_views.py Adjusts the staff_user fixture to grant a specific management permission.
tests/test_manage/test_report_views.py Updates report fixture to use the new Reports Viewer group and permission-based access.
tests/test_manage/test_financial_views.py Updates finance fixture to the new Finance Team group and new conference permission codenames.
tests/test_manage/test_analytics_reports.py Updates analytics fixture to use Reports Viewer + view_reports permission.
src/django_program/registration/views_checkin.py Expands check-in API authorization to include new check-in/terminal permissions.
src/django_program/programs/models.py Adds new TravelGrant permissions (view_travel_grant, disburse_travel_grant).
src/django_program/programs/migrations/0011_alter_travelgrant_options.py Migration to apply new TravelGrant model permissions.
src/django_program/manage/views.py Introduces ConferencePermissionMixin, sidebar permission context, and applies required_permission to many views.
src/django_program/manage/views_vouchers.py Adds required_permission for voucher bulk generation.
src/django_program/manage/views_terminal.py Adds required_permission for terminal POS view.
src/django_program/manage/views_reports.py Switches reports to ConferencePermissionMixin and enforces view_reports/export_reports permissions.
src/django_program/manage/views_overrides.py Adds per-view override permissions (view_overrides/change_overrides).
src/django_program/manage/views_letters.py Adds per-view registration permissions; tightens download permission due to PII.
src/django_program/manage/views_financial.py Switches financial dashboard to ConferencePermissionMixin with view_finance permission.
src/django_program/manage/views_checkin.py Adds required_permission for check-in management views.
src/django_program/manage/views_bulk_purchases.py Adds required_permission for bulk purchase views and actions.
src/django_program/manage/views_analytics.py Switches analytics views to ConferencePermissionMixin gated by view_reports.
src/django_program/manage/templates/django_program/manage/base.html Adds permission-based sidebar visibility and collapsible sidebar sections.
src/django_program/conference/models.py Adds 22 custom Conference permissions in Meta.permissions.
src/django_program/conference/migrations/0010_alter_conference_options.py Migration to apply new Conference model permissions.
src/django_program/conference/management/commands/setup_groups.py Rewrites group setup into 10 roles and assigns new permission sets.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

JacobCoffee and others added 2 commits March 19, 2026 14:34
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ions

# Conflicts:
#	src/django_program/manage/templates/django_program/manage/base.html
Copilot AI review requested due to automatic review settings March 19, 2026 20:04
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 32 out of 32 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 1164 to +1194
@@ -1168,13 +1182,17 @@
<span class="sidebar-nav-icon"><svg width="16" height="16" fill="none" stroke="currentColor" stroke-width="1.5"><path d="M3 2.5h10v11H3z"/><path d="M5.5 5.5h5M5.5 7.5h5M5.5 9.5h3"/><path d="M10 1v3M6 1v3"/></svg></span> Visa Letters
</a>
</li>
{% endif %}
{% if user_perms.badges %}
<li>
<a href="{% url 'manage:badge-list' conference.slug %}" class="{% if active_nav == 'badges' %}active{% endif %}">
<span class="sidebar-nav-icon"><svg width="16" height="16" fill="none" stroke="currentColor" stroke-width="1.5"><rect x="2" y="3" width="12" height="10" rx="1.5"/><circle cx="7" cy="7" r="1.5"/><path d="M9.5 10V6.5h1"/><path d="M12 10V8.5"/></svg></span> Badges
</a>
</li>
</ul>
</div>
{% endif %}
Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

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

The Registration sidebar subsection template tags are mis-nested: the {% endif %} for user_perms.registration_people closes before the corresponding <ul>/<div> are closed, but those closing tags are still rendered unconditionally. If user_perms.registration_people is false (but user_perms.badges is true), this will emit stray </ul></div> and break the sidebar markup. Move the endif to wrap the entire People subsection block (including its closing tags), and ensure the Badges link is rendered inside a valid <ul> when shown.

Copilot uses AI. Check for mistakes.
@JacobCoffee JacobCoffee merged commit e4de09d into main Mar 19, 2026
15 of 17 checks passed
@JacobCoffee JacobCoffee deleted the feat/granular-permissions branch March 19, 2026 20:56
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