-
Notifications
You must be signed in to change notification settings - Fork 136
refactor: replaced old pretix/venueless references with eventyay naming #1334
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
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRefactors the eventyay ticket video plugin from the old 'pretix_venueless' naming to the new 'eventyay_video' / 'eventyay-video' naming across Python config, URLs, templates, static asset paths, and locale directories, maintaining runtime behavior but aligning with Eventyay branding and namespace conventions. Updated class diagram for the eventyay video plugin configuration and viewsclassDiagram
class PluginApp {
bool "default" = true
str "name" = "eventyay-video"
str "verbose_name" = "Eventyay Video"
}
class EventyayVideoPluginMeta {
str "name" = "Eventyay Video"
str "author" = "Eventyay"
str "description" = "Grant access to your eventyay video event to your customers."
bool "visible" = true
str "picture" = "eventyay-video/eventyay-logo.192.png"
bool "featured" = true
str "version" = "__version__"
str "category" = "INTEGRATION"
list "compat" = []
}
class EventyayPluginMeta {
str "name" = "Eventyay Video"
str "author" = "Eventyay"
str "description" = "Grant access to your eventyay video event to your customers."
bool "visible" = true
str "picture" = "eventyay-video/eventyay-logo.192.png"
bool "featured" = true
str "version" = "__version__"
str "category" = "INTEGRATION"
list "compat" = []
}
class Event {
}
class VenuelessSettingsForm {
}
class EventSettingsViewMixin {
}
class EventSettingsFormView {
}
class SettingsView {
Event "model"
VenuelessSettingsForm "form_class"
str "template_name" = "eventyay_video/settings.html"
str "permission" = "can_change_settings"
str "get_success_url()"
}
PluginApp ..> EventyayVideoPluginMeta : "uses as upstream plugin meta"
PluginApp ..> EventyayPluginMeta : "uses as Eventyay plugin meta"
SettingsView --|> EventSettingsFormView : "inherits from"
SettingsView ..|> EventSettingsViewMixin : "mixes in"
SettingsView --> Event : "configures model"
SettingsView --> VenuelessSettingsForm : "uses form_class"
File-Level Changes
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 found some issues that need to be addressed.
- The plugin config
namewas changed toeventyay-videowhile the module/URL namespace useseventyay_video; consider aligning these (dash vs underscore) to avoid subtle namespace or plugin discovery issues. - The inner meta class
PretixPluginMetawas renamed toEventyayVideoPluginMetaeven though the comment says it's for upstream pretix compatibility; double-check whether the class name itself is relied on upstream and if so keep the original name while updating its contents. - Asset/template paths now mix
eventyay-video(e.g., inpicture) andeventyay_video(e.g., templates and URLs); it would be safer to standardize on one naming convention across filesystem paths and Django namespaces to prevent missing file or static path issues.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The plugin config `name` was changed to `eventyay-video` while the module/URL namespace uses `eventyay_video`; consider aligning these (dash vs underscore) to avoid subtle namespace or plugin discovery issues.
- The inner meta class `PretixPluginMeta` was renamed to `EventyayVideoPluginMeta` even though the comment says it's for upstream pretix compatibility; double-check whether the class name itself is relied on upstream and if so keep the original name while updating its contents.
- Asset/template paths now mix `eventyay-video` (e.g., in `picture`) and `eventyay_video` (e.g., templates and URLs); it would be safer to standardize on one naming convention across filesystem paths and Django namespaces to prevent missing file or static path issues.
## Individual Comments
### Comment 1
<location> `app/eventyay/plugins/eventyay-ticket-video/eventyay_video/apps.py:14` </location>
<code_context>
class PluginApp(_BasePluginConfig):
default = True
- name = 'pretix_venueless'
+ name = 'eventyay-video'
verbose_name = 'Eventyay Video'
</code_context>
<issue_to_address>
**issue (bug_risk):** AppConfig.name should generally be the Python path (likely with underscore) rather than a hyphenated identifier.
In Django AppConfig, `name` must be an importable Python module path. `'eventyay-video'` is not a valid module name and may break Django’s app registry or pretix’s plugin detection. Please use the actual package/module name instead, e.g. `'eventyay_video'`.
</issue_to_address>
### Comment 2
<location> `app/eventyay/plugins/eventyay-ticket-video/eventyay_video/apps.py:17` </location>
<code_context>
verbose_name = 'Eventyay Video'
- class PretixPluginMeta: # Keep for upstream pretix compatibility
+ class EventyayVideoPluginMeta: # Keep for upstream pretix compatibility
name = gettext_lazy('Eventyay Video')
author = 'Eventyay'
</code_context>
<issue_to_address>
**question (bug_risk):** Renaming PretixPluginMeta may break upstream pretix compatibility expectations.
This inner class is documented as being for upstream pretix compatibility, but its name changed from `PretixPluginMeta` to `EventyayVideoPluginMeta`. If pretix or related tooling expects the original `PretixPluginMeta` name (e.g., via reflection), this could silently break integration. Please either keep the original class name and update its contents, or confirm that no upstream code relies on the `PretixPluginMeta` identifier before proceeding with the rename.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| class PluginApp(_BasePluginConfig): | ||
| default = True | ||
| name = 'pretix_venueless' | ||
| name = 'eventyay-video' |
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.
issue (bug_risk): AppConfig.name should generally be the Python path (likely with underscore) rather than a hyphenated identifier.
In Django AppConfig, name must be an importable Python module path. 'eventyay-video' is not a valid module name and may break Django’s app registry or pretix’s plugin detection. Please use the actual package/module name instead, e.g. 'eventyay_video'.
| verbose_name = 'Eventyay Video' | ||
|
|
||
| class PretixPluginMeta: # Keep for upstream pretix compatibility | ||
| class EventyayVideoPluginMeta: # Keep for upstream pretix compatibility |
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.
question (bug_risk): Renaming PretixPluginMeta may break upstream pretix compatibility expectations.
This inner class is documented as being for upstream pretix compatibility, but its name changed from PretixPluginMeta to EventyayVideoPluginMeta. If pretix or related tooling expects the original PretixPluginMeta name (e.g., via reflection), this could silently break integration. Please either keep the original class name and update its contents, or confirm that no upstream code relies on the PretixPluginMeta identifier before proceeding with the rename.
|
check this i replaced the files name to thier new respective names . |
Description me likh dena:
Found and replaced all old references
Updated imports, template folders, PluginApp name etc.
Verified using automated search commands
Tested plugin loads without errors (if applicable)
Summary by Sourcery
Rename the event ticket video plugin to use the new eventyay_video naming across code and assets.
Enhancements: