chore: common unified admin page #1348
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Reviewer's GuideUnifies 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 viewsequenceDiagram
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
Sequence diagram for video access token generation with fine-grained traitssequenceDiagram
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
Sequence diagram for SPA room creation filtered by permissionssequenceDiagram
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
Updated class diagram for team and video permission modelingclassDiagram
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
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Saksham-Sirohi
force-pushed
the
unified-admin
branch
from
7db9116 to
4ba0ea3
Compare
Saksham-Sirohi
force-pushed
the
unified-admin
branch
3 times, most recently
from
de137e0 to
1043149
Compare
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes and found some issues that need to be addressed.
- In OrganizerUpdate._send_invite you pass
selfas theuserin the email context, which is the view instance rather than a User; this likely should beself.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_dictand_collect_team_change_dataare very similar and both walk m2m fields underscopes_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>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Saksham-Sirohi
force-pushed
the
unified-admin
branch
from
16bd1d6 to
1de7855
Compare
Contributor
There was a problem hiding this comment.
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
force-pushed
the
unified-admin
branch
2 times, most recently
from
9772026 to
28e5b72
Compare
mariobehling
requested changes
Nov 30, 2025
Member
mariobehling
left a comment
mariobehling
left a comment
There was a problem hiding this comment.
@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.
Saksham-Sirohi
force-pushed
the
unified-admin
branch
from
3a71613 to
9021935
Compare
Saksham-Sirohi
force-pushed
the
unified-admin
branch
from
9021935 to
6b10ea9
Compare
Contributor
There was a problem hiding this comment.
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.
Comment on lines
+27
to
+31
| // Check permission before creating direct message | ||
| if (!this.hasPermission('world:chat.direct')) { | ||
| this.$emit('close') | ||
| return | ||
| } |
There was a problem hiding this comment.
The create() method checks permissions and closes the prompt with this.$emit('close') but doesn't return early after the permission check fails (lines 28-30). The method continues to line 32 where it dispatches the action. Add an explicit return statement after this.$emit('close') on line 30.
| this.error = null | ||
| // Check if any types are available | ||
| if (this.types.length === 0) { | ||
| this.error = this.$t('CreateChatPrompt:error:no-permission') || 'You do not have permission to create channels.' |
There was a problem hiding this comment.
The error message on line 76 uses this.$t('CreateChatPrompt:error:no-permission') which falls back to a hard-coded string. However, this translation key doesn't appear to be defined. Consider either adding the translation key or using a more standard error message pattern. The same applies to lines 83, 88, and other similar error messages in this file.
Comment on lines
+82
to
+91
| if (this.type === 'text' && !this.hasPermission('world:rooms.create.chat')) { | ||
| this.error = this.$t('CreateChatPrompt:error:no-text-permission') || 'You do not have permission to create text channels.' | ||
| this.$emit('close') | ||
| return | ||
| } | ||
| if (this.type === 'video' && !this.hasPermission('world:rooms.create.bbb')) { | ||
| this.error = this.$t('CreateChatPrompt:error:no-video-permission') || 'You do not have permission to create video channels.' | ||
| this.$emit('close') | ||
| return | ||
| } |
There was a problem hiding this comment.
In the permission check on line 82, the error message is set but then followed by return on line 85 which closes the prompt. However, the error won't be visible to the user if the component immediately returns. The pattern here differs from lines 73-78 where this.$emit('close') is called. Consider making the error handling consistent across all permission checks.
Comment on lines
+29
to
+41
| team_id = kwargs.get('team') | ||
| query = '?section=teams' | ||
| if team_id: | ||
| query = f'{query}&team={team_id}' | ||
| target = reverse( | ||
| 'eventyay_common:organizer.update', | ||
| kwargs={'organizer': request.organizer.slug}, | ||
| ) | ||
| messages.info( | ||
| request, | ||
| _('Team management has moved into the unified organizer page.'), | ||
| ) | ||
| return redirect(f'{target}{query}') |
There was a problem hiding this comment.
The redirect message on line 38 says "Team management has moved into the unified organizer page." However, the target URL uses section=teams which should probably be section=permissions based on the tab structure shown in the template (edit.html lines 22-25). This could lead to users being redirected to the wrong tab.
Comment on lines
+206
to
+215
| # limit_tracks is event-scoped, so we don't prefetch it here | ||
| # It will be accessed through TeamForm which has proper scoping | ||
| with scopes_disabled(): | ||
| self._team_queryset = ( | ||
| self.request.organizer.teams.annotate( | ||
| memcount=Count('members', distinct=True), | ||
| eventcount=Count('limit_events', distinct=True), | ||
| invcount=Count('invites', distinct=True), | ||
| ) | ||
| .prefetch_related('members', 'invites', 'tokens', 'limit_events') |
There was a problem hiding this comment.
[nitpick] The comment on line 206 says "limit_tracks is event-scoped, so we don't prefetch it here" but then the queryset on line 215 does include .prefetch_related('members', 'invites', 'tokens', 'limit_events'). While limit_tracks is not prefetched, limit_events is, which could still cause scoping issues if not handled carefully. Verify that the scoping is correctly managed for limit_events.
Comment on lines
26
to
34
| async create(users) { | ||
| // Check permission before creating direct message | ||
| if (!this.hasPermission('world:chat.direct')) { | ||
| this.$emit('close') | ||
| return | ||
| } | ||
| // TODO error handling, progress | ||
| await this.$store.dispatch('chat/openDirectMessage', {users: users}) | ||
| this.$emit('close') |
There was a problem hiding this comment.
Missing hasPermission method in component: The code uses this.hasPermission() on lines 28 and following, but the component doesn't import or define hasPermission in its computed properties or methods. This will cause a runtime error. The component should include ...mapGetters(['hasPermission']) in the computed section.
Comment on lines
+75
to
+78
| if (this.types.length === 0) { | ||
| this.error = this.$t('CreateChatPrompt:error:no-permission') || 'You do not have permission to create channels.' | ||
| this.$emit('close') | ||
| return |
There was a problem hiding this comment.
After checking permissions and setting an error, the code calls this.$emit('close') (lines 77, 84, 89) which closes the prompt. However, the user won't see the error message because the prompt is immediately closed. Either remove the close call to keep the prompt open with the error, or remove the error assignment since it won't be displayed.
Comment on lines
+188
to
+193
| def redirect_team_management(request, organizer, team_pk=None, **kwargs): | ||
| target = reverse("eventyay_common:organizer.update", kwargs={"organizer": organizer}) | ||
| query = "?section=permissions" | ||
| if team_pk is not None: | ||
| query = f"{query}&team={team_pk}" | ||
| return redirect(f"{target}{query}") |
There was a problem hiding this comment.
The query string construction uses f-string concatenation which could lead to inconsistencies. On line 192, if team_pk is None, the result will be "?section=permissions&team=None" which may not be the intended behavior. Consider using urllib.parse.urlencode() or conditional logic to only include the team parameter when team_pk is not None.
Comment on lines
+39
to
+42
| VIDEO_TRAIT_ROLE_MAP: Dict[str, str] = { | ||
| definition.trait_name: definition.trait_name | ||
| for definition in VIDEO_PERMISSION_DEFINITIONS | ||
| } |
There was a problem hiding this comment.
[nitpick] The VIDEO_TRAIT_ROLE_MAP dictionary on lines 39-42 maps each trait name to itself, which seems redundant. If this is intentional for future extensibility, consider adding a comment explaining the purpose. Otherwise, this could be simplified or removed if not needed.
Suggested change
| VIDEO_TRAIT_ROLE_MAP: Dict[str, str] = { | |
| definition.trait_name: definition.trait_name | |
| for definition in VIDEO_PERMISSION_DEFINITIONS | |
| } |
| from eventyay.common.text.phrases import phrases | ||
| from eventyay.common.urls import EventUrls | ||
| from eventyay.core.permissions import Permission, SYSTEM_ROLES | ||
| from eventyay.core.permissions import MAX_PERMISSIONS_IF_SILENCED, Permission, SYSTEM_ROLES |
There was a problem hiding this comment.
The import statement from eventyay.core.permissions import MAX_PERMISSIONS_IF_SILENCED, Permission, SYSTEM_ROLES adds MAX_PERMISSIONS_IF_SILENCED but this import is never used in the visible code changes. Consider removing unused imports to keep the code clean.
Suggested change
| from eventyay.core.permissions import MAX_PERMISSIONS_IF_SILENCED, Permission, SYSTEM_ROLES | |
| from eventyay.core.permissions import Permission, SYSTEM_ROLES |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #921,#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:
Bug Fixes:
Enhancements: