Skip to content

Conversation

@Saksham-Sirohi
Copy link
Contributor

@Saksham-Sirohi Saksham-Sirohi commented Nov 25, 2025

Fixes #921
Fixes #919

Summary by Sourcery

Unify organizer administration by moving all team and video permission management into the organizer settings page, deprecating legacy team-specific views/URLs, and tightening video access control across backend and frontend.

New Features:

  • Introduce a tabbed organizer management page that combines general organizer settings with team creation and inline management of members, invites, and API tokens.
  • Add granular video-related team permissions that map to specific Eventyay Video roles and traits for event worlds.

Bug Fixes:

  • Restrict Eventyay Video admin-level traits in JWTs to staff users only and base other video traits on team permissions.
  • Ensure direct messaging, room creation, kiosk management, and moderation actions in the video frontend are only available when the corresponding permissions are present.

Enhancements:

  • Refactor organizer update handling to process team CRUD, member, invite, and token operations in a scoped, transactional way with audit logging of changes.
  • Centralize and reuse team permission fieldsets between organizer and team forms for consistent UI.
  • Redirect all legacy team management routes in both control and orga backends to the unified organizer settings page and update navigation/links accordingly.
  • Refine video/world permission mappings, default roles, and trait grants so event-level permissions translate cleanly into video-world capabilities and API behavior.
  • Tighten visibility of chats, DMs, admin tools, and room types in the video UI based on the user’s effective permissions.

@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Nov 25, 2025

Reviewer's Guide

Unifies organizer administration by turning the organizer settings page into a tabbed management surface that handles organizer details, teams, membership/invites, API tokens, and per-team Eventyay Video permissions, while deprecating legacy team views and tightening video permission and trait handling across backend and SPA.

Sequence diagram for team management actions on organizer update view

sequenceDiagram
  actor AdminUser
  participant Browser
  participant OrganizerUpdateView
  participant TeamForm
  participant DB

  AdminUser->>Browser: Submit POST with team_action=create
  Browser->>OrganizerUpdateView: HTTP POST /organizer/<slug>/settings
  OrganizerUpdateView->>OrganizerUpdateView: dispatch()
  OrganizerUpdateView->>OrganizerUpdateView: can_manage_teams check
  alt User_can_manage_teams
    OrganizerUpdateView->>OrganizerUpdateView: _handle_team_create()
    OrganizerUpdateView->>TeamForm: instantiate TeamForm(organizer, data, prefix=team-create)
    OrganizerUpdateView->>TeamForm: is_valid()
    alt Form_valid
      OrganizerUpdateView->>DB: transaction.atomic begin
      OrganizerUpdateView->>DB: save Team instance with organizer
      OrganizerUpdateView->>DB: save_m2m limit_events and tracks
      OrganizerUpdateView->>DB: add current user to team.members
      OrganizerUpdateView->>DB: team.log_action(eventyay.team.created, changed_data)
      OrganizerUpdateView->>DB: commit transaction
      OrganizerUpdateView->>Browser: redirect to _teams_tab_url(team_id, section=teams)
      Browser-->>AdminUser: Show unified organizer page teams tab with success message
    else Form_invalid
      OrganizerUpdateView->>OrganizerUpdateView: _team_create_form_override = form
      OrganizerUpdateView->>OrganizerUpdateView: _render_with_team_errors(section=teams)
      OrganizerUpdateView->>Browser: render organizer template with errors
      Browser-->>AdminUser: Show validation errors in Create Team tab
    end
  else Missing_team_permissions
    OrganizerUpdateView->>OrganizerUpdateView: raise PermissionDenied
    OrganizerUpdateView->>Browser: 403 Forbidden
    Browser-->>AdminUser: Show permission error
  end

  AdminUser->>Browser: Submit POST with team_action=members
  Browser->>OrganizerUpdateView: HTTP POST /organizer/<slug>/settings
  OrganizerUpdateView->>OrganizerUpdateView: _handle_team_members()
  OrganizerUpdateView->>DB: load Team by team_id and organizer
  alt Remove_member
    OrganizerUpdateView->>DB: check other admin teams and member count
    alt Safe_to_remove
      OrganizerUpdateView->>DB: team.members.remove(user)
      OrganizerUpdateView->>DB: team.log_action(eventyay.team.member.removed)
      OrganizerUpdateView->>Browser: redirect to _teams_tab_url(team_id, section=permissions)
    else Last_team_admin
      OrganizerUpdateView->>Browser: redirect to _teams_tab_url(team_id, section=permissions, panel=members) with error
    end
  else Invite_new_member
    OrganizerUpdateView->>DB: lookup User by email
    alt User_not_found
      OrganizerUpdateView->>DB: create TeamInvite
      OrganizerUpdateView->>DB: send invite mail
      OrganizerUpdateView->>DB: team.log_action(eventyay.team.invite.created)
    else User_exists
      OrganizerUpdateView->>DB: team.members.add(user)
      OrganizerUpdateView->>DB: team.log_action(eventyay.team.member.added)
    end
    OrganizerUpdateView->>Browser: redirect to _teams_tab_url(team_id, section=permissions)
  end
Loading

Sequence diagram for video access token generation with fine-grained traits

sequenceDiagram
  actor EventUser
  participant Browser
  participant EventVideoAccessView as EventVideoAccessView
  participant EventModel as Event
  participant VideoPermModule as VideoPermissions
  participant VideoSystem

  EventUser->>Browser: Click "Open Video" button
  Browser->>EventVideoAccessView: HTTP GET /control/event/<slug>/video

  EventVideoAccessView->>EventVideoAccessView: _has_staff_video_access()
  alt Staff_or_superuser
    EventVideoAccessView->>EventVideoAccessView: has_staff_video_access = True
  else Regular_organizer_or_team_member
    EventVideoAccessView->>EventVideoAccessView: has_staff_video_access = False
  end

  EventVideoAccessView->>EventUser: user.get_event_permission_set(organizer, event)
  EventVideoAccessView->>VideoPermModule: collect_user_video_traits(event.slug, permission_set)
  VideoPermModule-->>EventVideoAccessView: video_traits list

  EventVideoAccessView->>EventVideoAccessView: _ensure_video_configuration()
  EventVideoAccessView->>EventModel: build_video_url()

  EventVideoAccessView->>EventVideoAccessView: _build_token_traits(has_staff_video_access, video_traits)
  Note over EventVideoAccessView: traits = ['attendee'] + video_traits
  alt has_staff_video_access
    EventVideoAccessView->>EventVideoAccessView: add 'admin' and event organizer trait
  end
  EventVideoAccessView-->>EventVideoAccessView: deduplicated traits list

  EventVideoAccessView->>EventVideoAccessView: generate_token_url(request, traits)
  EventVideoAccessView->>EventVideoAccessView: build JWT payload with uid, exp, traits
  EventVideoAccessView->>VideoSystem: Redirect with token URL
  VideoSystem-->>Browser: Load event world with permissions from traits
  Browser-->>EventUser: Show Eventyay Video with scoped capabilities
