-
Notifications
You must be signed in to change notification settings - Fork 131
feat(permission): make talk component accessible in admin mode #1159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reviewer's GuideThis PR introduces an admin mode for staff users via temporary sudo sessions, injecting UI controls to enter/exit admin mode, providing warnings for unaudited sessions, and overriding permission checks across the application through a new check_admin_mode hook and has_event_perm tag; it also adapts view querysets and talk question filtering to respect admin mode and adds minimal styling for the new controls. Sequence diagram for permission checks with admin modesequenceDiagram
participant StaffUser as actor Staff User
participant WebApp as Web Application
participant Permission as Permission System
StaffUser->>WebApp: Request access to restricted resource
WebApp->>Permission: has_event_perm(permission, user, request, obj)
Permission->>WebApp: Calls check_admin_mode(user, request)
alt Active staff session
Permission-->>WebApp: Returns True (grant access)
else No active staff session
Permission-->>WebApp: Checks standard permission
Permission-->>WebApp: Returns result
end
WebApp->>StaffUser: Grants or denies access
Class diagram for staff session and admin mode permission checkclassDiagram
class User {
+has_perm(perm, obj)
+has_active_staff_session(session_key)
}
class StaffSession {
user
date_end
comment
}
class Permissions {
+check_admin_mode(self, request)
}
User "1" -- "*" StaffSession : has
Permissions ..> User : uses
Permissions ..> StaffSession : checks
StaffSession : +user
StaffSession : +date_end
StaffSession : +comment
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this 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 they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `app/eventyay/common/permissions.py:6` </location>
<code_context>
+logger = logging.getLogger(__name__)
+
+
+def check_admin_mode(self, request=None):
+ if request and hasattr(request.user, 'has_active_staff_session') and request.user.has_active_staff_session(request.session.session_key):
+ logger.debug(
</code_context>
<issue_to_address>
**suggestion:** The use of 'self' in check_admin_mode is ambiguous.
The function's 'self' parameter is inconsistently used, which may cause confusion or errors. Rename it to 'user' or clarify its expected type for better readability and maintainability.
</issue_to_address>
### Comment 2
<location> `app/eventyay/static/common/css/base.css:173-175` </location>
<code_context>
+ color: var(--color-offwhite);
+}
+
+.end-admin-mode {
+ padding: 12px;
+ background-color: red;
+}
+
</code_context>
<issue_to_address>
**suggestion:** Hardcoded color value for .end-admin-mode may reduce theme flexibility.
Consider replacing the hardcoded 'red' with a CSS variable or an existing color class to improve maintainability and theme consistency.
Suggested implementation:
```
.end-admin-mode {
padding: 12px;
background-color: var(--color-danger);
}
```
If `--color-danger` is not already defined in your CSS variables, you should add it to your root or theme section, for example:
:root {
--color-danger: #d32f2f; /* or your preferred danger color */
}
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this 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 pull request implements an admin mode feature that allows staff users to bypass normal permission checks across the application. The changes introduce a staff session mechanism where superusers can enter an elevated "admin mode" to access resources regardless of their regular team permissions.
Key changes:
- Added
check_admin_mode()utility function to verify if a user has an active staff session - Updated permission checks across views, serializers, and templates to incorporate admin mode bypasses
- Introduced
has_event_permtemplate tag to consolidate permission checking with admin mode support - Added UI elements for starting/ending admin sessions in the navigation bar
Reviewed Changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| app/eventyay/common/permissions.py | New module defining check_admin_mode() to check if user has active staff session |
| app/eventyay/orga/templatetags/staff_session.py | New template tags including has_event_perm for permission checks with admin mode support |
| app/eventyay/talk_rules/submission.py | Updated questions_for_user() to accept self parameter and check admin mode |
| app/eventyay/common/views/mixins.py | Added admin mode check to _check_permission() method |
| app/eventyay/common/views/generic.py | Added admin mode checks to permission validation methods |
| app/eventyay/orga/views/dashboard.py | Modified event list queryset to show all events in admin mode |
| app/eventyay/orga/views/cfp.py | Updated questions_for_user() call with new signature |
| talk/src/pretalx/api/views/*.py | Updated questions_for_user() calls in submission, speaker, and question viewsets |
| talk/src/pretalx/api/serializers/question.py | Updated questions_for_user() call in serializer init |
| app/eventyay/orga/templates/orga/base.html | Added admin mode UI elements and updated permission checks |
| Multiple template files | Replaced has_perm with has_event_perm to support admin mode |
| app/eventyay/static/common/css/base.css | Added CSS styling for admin mode UI elements |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sak1012
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @Gagan-Ram! thanks, the changes work, but there is a design mismatch
In the common dashboard the "End Admin Session" is highlighted in Red

But this does not match in the talks dashboard

mariobehling
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the talk component please match the spacing between the button "admin mode" and the right side button/user info as it is in the tickets component.
The red background is indeed added in css file. Please run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 32 out of 32 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fixes: #1099
Summary by Sourcery
Enable a staff impersonation (admin) mode that bypasses standard permission checks across Orga, CFP, agenda, and talk components by introducing UI controls, centralizing admin mode logic, updating permission tags, and adjusting data queries to grant full access when in admin mode.
New Features:
Enhancements: