-
Notifications
You must be signed in to change notification settings - Fork 122
chore(ui/permissions): hide “Create Organizer” in admin tickets page, allow creation in common organizers view #890
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
base: enext
Are you sure you want to change the base?
Conversation
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.
The pull request #890 has too many files changed.
The GitHub API will only let us fetch up to 300 changed files, and this pull request has 3968.
|
So, as we proceed I think we need to have this in a bit more advanced way. In the admin section of the platform (e.g. https://wikimedia.eventyay.com/tickets/control/admin/global/settings/) there should be a section "Organizer Creation" with the following tick boxes.
For the second option test that accounts have payment info on file. |
|
Following the above comment which requires to add an option for create organiser permissions in /tickets/control/admin/global/settings/ should follow the PR at #794. We need this PR to be made against the enext branch first. |
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.
Please make the required changes to display the create organizer option depending on the platform configuration.
110ebaa to
473f9fa
Compare
473f9fa to
221dd1a
Compare
221dd1a to
896f553
Compare
@mariobehling updated pr please take a look
|
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 PR implements configurable permissions for organizer creation by introducing a new OrganizerCreationPermissionMixin that allows system administrators to control who can create organizers through global settings. Previously, only users with active staff sessions could create organizers.
Key changes:
- Added
OrganizerCreationPermissionMixinwith configurable permission logic based on global settings - Introduced two new global settings:
allow_all_users_create_organizerandallow_payment_users_create_organizer - Updated
OrganizerCreateandOrganizerListviews to use the new permission mixin and display create buttons conditionally
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| app/eventyay/control/permissions.py | Added OrganizerCreationPermissionMixin with _can_create_organizer and _user_has_payment_info helper methods |
| app/eventyay/control/forms/global_settings.py | Added two new boolean fields for controlling organizer creation permissions |
| app/eventyay/control/views/organizer_views/organizer_view.py | Integrated permission mixin into OrganizerCreate and OrganizerList views with permission checks in dispatch |
| app/eventyay/control/templates/pretixcontrol/organizers/index.html | Conditionally display create button based on can_create_organizer context variable |
| app/eventyay/eventyay_common/views/organizer.py | Integrated permission mixin into OrganizerCreate and OrganizerList views with permission checks in dispatch |
| app/eventyay/eventyay_common/templates/eventyay_common/organizers/index.html | Changed from staff_session to can_create_organizer for displaying create button |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| allow_all_users = gs.settings.get('allow_all_users_create_organizer', None, as_type=bool) | ||
| allow_payment_users = gs.settings.get('allow_payment_users_create_organizer', None, as_type=bool) | ||
|
|
||
| # If neither option is explicitly set, default to allowing all users (permissive default) | ||
| if allow_all_users is None and allow_payment_users is None: | ||
| return True | ||
|
|
||
| # If all users are allowed | ||
| if allow_all_users: | ||
| return True |
Copilot
AI
Nov 1, 2025
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.
The permission logic could lead to unexpected behavior when both settings are enabled. If both allow_all_users_create_organizer and allow_payment_users_create_organizer are set to True, the function returns True for all users at line 197, making the second check redundant. Consider documenting this precedence or simplifying the logic to make the relationship between these settings clearer (e.g., treat allow_all_users as overriding allow_payment_users).
| for organizer in user_organizers: | ||
| billing = OrganizerBillingModel.objects.filter( | ||
| organizer=organizer, | ||
| stripe_customer_id__isnull=False | ||
| ).exclude(stripe_customer_id='').first() | ||
| if billing: | ||
| return True | ||
|
|
||
| return False |
Copilot
AI
Nov 1, 2025
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.
The N+1 query pattern in _user_has_payment_info will execute one query per organizer. This can be optimized using a single query with exists(). Consider replacing lines 226-232 with: return OrganizerBillingModel.objects.filter(organizer__in=user_organizers, stripe_customer_id__isnull=False).exclude(stripe_customer_id='').exists(). This will execute a single database query instead of potentially many.
| for organizer in user_organizers: | |
| billing = OrganizerBillingModel.objects.filter( | |
| organizer=organizer, | |
| stripe_customer_id__isnull=False | |
| ).exclude(stripe_customer_id='').first() | |
| if billing: | |
| return True | |
| return False | |
| return OrganizerBillingModel.objects.filter( | |
| organizer__in=user_organizers, | |
| stripe_customer_id__isnull=False | |
| ).exclude(stripe_customer_id='').exists() |
| messages.error( | ||
| request, | ||
| _('You do not have permission to create organizers. Please contact an administrator.') | ||
| ) |
Copilot
AI
Nov 1, 2025
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.
The error message is duplicated - shown both as a user-facing message via messages.error() and as the exception message in PermissionDenied(). Since PermissionDenied typically triggers error handling that may display its message, this could result in duplicate messages to the user. Consider removing the messages.error() call and relying on exception handling to display the error, or ensure the exception handler doesn't display the PermissionDenied message.
| messages.error( | |
| request, | |
| _('You do not have permission to create organizers. Please contact an administrator.') | |
| ) |
| messages.error( | ||
| request, | ||
| _('You do not have permission to create organizers. Please contact an administrator.') | ||
| ) |
Copilot
AI
Nov 1, 2025
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.
The error message is duplicated - shown both as a user-facing message via messages.error() and as the exception message in PermissionDenied(). Since PermissionDenied typically triggers error handling that may display its message, this could result in duplicate messages to the user. Consider removing the messages.error() call and relying on exception handling to display the error, or ensure the exception handler doesn't display the PermissionDenied message.
| messages.error( | |
| request, | |
| _('You do not have permission to create organizers. Please contact an administrator.') | |
| ) |

fixes #651 to enext