Loading

Sequence diagram for SPA room creation filtered by permissions

sequenceDiagram
  actor AdminUser
  participant SPA as VueSPA
  participant VuexStore as VuexStore
  participant Backend as EventyayBackend

  AdminUser->>SPA: Open admin rooms new view
  SPA->>VuexStore: mapGetters hasPermission
  SPA->>VuexStore: hasPermission('world:rooms.create.stage')
  SPA->>VuexStore: hasPermission('world:rooms.create.bbb')
  SPA->>VuexStore: hasPermission('world:rooms.create.chat')
  SPA->>VuexStore: hasPermission('world:rooms.create.exhibition')
  SPA->>VuexStore: hasPermission('world:rooms.create.poster')
  SPA-->>SPA: ROOM_TYPES computed = filtered by permissions
  SPA-->>AdminUser: Show only allowed room types in selector

  AdminUser->>SPA: Choose allowed room type and submit
  SPA->>Backend: API call room.create with selected type
  Backend->>Backend: Check event permissions mapped to world aliases
  alt User_has_required_event_permission
    Backend-->>SPA: Room created successfully
    SPA-->>AdminUser: Navigate to new room view
  else Missing_permission
    Backend-->>SPA: Error forbidden
    SPA-->>AdminUser: Show error or no action
  end
Loading

Updated class diagram for team and video permission modeling

classDiagram
  class Team {
    +BooleanField can_create_events
    +BooleanField can_manage_gift_cards
    +BooleanField can_change_teams
    +BooleanField can_change_organizer_settings
    +BooleanField all_events
    +ManyToMany limit_events
    +BooleanField can_change_event_settings
    +BooleanField can_change_items
    +BooleanField can_view_orders
    +BooleanField can_change_orders
    +BooleanField can_checkin_orders
    +BooleanField can_view_vouchers
    +BooleanField can_change_vouchers
    +BooleanField can_change_submissions
    +BooleanField is_reviewer
    +BooleanField force_hide_speaker_names
    +ManyToMany limit_tracks
    +BooleanField can_video_create_stages
    +BooleanField can_video_create_channels
    +BooleanField can_video_direct_message
    +BooleanField can_video_manage_announcements
    +BooleanField can_video_view_users
    +BooleanField can_video_manage_users
    +BooleanField can_video_manage_rooms
    +BooleanField can_video_manage_kiosks
    +BooleanField can_video_manage_configuration
    +ManyToMany members
    +ManyToMany invites
    +ManyToMany tokens
  }

  class OrganizerUpdateView {
    +Organizer object
    +Boolean can_edit_general_info
    +Boolean can_manage_teams
    +dict _team_form_overrides
    +TeamForm _team_create_form_override
    +str _forced_section
    +str _selected_team_override
    +str _selected_panel_override
    +post(request, args, kwargs)
    +get_context_data(kwargs)
    +_get_team_queryset()
    +_get_team_create_form()
    +_build_team_panel(team, selected_team_id, selected_panel)
    +_handle_team_create()
    +_handle_team_update()
    +_handle_team_members()
    +_handle_team_tokens()
    +_handle_team_delete()
    +_send_invite(instance)
    +_collect_team_change_data(team, form)
  }

  class TeamForm {
    +Meta model Team
    +clean()
  }

  class VideoPermissionDefinition {
    +str field
    +str trait_name
    +trait_value(event_slug) str
  }

  class VideoPermissionsModule {
    +list VIDEO_PERMISSION_DEFINITIONS
    +dict VIDEO_PERMISSION_BY_FIELD
    +list VIDEO_PERMISSION_TRAIT_NAMES
    +dict VIDEO_TRAIT_ROLE_MAP
    +iter_video_permission_definitions()
    +build_video_traits_for_event(event_slug) dict
    +collect_user_video_traits(event_slug, team_permission_set) list
  }

  class Event {
    +dict trait_grants
    +dict roles
    +_get_trait_grants_with_defaults() dict
    +has_permission_implicit(kwargs) bool
    +get_all_permissions(user) dict
  }

  class PermissionEnum {
    <<enumeration>>
    EVENT_KIOSKS_MANAGE
    EVENT_ROOMS_CREATE_STAGE
    EVENT_ROOMS_CREATE_BBB
    EVENT_ROOMS_CREATE_CHAT
    EVENT_ANNOUNCE
    EVENT_USERS_LIST
    EVENT_USERS_MANAGE
    EVENT_UPDATE
    EVENT_CHAT_DIRECT
  }

  TeamForm --> Team : model
  OrganizerUpdateView --> TeamForm : uses
  OrganizerUpdateView --> Team : manages

  VideoPermissionsModule --> VideoPermissionDefinition : contains
  Event --> VideoPermissionsModule : uses VIDEO_PERMISSION_BY_FIELD

  PermissionEnum ..> Event : role permissions

  Team --> VideoPermissionsModule : boolean fields map to trait definitions
Loading

File-Level Changes

Change Details Files
Extend OrganizerUpdate view to handle full team lifecycle (CRUD, members, tokens) and drive new tabbed organizer management UI.
  • Add permission-cached properties to distinguish general organizer edit and team management rights.
  • Route POSTed team_action values (create/update/members/tokens/delete) to dedicated handler methods with transaction boundaries and permission checks.
  • Introduce helpers to build team querysets, per-team panel state, form prefixes, redirects, and error rendering while keeping organizer form unbound when team operations fail.
  • Implement creation/update/delete of teams with logging of detailed changed-data snapshots, including ManyToMany fields, under scopes_disabled where needed.
  • Add inline management of team members, invites, and API tokens, including invite send/resend/revoke flows, token create/revoke, and safe handling of last team admin removal.
app/eventyay/eventyay_common/views/organizer.py
Replace simple organizer edit template with tabbed UI that embeds organizer general settings, inline team creation, and per-team permissions/members/API management using shared permission fieldset partial.
  • Introduce nav-tabs layout for General / Create Team / Manage Team Permissions, wiring active state via active_section context.
  • Render organizer form unchanged in General tab while wiring Save to base OrganizerUpdate view.
  • Add Teams tab with inline team_create_form using shared _permissions_fieldsets partial to create new teams without leaving organizer page.
  • Add Permissions tab that renders a panel per team with collapsible sub-panels for permission editing and member/API management, including delete actions and contextual counts.
  • Use new team_panels context structure (forms, members, invites, tokens, flags) and anchor element for message scrolling.
app/eventyay/eventyay_common/templates/eventyay_common/organizers/edit.html
app/eventyay/eventyay_common/templates/eventyay_common/organizers/teams/_permissions_fieldsets.html
app/eventyay/eventyay_common/templates/eventyay_common/base.html
Deprecate legacy team management views/URLs in both control and orga apps in favor of unified organizer page, while preserving backwards-compatible redirects.
  • Introduce UnifiedTeamManagementRedirectMixin that redirects legacy team list/detail/create/update/delete views to organizer.update with section/team query parameters and a flash message.
  • Apply redirect mixin to TeamListView/TeamMemberView/TeamCreateView/TeamUpdateView/TeamDeleteView in eventyay_common views.
  • Remove control-side team management URLs and templates and the OrganizerTeamView, updating navigation entries to point to organizer.update?section=permissions.
  • In the orga app, delete Team* views/templates and instead add organizer.redirect_team_management helper plus new /orga/organizer//teams* routes that 302 to unified management.
  • Update organizer index, event info boxes, and admin user team links to route to the new unified permissions section with optional team anchors.
app/eventyay/eventyay_common/views/team.py
app/eventyay/control/urls.py
src/pretix/control/urls.py
app/eventyay/control/navigation.py
app/eventyay/control/views/organizer_views/organizer_view.py
app/eventyay/orga/views/organizer.py
app/eventyay/orga/urls.py
app/eventyay/control/templates/pretixcontrol/event/fragment_info_box.html
app/eventyay/eventyay_common/templates/eventyay_common/organizers/index.html
app/eventyay/control/templates/pretixcontrol/admin/users/form.html
app/eventyay/orga/templates/orga/base.html
app/eventyay/control/templates/pretixcontrol/organizers/team_delete.html
app/eventyay/control/templates/pretixcontrol/organizers/team_edit.html
app/eventyay/control/templates/pretixcontrol/organizers/team_members.html
app/eventyay/control/templates/pretixcontrol/organizers/teams.html
app/eventyay/control/views/organizer_views/team_view.py
app/eventyay/orga/templates/orga/organizer/team/_form.html
app/eventyay/orga/templates/orga/organizer/team/create.html
app/eventyay/orga/templates/orga/organizer/team/list.html
app/eventyay/orga/templates/orga/organizer/team/update.html
Add fine-grained Eventyay Video permission fields on teams and propagate them through Django models, forms, API, and video trait mapping utilities.
  • Extend Team model and Organizer serializer/form with boolean can_video_* fields covering stage/channel creation, DM, announcements, user viewing/moderation, room/kiosk/config management.
  • Add matching permission groups to core Permission enum and world/event default_roles, plus new role names like video_stage_manager, video_channel_manager, etc.
  • Introduce eventyay_common.video.permissions module that maps team permission fields to video trait names/values and provides helpers to build per-event trait maps and compute user video traits.
  • Wire video traits into Event model permission resolution by augmenting trait_grants via _get_trait_grants_with_defaults and aligning default_grants/video trait roles.
  • Ensure direct messaging permission (EVENT_CHAT_DIRECT) is only granted if user has corresponding video trait; otherwise strip it from role permissions.
app/eventyay/base/models/organizer.py
app/eventyay/base/migrations/0003_team_can_video_create_channels_and_more.py
app/eventyay/control/forms/organizer_forms/team_form.py
app/eventyay/api/serializers/organizer.py
app/eventyay/core/permissions.py
app/eventyay/base/models/world.py
app/eventyay/base/models/event.py
app/eventyay/eventyay_common/video/permissions.py
Tighten video access token generation and align JWT traits, kiosk permissions, and SPA feature gating with new event-scoped permission model.
  • Refactor EventVideoSetupView.get to build token traits via _has_staff_video_access, collect_user_video_traits, and build_token_traits, giving everyone attendee + team-derived traits and reserving admin trait/organizer trait for staff-only users.
  • Change generate_token_url signature to accept traits and embed them directly instead of always adding admin+organizer traits.
  • Update World/Event default_roles and Permission enum to use EVENT* names and add EVENT_KIOSKS_MANAGE, plus alias event:update -> world:update in get_event_config_for_user.
  • Restrict kiosk.create and kiosk.fetch commands to EVENT_KIOSKS_MANAGE instead of generic update/users.manage, tightening who can manage kiosks.
  • Update event creation API and world creation task to accept structured traits payload and construct trait_grants using VIDEO_TRAIT_ROLE_MAP plus build_video_traits_for_event so video roles get consistent trait identifiers.
app/eventyay/eventyay_common/views/event.py
app/eventyay/base/models/world.py
app/eventyay/core/permissions.py
app/eventyay/features/live/modules/auth.py
app/eventyay/base/services/event.py
app/eventyay/api/views/event.py
app/eventyay/eventyay_common/tasks.py
Gate video UI capabilities in the Vue SPA (rooms, chats, DMs, admin screens) using new permission flags and ensure components behave when access is missing.
  • In admin room creation/edit components, filter available ROOM_TYPES based on hasPermission checks for world:rooms.create.* and room:update, and ensure chosenType uses filtered list.
  • In CreateChatPrompt and CreateStagePrompt, validate both available types and hasPermission before creating resources; adjust routing in CreateRoomPrompt after room creation.
  • In RoomsSidebar, only render chats/DM sections and prompts when corresponding permissions exist, use world:kiosks.manage/world:update for kiosks/config links, and hide DM channels if user lacks world:chat.direct.
  • In admin user list/detail views, wrap DM and moderation controls (ban/silence/delete/reactivate) in hasPermission checks for world:chat.direct and world:users.manage.
  • Ensure CreateDmPrompt enforces world:chat.direct permission before opening a DM channel.
app/eventyay/webapp/src/views/admin/rooms/new.vue
app/eventyay/webapp/src/views/admin/rooms/EditForm.vue
app/eventyay/webapp/src/components/CreateChatPrompt.vue
app/eventyay/webapp/src/components/RoomsSidebar.vue
app/eventyay/webapp/src/views/admin/user.vue
app/eventyay/webapp/src/views/admin/users.vue
app/eventyay/features/live/modules/auth.py
app/eventyay/webapp/src/views/admin/config/CreateRoomPrompt.vue
app/eventyay/webapp/src/components/CreateStagePrompt.vue
app/eventyay/webapp/src/components/CreateDmPrompt.vue

Assessment against linked issues

Issue Objective Addressed Explanation
#921 Consolidate team creation, listing, and permission management into a single unified organizer management page under /tickets/common/organizer/{organisername}, merging the former overview and edit pages.
#921 Deprecate/redirect the old /teams, /team/{id}, and /team/{id}/edit routes and update navigation to point to the unified organizer permissions interface with a consistent layout.
#921 Integrate talk and video permissions into the unified team management page and underlying permission system so that talk and video features are visible and functional from this interface.

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@Saksham-Sirohi Saksham-Sirohi force-pushed the unified-admin branch 3 times, most recently from de137e0 to 1043149 Compare November 29, 2025 20:25
@Saksham-Sirohi Saksham-Sirohi marked this pull request as ready for review November 29, 2025 20:25
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes and found some issues that need to be addressed.

  • In OrganizerUpdate._send_invite you pass self as the user in the email context, which is the view instance rather than a User; this likely should be self.request.user (or the invited user) to match the template’s expectations.
  • The OrganizerUpdate view has grown quite large and now handles both organizer updates and complex team/member/token flows; consider extracting the team-management logic into a separate helper/service or mixin to keep the view more maintainable and easier to reason about.
  • The change-tracking helpers _build_changed_data_dict and _collect_team_change_data are very similar and both walk m2m fields under scopes_disabled; you could consolidate them into a single reusable utility to avoid duplication and reduce the chance of subtle divergence over time.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In OrganizerUpdate._send_invite you pass `self` as the `user` in the email context, which is the view instance rather than a User; this likely should be `self.request.user` (or the invited user) to match the template’s expectations.
- The OrganizerUpdate view has grown quite large and now handles both organizer updates and complex team/member/token flows; consider extracting the team-management logic into a separate helper/service or mixin to keep the view more maintainable and easier to reason about.
- The change-tracking helpers `_build_changed_data_dict` and `_collect_team_change_data` are very similar and both walk m2m fields under `scopes_disabled`; you could consolidate them into a single reusable utility to avoid duplication and reduce the chance of subtle divergence over time.

## Individual Comments

### Comment 1
<location> `app/eventyay/eventyay_common/views/organizer.py:578-585` </location>
<code_context>
-        ctx['add_token_form'] = self.add_token_form
-        return ctx
-
-    def _send_invite(self, instance):
-        try:
-            mail(
-                instance.email,
-                _('eventyay account invitation'),
-                'pretixcontrol/email/invitation.txt',
-                {
-                    'user': self,
-                    'organizer': self.request.organizer.name,
-                    'team': instance.team.name,
</code_context>

<issue_to_address>
**issue (bug_risk):** Invitation mail context uses `self` instead of the actual requesting user.

In `_send_invite`, `'user': self` passes the `OrganizerUpdate` view instance to the template instead of the authenticated user. Invitation templates typically expect a `User` object and may break or render incorrectly. Use `self.request.user` (or the appropriate `User` instance) here instead.
</issue_to_address>

### Comment 2
<location> `app/eventyay/webapp/src/components/CreateChatPrompt.vue:71-80` </location>
<code_context>
+		}
 	},
 	methods: {
 		async create() {
+			// Check if any types are available
+			if (this.types.length === 0) {
+				return
+			}
+
+			// Verify permission for selected type
+			if (this.type === 'text' && !this.hasPermission('world:rooms.create.chat')) {
+				return
+			}
+			if (this.type === 'video' && !this.hasPermission('world:rooms.create.bbb')) {
+				return
+			}
</code_context>

<issue_to_address>
**issue (bug_risk):** `hasPermission` is used but never mapped/injected in this component.

`create()` now calls `this.hasPermission`, but this component never defines it (no `mapGetters` import or mapping). At runtime `this.hasPermission` will be `undefined` and calling it will throw. Please wire it up as in other components (e.g. `RoomsSidebar`, `CreateStagePrompt`) by importing `mapGetters` from `vuex` and adding `...mapGetters(['hasPermission'])` to `computed`.
</issue_to_address>

### Comment 3
<location> `app/eventyay/webapp/src/components/CreateDmPrompt.vue:26-28` </location>
<code_context>
 	},
 	methods: {
 		async create(users) {
+			// Check permission before creating direct message
+			if (!this.hasPermission('world:chat.direct')) {
+				this.$emit('close')
+				return
</code_context>

<issue_to_address>
**issue (bug_risk):** `CreateDmPrompt` now relies on `hasPermission` but does not define it.

`create(users)` calls `this.hasPermission('world:chat.direct')`, but this component never defines `hasPermission` (e.g., via Vuex getters or props), so opening a DM will throw at runtime. Please either import `mapGetters` and add `...mapGetters(['hasPermission'])` under `computed`, or pass the permission value in as a prop and use that instead.
</issue_to_address>

### Comment 4
<location> `app/eventyay/base/models/event.py:1540-1543` </location>
<code_context>
-        # Removed user.world_grants loop (attribute not present on unified User model)
+                role_perms = event_roles.get(role, SYSTEM_ROLES.get(role, []))
+                
+                direct_messaging_def = VIDEO_PERMISSION_BY_FIELD.get('can_video_direct_message')
+                if direct_messaging_def and role != 'video_direct_messaging':
+                    direct_messaging_trait = direct_messaging_def.trait_value(self.slug)
+                    has_direct_messaging_trait = direct_messaging_trait in user.traits
+                    
+                    if not has_direct_messaging_trait:
</code_context>

<issue_to_address>
**issue (bug_risk):** Potential `TypeError` when checking `direct_messaging_trait in user.traits` if `user.traits` is `None`.

In `get_all_permissions`, this containment check will raise a `TypeError` when `user.traits` is `None` (which is valid for many users). Please normalize to an iterable first, e.g.:

```python
traits = user.traits or []
has_direct_messaging_trait = direct_messaging_trait in traits
```
</issue_to_address>

### Comment 5
<location> `app/eventyay/eventyay_common/views/organizer.py:312-321` </location>
<code_context>
+            )
+        self._forced_section = 'permissions'
+
+        if form.is_valid():
+            with transaction.atomic():
+                with scopes_disabled():
+                    form.instance.organizer = self.request.organizer
+                    form.save()
+            if form.has_changed():
+                team.log_action(
+                    'eventyay.team.changed',
+                    user=self.request.user,
+                    data=self._collect_team_change_data(team, form),
+                )
+            team_name = team.name
+            messages.success(
+                self.request,
+                _("Changes to the team '%(team_name)s' have been saved.") % {'team_name': team_name},
+            )
+            return redirect(self._teams_tab_url(section='permissions'))
+
+        messages.error(
</code_context>

<issue_to_address>
**suggestion:** Successful team update redirect drops the current `team` context.

After `_handle_team_update`, the redirect calls `self._teams_tab_url(section='permissions')` without the team id, so the user is returned to the permissions tab but not to the team they just edited. Consider including `team=team.pk` (and any relevant `panel` param) in the redirect so the same team panel stays open after saving.

Suggested implementation:

```python
            messages.success(
                self.request,
                _("Changes to the team '%(team_name)s' have been saved.") % {'team_name': team_name},
            )
            # Preserve the current team context (and permissions section) after saving
            return redirect(self._teams_tab_url(team=team.pk, section='permissions'))

```

If `_teams_tab_url` has a different signature (for example if it expects the team id as a positional argument, or also accepts a `panel` parameter), you should adjust the call accordingly, e.g.:
- Positional team id: `self._teams_tab_url(team.pk, section='permissions')`
- With panel: `self._teams_tab_url(team=team.pk, section='permissions', panel='team')`

Make sure this call matches the existing usage pattern of `_teams_tab_url` elsewhere in the file.
</issue_to_address>

### Comment 6
<location> `app/eventyay/eventyay_common/video/permissions.py:66-67` </location>
<code_context>
def collect_user_video_traits(event_slug: str, team_permission_set: Iterable[str]) -> List[str]:
    """
    Given an event slug and the permission set for the current user, return the list of
    video trait values that should be embedded into the JWT token.
    """
    traits = []
    for perm_name in team_permission_set:
        definition = VIDEO_PERMISSION_BY_FIELD.get(perm_name)
        if definition:
            traits.append(definition.trait_value(event_slug))
    return traits

</code_context>

<issue_to_address>
**suggestion (code-quality):** Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))

```suggestion
        if definition := VIDEO_PERMISSION_BY_FIELD.get(perm_name):
```
</issue_to_address>

### Comment 7
<location> `app/eventyay/eventyay_common/views/organizer.py:131` </location>
<code_context>
    def post(self, request, *args, **kwargs):
        self.object = self.get_object()
        action = request.POST.get('team_action')
        if action:
            if not self.can_manage_teams:
                raise PermissionDenied()
            handlers = {
                'create': self._handle_team_create,
                'update': self._handle_team_update,
                'members': self._handle_team_members,
                'tokens': self._handle_team_tokens,
                'delete': self._handle_team_delete,
            }
            handler = handlers.get(action)
            if handler:
                return handler()
        return super().post(request, *args, **kwargs)

</code_context>

<issue_to_address>
**issue (code-quality):** Use named expression to simplify assignment and conditional [×2] ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
</issue_to_address>

### Comment 8
<location> `app/eventyay/eventyay_common/views/organizer.py:298-302` </location>
<code_context>
    def _get_team_from_post(self):
        team = get_object_or_404(
            Team,
            organizer=self.request.organizer,
            pk=self.request.POST.get('team_id'),
        )
        return team

</code_context>

<issue_to_address>
**issue (code-quality):** Inline variable that is immediately returned ([`inline-immediately-returned-variable`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/inline-immediately-returned-variable/))
</issue_to_address>

### Comment 9
<location> `app/eventyay/eventyay_common/views/organizer.py:375` </location>
<code_context>
    def _handle_team_members(self):
        team = self._get_team_from_post()
        self._forced_section = 'permissions'
        invite_form_prefix = self._invite_form_prefix(team)
        prefixed_user_field = f'{invite_form_prefix}-user'
        invite_form = InviteForm(
            data=(self.request.POST if prefixed_user_field in self.request.POST else None),
            prefix=invite_form_prefix,
        )

        request = self.request
        post = request.POST

        with transaction.atomic():
            if 'remove-member' in post:
                try:
                    user = User.objects.get(pk=post.get('remove-member'))
                except (User.DoesNotExist, ValueError):
                    pass
                else:
                    other_admin_teams = (
                        request.organizer.teams.exclude(pk=team.pk)
                        .filter(can_change_teams=True, members__isnull=False)
                        .exists()
                    )
                    if not other_admin_teams and team.can_change_teams and team.members.count() == 1:
                        messages.error(
                            request,
                            _(
                                'You cannot remove the last member from this team as no one would '
                                'be left with the permission to change teams.'
                            ),
                        )
                        self._set_panel_override(team.pk, 'members')
                        return redirect(self._teams_tab_url(team.pk, section='permissions', panel='members'))
                    team.members.remove(user)
                    team.log_action(
                        'eventyay.team.member.removed',
                        user=request.user,
                        data={'email': user.email, 'user': user.pk},
                    )
                    messages.success(request, _('The member has been removed from the team.'))
                    return redirect(self._teams_tab_url(team.pk, section='permissions'))

            elif 'remove-invite' in post:
                try:
                    invite = team.invites.get(pk=post.get('remove-invite'))
                except (TeamInvite.DoesNotExist, ValueError):
                    messages.error(request, _('Invalid invite selected.'))
                    self._set_panel_override(team.pk, 'members')
                    return redirect(self._teams_tab_url(team.pk, section='permissions', panel='members'))
                else:
                    invite.delete()
                    team.log_action(
                        'eventyay.team.invite.deleted',
                        user=request.user,
                        data={'email': invite.email},
                    )
                    messages.success(request, _('The invite has been revoked.'))
                return redirect(self._teams_tab_url(team.pk, section='permissions'))

            elif 'resend-invite' in post:
                try:
                    invite = team.invites.get(pk=post.get('resend-invite'))
                except (TeamInvite.DoesNotExist, ValueError):
                    messages.error(request, _('Invalid invite selected.'))
                    self._set_panel_override(team.pk, 'members')
                    return redirect(self._teams_tab_url(team.pk, section='permissions', panel='members'))
                else:
                    self._send_invite(invite)
                    team.log_action(
                        'eventyay.team.invite.resent',
                        user=request.user,
                        data={'email': invite.email},
                    )
                    messages.success(request, _('The invite has been resent.'))
                return redirect(self._teams_tab_url(team.pk, section='permissions'))

            elif f'{invite_form_prefix}-user' in post and invite_form.is_valid() and invite_form.has_changed():
                try:
                    user = User.objects.get(email__iexact=invite_form.cleaned_data['user'])
                except User.DoesNotExist:
                    if team.invites.filter(email__iexact=invite_form.cleaned_data['user']).exists():
                        messages.error(
                            request,
                            _('This user already has been invited for this team.'),
                        )
                        self._set_panel_override(team.pk, 'members')
                        self._set_team_override(team.pk, invite_form=invite_form)
                        return self._render_with_team_errors('permissions')
                    if 'native' not in get_auth_backends():
                        messages.error(
                            request,
                            _('Users need to have a eventyay account before they can be invited.'),
                        )
                        self._set_panel_override(team.pk, 'members')
                        self._set_team_override(team.pk, invite_form=invite_form)
                        return self._render_with_team_errors('permissions')

                    invite = team.invites.create(email=invite_form.cleaned_data['user'])
                    self._send_invite(invite)
                    team.log_action(
                        'eventyay.team.invite.created',
                        user=request.user,
                        data={'email': invite_form.cleaned_data['user']},
                    )
                    messages.success(request, _('The new member has been invited to the team.'))
                    return redirect(self._teams_tab_url(team.pk, section='permissions'))
                else:
                    if team.members.filter(pk=user.pk).exists():
                        messages.error(
                            request,
                            _('This user already has permissions for this team.'),
                        )
                        self._set_panel_override(team.pk, 'members')
                        self._set_team_override(team.pk, invite_form=invite_form)
                        return self._render_with_team_errors('permissions')

                    team.members.add(user)
                    team.log_action(
                        'eventyay.team.member.added',
                        user=request.user,
                        data={'email': user.email, 'user': user.pk},
                    )
                    messages.success(request, _('The new member has been added to the team.'))
                    return redirect(self._teams_tab_url(team.pk, section='permissions'))

        messages.error(request, _('Your changes could not be saved.'))
        self._set_panel_override(team.pk, 'members')
        self._set_team_override(team.pk, invite_form=invite_form)
        return self._render_with_team_errors('permissions')

</code_context>

<issue_to_address>
**issue (code-quality):** We've found these issues:

- Extract duplicate code into method [×2] ([`extract-duplicate-method`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/extract-duplicate-method/))
- Low code quality found in OrganizerUpdate.\_handle\_team\_members - 17% ([`low-code-quality`](https://docs.sourcery.ai/Reference/Default-Rules/comments/low-code-quality/))

<br/><details><summary>Explanation</summary>

The quality score for this function is below the quality threshold of 25%.
This score is a combination of the method length, cognitive complexity and working memory.

How can you solve this?

It might be worth refactoring this function to make it shorter and more readable.

- Reduce the function length by extracting pieces of functionality out into
  their own functions. This is the most important thing you can do - ideally a
  function should be less than 10 lines.
- Reduce nesting, perhaps by introducing guard clauses to return early.
- Ensure that variables are tightly scoped, so that code using related concepts
  sits together within the function rather than being scattered.</details>
</issue_to_address>

### Comment 10
<location> `app/eventyay/eventyay_common/views/team.py:29` </location>
<code_context>
    def dispatch(self, request, *args, **kwargs):
        target = reverse(
            'eventyay_common:organizer.update',
            kwargs={'organizer': request.organizer.slug},
        )
        team_id = kwargs.get('team')
        query = '?section=teams'
        if team_id:
            query = f'{query}&team={team_id}'
        messages.info(
            request,
            _('Team management has moved into the unified organizer page.'),
        )
        return redirect(f'{target}{query}')

</code_context>

<issue_to_address>
**issue (code-quality):** We've found these issues:

- Move assignment closer to its usage within a block ([`move-assign-in-block`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/move-assign-in-block/))
- Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Copy link
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 unifies team and video permission management by consolidating all organizer administration into a single tabbed interface and introducing granular video-specific permissions that map to Eventyay Video roles and traits.

Key Changes:

  • Replaces separate team management views with a unified organizer settings page featuring inline team CRUD, member/invite/token management, and permission configuration
  • Adds nine granular video permissions (stages, channels, DMs, announcements, users, rooms, kiosks, config) to the Team model with corresponding JWT trait mappings
  • Restricts video admin-level access to staff users only; regular organizers receive specific video traits based on team permissions

Reviewed changes

Copilot reviewed 48 out of 48 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
src/pretix/control/urls.py Removes legacy team management URL patterns
app/eventyay/orga/views/organizer.py Removes legacy team view classes, adds redirect helper to unified page
app/eventyay/orga/urls.py Replaces team URLs with redirects to unified organizer settings
app/eventyay/orga/templates/orga/organizer/team/*.html Deletes deprecated team templates (list, create, update, _form)
app/eventyay/orga/templates/orga/base.html Updates Teams navigation link to point to unified organizer page
app/eventyay/webapp/src/views/admin/users.vue Adds permission checks for direct messaging and user moderation actions
app/eventyay/webapp/src/views/admin/user.vue Adds permission guards for DM, call, and user management buttons
app/eventyay/webapp/src/views/admin/rooms/new.vue Filters available room types based on user's video permissions
app/eventyay/webapp/src/views/admin/rooms/EditForm.vue Applies same room type filtering as new room view
app/eventyay/webapp/src/views/admin/config/CreateRoomPrompt.vue Fixes routing and removes console.log
app/eventyay/webapp/src/components/RoomsSidebar.vue Adds permission-based visibility for chats, DMs, kiosks, and config links
app/eventyay/webapp/src/components/CreateStagePrompt.vue Adds permission check before stage creation, removes console.log
app/eventyay/webapp/src/components/CreateDmPrompt.vue Guards DM creation with permission check
app/eventyay/webapp/src/components/CreateChatPrompt.vue Filters channel types by permissions, validates before creation
app/eventyay/eventyay_common/views/team.py Adds redirect mixin to forward legacy team URLs to unified page
app/eventyay/eventyay_common/views/organizer.py Implements unified organizer update view with tabbed team management
app/eventyay/eventyay_common/views/event.py Refactors video token generation to use permission-based traits instead of admin-only
app/eventyay/eventyay_common/video/permissions.py Defines video permission to trait mappings and helper functions
app/eventyay/eventyay_common/templates/eventyay_common/organizers/teams/*.html Replaces team templates with unified edit page and reusable permission fieldsets
app/eventyay/eventyay_common/templates/eventyay_common/base.html Adds organizer-messages anchor for scroll targets
app/eventyay/eventyay_common/tasks.py Includes video traits in world creation payload
app/eventyay/features/live/modules/auth.py Updates kiosk permissions to use new EVENT_KIOSKS_MANAGE
app/eventyay/core/permissions.py Adds EVENT_KIOSKS_MANAGE permission and video role mappings to SYSTEM_ROLES
app/eventyay/control/views/organizer_views/team_view.py Deletes entire legacy team view module
app/eventyay/control/views/organizer_views/organizer_view.py Removes OrganizerTeamView class
app/eventyay/control/views/organizer_views/init.py Removes team view imports
app/eventyay/control/urls.py Removes team management URL patterns
app/eventyay/control/templates/pretixcontrol/organizers/*.html Deletes legacy team management templates
app/eventyay/control/templates/pretixcontrol/event/fragment_info_box.html Updates team links to unified organizer page
app/eventyay/control/templates/pretixcontrol/admin/users/form.html Updates team links to point to unified page
app/eventyay/control/navigation.py Changes Teams navigation to use unified organizer page
app/eventyay/control/forms/organizer_forms/team_form.py Adds video permission fields to TeamForm
app/eventyay/base/services/event.py Maps event.update permission to world:update for video config access
app/eventyay/base/models/world.py Removes EVENT_CHAT_DIRECT from default attendee role, adds video permission roles
app/eventyay/base/models/organizer.py Adds nine can_video_* boolean fields to Team model
app/eventyay/base/models/event.py Removes EVENT_CHAT_DIRECT from attendee, filters DM permission by trait, augments trait_grants with video roles
app/eventyay/base/migrations/0003_team_can_video_create_channels_and_more.py Migration adding video permission fields to Team
app/eventyay/api/views/event.py Accepts video traits in event creation/update payload
app/eventyay/api/serializers/organizer.py Exposes video permission fields in Team serializer

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

@Saksham-Sirohi Saksham-Sirohi force-pushed the unified-admin branch 2 times, most recently from 9772026 to 28e5b72 Compare November 30, 2025 10:04
Copy link
Member

@mariobehling mariobehling left a comment

Choose a reason for hiding this comment

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

@Saksham-Sirohi These are a lot of file changes. The two issues require different changes. So, is it possible to split this PR into two, please? Issue 1 only requires changes to the UI. Issue 2 actually involves some feature changes. So, best would be to keep the PRs separated in the same way the issues are separated. This makes the review clearer as well.

Copy link
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 51 out of 51 changed files in this pull request and generated 13 comments.


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

@hongquan
Copy link
Member

hongquan commented Dec 11, 2025

Instead of multiple if check like this:

traits_payload = request.data.get("traits") or {}
if not isinstance(traits_payload, dict):
    raise ValidationError("Traits must be provided as an object.")

attendee_trait_grants = traits_payload.get("attendee", "")
if attendee_trait_grants and not isinstance(attendee_trait_grants, str):
    raise ValidationError("Attendee traits must be a string")

You should define the expected data shape with Pydantic:

@dataclass
class TraitsPayload:
    attendee: str = 'attendee'

class EventCreationRequestData(BaseModel):
    traits: TraitsPayload

then use like this:

from pydantic import ValidationError as PyValidationError
from pyngo import drf_error_details

try:
    req = EventCreationRequestData.model_validate(request.data)
except PyValidationError as e:
    # Convert Pydantic error to DRF error.
    raise drf_error_details(e)
    
req.traits.attendee

The benefit of this method is that, you can look into the EventCreationRequestData class to know how the data should look like (which fields, data type of each field).

@norbusan
Copy link
Member

Comments from claude

Summary & Recommendations

● Overall Assessment: ⚠ NEEDS REVISION BEFORE MERGE

Verdict: The PR implements valuable features (unified admin, granular video permissions) but has blocking issues that must be addressed.


BLOCKING ISSUES 🚨

Must fix before merge:

  1. Bug in _send_invite: Passes view instance instead of self.request.user to email context
  2. Missing Vue imports: mapGetters imports missing in permission-checking components
  3. TypeError risk: user.traits null safety checks missing
  4. Test coverage: Zero tests for 9 new permission fields and trait mapping
  5. Redirect inconsistency: Team redirect uses section=teams instead of section=permissions

HIGH PRIORITY 🔴

Should address before merge:

  1. View refactoring: Extract team management logic from OrganizerUpdate into mixins/separate views
  2. Permission enforcement: Add backend validation at all video operation endpoints
  3. Audit logging: Verify permission changes are logged
  4. Documentation: Add video permission fields to API docs and developer guide
  5. Migration rollback test: Verify safe rollback to previous schema

MEDIUM PRIORITY 🟡

Address soon after merge:

  1. Performance: Add caching for trait computation if used in hot paths
  2. API versioning: Consider bumping API version or documenting breaking changes
  3. Integration tests: Add end-to-end tests for permission workflows
  4. Frontend tests: Add Vue component tests for permission gating
  5. User docs: Document new permission options for event organizers

Recommended Action Plan

Before Re-Review:

  1. Fix blocking bugs:

    • Fix _send_invite user context bug
    • Add missing Vue imports for mapGetters
    • Add null safety for user.traits checks
    • Fix section=teams → section=permissions
  2. Add minimum test coverage:

    • test_video_permissions.py (trait mapping)
    • test_organizer_view.py (team CRUD)
    • test_team_api.py (API serialization)
  3. Security hardening:

    • Add permission checks at video API endpoints
    • Verify audit logging completeness
    • Test migration rollback
  4. Documentation:

    • Update API changelog
    • Add permission field descriptions to docs

After merge (follow-up PR):

  • Refactor OrganizerUpdate into smaller components
  • Add comprehensive integration tests
  • Performance profiling of trait computation

Final Notes

This PR tackles an important consolidation effort and the video permission system is well-designed. However, the implementation has several bugs and gaps that could cause production issues. The Sourcery AI and Copilot feedback aligns with this review - the code needs refinement before merge.

@norbusan
Copy link
Member

Details from Claude:

● 1. Architecture & Design ✅

Strengths:

  • Good consolidation approach: Unifying team and organizer management reduces navigation complexity
  • Permission granularity: Nine distinct video permissions provide fine-grained access control
  • Trait-based system: Mapping team permissions to JWT traits is a solid pattern for video integration
  • Backward compatibility: Redirect mixin preserves existing workflows

Concerns:

  • View bloat: The OrganizerUpdate view is handling too many responsibilities (organizer edit, team CRUD, member management, invites, tokens). This violates Single Responsibility Principle.
  • Missing separation: Consider extracting team operations into a separate TeamManagementView or using Django's ViewSet pattern

Recommendation:

Consider refactoring to:

class OrganizerUpdate(View):
# Only organizer-specific logic

class TeamManagementMixin:
# Team CRUD operations

class TeamMemberManagementMixin:
# Member/invite/token operations


● 2. Security Analysis ⚠

Critical Issues:

  1. Potential TypeError with user.traits
    - Code checks user.traits but doesn't handle None case
    - Could cause runtime errors if traits field is null
    - Fix needed: Add null safety checks
  2. Permission escalation risk
    - Staff users automatically receive admin/organizer traits
    - Ensure this is intentional and properly documented
    - Verify that staff status cannot be self-granted
  3. Direct messaging permission bypass
    - Logic conditionally removes DM permissions for organizers
    - Ensure this enforcement happens at API layer, not just UI
    - Need backend validation to prevent API abuse
  4. Audit logging gaps
    - Team operations have audit logging mentioned, but verify completeness
    - Video permission changes should be logged for compliance

Recommendations:

  • Add explicit permission checks at API endpoints for video operations
  • Implement rate limiting for permission-sensitive operations
  • Add integration tests for permission boundary cases

● 3. Code Quality 🔧

Issues Identified:

  1. Sourcery AI feedback (from PR description):
    - _send_invite passes view instance instead of self.request.user - this is a bug
    - Duplicate change-tracking helpers should be consolidated
    - Large view methods need refactoring
  2. Missing imports in Vue components:
    - Copilot identified missing mapGetters imports for hasPermission
    - Will cause runtime errors in production
  3. Inconsistent error handling:
    - Permission error patterns vary across components
    - Should standardize on a common approach
  4. Team redirect logic:
    - Uses section=teams but should use section=permissions based on new UI structure

Required Fixes:

Fix _send_invite to use proper user context

def _send_invite(self, ...):
context = {
'user': self.request.user, # Not self
# ...
}


  1. Database Migration ✅

Migration 0003_team_can_video_create_channels_and_more.py:

Strengths:

  • All new fields have default=False - safe for existing rows
  • Proper verbose names and help text
  • No data transformations - low risk

Concerns:

  • No rollback test: Ensure migration can be reversed cleanly
  • Index consideration: Nine new boolean fields may benefit from partial indexes if queried frequently
  • Schema size: Adding 9 fields increases table width - acceptable but monitor

Recommendation:

  • Test migration rollback: python manage.py migrate app_name 0002
  • Consider adding compound index if filtering by multiple video permissions is common:
    indexes = [
    models.Index(fields=['can_video_manage_users', 'can_video_manage_rooms'],
    name='team_video_mgmt_idx')
    ]

● 5. API Backward Compatibility ⚠

Changes to API Serializers:

  • Extended organizer.py serializer with nine new video permission fields
  • Fields are additive only - good for backward compatibility

Potential Issues:

  1. API version consideration:
    - New fields exposed in /api/v1/ endpoints
    - Existing API clients won't break (fields are optional)
    - But no API version bump - consider documenting in changelog
  2. Permission filtering:
    - Does API properly filter teams based on requesting user's permissions?
    - Verify that users can't read video permissions they shouldn't see
  3. Batch operations:
    - If bulk team updates are supported, ensure video permissions are validated

Recommendation:

  • Add API tests for new permission fields
  • Document new fields in API schema/OpenAPI spec
  • Verify permission-based filtering in serializer querysets

  1. Frontend-Backend Integration 🔍

Key Integration Points:

  1. Permission checks in Vue components:
    - Components gate UI elements behind hasPermission checks
    - Risk: Missing imports could cause undefined behavior
    - Need: Ensure all components importing permission helpers are tested
  2. Trait synchronization:
    - Backend generates JWT traits from team permissions
    - Frontend must decode and respect these traits
    - Verify: Token refresh after permission changes
  3. Real-time updates:
    - Permission changes may not propagate to active sessions immediately
    - Consider: WebSocket notification for permission revocation
  4. Validation mismatch:
    - Frontend filters UI based on permissions
    - Backend MUST enforce same rules at API level
    - Critical: Double-check all video operation endpoints validate permissions

Recommendation:
// Ensure consistent permission checks
// Backend (Django)
@permission_required('can_video_manage_rooms')
def create_room(request):
# ...

// Frontend (Vue)
if (hasPermission('can_video_manage_rooms')) {
// Show create button
}


● 7. Testing Requirements ❗

Critical Gaps:

Based on the scope of changes (52 files, major permission system overhaul), the PR should include:

  1. Unit tests needed:
    - test_video_permissions.py: Test trait mapping for all 9 permission combinations
    - test_team_model.py: Validate permission field constraints
    - test_organizer_view.py: Test team CRUD operations in unified view
    - test_permission_inheritance.py: Test staff user trait escalation
  2. Integration tests needed:
    - Team creation with video permissions via API
    - Permission enforcement at video endpoints
    - Trait generation and JWT encoding
    - Permission revocation propagation
  3. Frontend tests needed:
    - Component permission gating (Vue Test Utils)
    - Permission helper function coverage
    - UI state updates after permission changes
  4. Migration tests:
    - Forward migration on production-like dataset
    - Rollback migration safety
    - Performance test with large team tables
  5. Regression tests:
    - Legacy URL redirects work correctly
    - Existing team permissions remain functional
    - No permission leakage between organizers

Recommendation:

Minimum test coverage before merge:

src/tests/base/test_video_permissions.py # New file
src/tests/control/test_organizer_unified.py # New file
src/tests/api/test_team_api_video_perms.py # New file

Run with:

py.test src/tests/ -k video_permission -v


● 8. Additional Concerns

  1. Documentation:
    - No updates to CLAUDE.md or developer docs visible
    - Video permission fields need documenting for future developers
    - API changelog should note new serializer fields
  2. Performance:
    - Nine new boolean fields on Team model - minimal impact
    - Permission caching mentioned but implementation not visible
    - Consider Django's @cached_property for trait computation
  3. User Experience:
    - Redirect from legacy URLs to unified page - good
    - But users may be confused by section parameter inconsistency
    - Need user documentation for new permission granularity

norbusan added a commit that referenced this pull request Dec 16, 2025
This commit fixes critical bugs and adds test coverage for the unified
admin page and video permissions feature.

## Blocking Issues Fixed

1. **_send_invite Bug**: Fixed 3 locations passing view/serializer instance
   instead of user object to email context
   - app/eventyay/eventyay_common/views/team.py
   - app/eventyay/eventyay_common/views/organizer.py
   - app/eventyay/api/serializers/organizer.py

2. **Null Safety for user.traits**: Added defensive checks to prevent TypeError
   - app/eventyay/base/models/event.py (2 locations)
   - app/eventyay/base/models/world.py (2 locations)
   - app/eventyay/base/services/user.py (2 locations)

3. **Redirect Consistency**: Fixed legacy team redirect to use correct section
   - Changed from 'section=teams' to 'section=permissions'

## Tests Added

- Created src/tests/base/test_video_permissions.py (12 new tests)
- Added 6 video permission API tests to src/tests/api/test_teams.py
- Achieved 100% coverage of video permission fields

## Documentation

- Added PR_1348_FIXES_SUMMARY.md with complete change summary

All changes are backward compatible and migration-safe.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Copy link
Member

@hongquan hongquan left a comment

Choose a reason for hiding this comment

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

For type hint, please use one with lowercase, like list, dict, not uppercase like List, Dict.
The uppercase is for older Python, we don't need to keep compatible with older Python.

Copy link
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 52 out of 52 changed files in this pull request and generated 15 comments.


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

@Saksham-Sirohi
Copy link
Contributor Author

@sourcery-ai review

@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Dec 19, 2025

Sorry @Saksham-Sirohi, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@Saksham-Sirohi
Copy link
Contributor Author

Screen.Recording.2025-12-19.at.7.43.42.PM.mov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Backlog

4 participants