Eventyay Common: Create an event dashboard #505
Conversation
This file contains 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
Reviewer's Guide by SourceryThis pull request introduces a new event dashboard feature in the Eventyay Common platform. It provides an overview of event information, including ticket sales, talks, and video configuration. The dashboard UI is implemented with a new template and leverages existing components for tickets, talks, and videos. Routing is updated to include the new dashboard view. Class diagram for Event Dashboard ViewsclassDiagram
class EventIndexView {
+template_name: str
+get_context_data()
-_get_user_permissions()
-_collect_dashboard_widgets()
-_filter_log_entries()
-_check_event_statuses()
+rearrange(widgets)
}
class Event {
+talk_schedule_url
+talk_session_url
+talk_speaker_url
+talk_dashboard_url
+talk_settings_url
}
class EventMixin {
+has_paid_things()
}
Event --|> EventMixin
EventIndexView ..> Event: uses
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @HungNgien - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| widgets.extend(result) | ||
| return self.rearrange(widgets) | ||
|
|
||
| def _filter_log_entries( |
There was a problem hiding this comment.
suggestion (performance): Consider caching ContentType lookups as class attributes to avoid repeated database queries
Move the ContentType.objects.get_for_model() calls outside the method and store them as class attributes. This will prevent executing the same queries multiple times.
Suggested implementation:
return []
# Cache ContentType lookups at class level
order_content_type = ContentType.objects.get_for_model(Order)
invoice_content_type = ContentType.objects.get_for_model(Invoice)
checkin_content_type = ContentType.objects.get_for_model(Checkin)
item_content_type = ContentType.objects.get_for_model(Item)
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
request = self.requestYou'll also need to:
- Add imports for ContentType and the models (Order, Invoice, Checkin, Item) at the top of the file if not already present
- Update any places in the code where ContentType.objects.get_for_model() is called to use these class attributes instead
- Make sure these models are the correct ones being used in the log entries - adjust the model list according to what's actually being filtered in the log entries
Comment on lines
+87
to
89
| @media (min-width: $screen-xs-min) and (max-width: $screen-xs-max) { | ||
| .dashboard .widget-container.widget-small.no-padding.column { | ||
| margin-right: 0px; |
There was a problem hiding this comment.
suggestion: Add media query for screens smaller than xs breakpoint
Consider adding a media query for screens smaller than $screen-xs-min to ensure proper layout on very small devices.
Suggested change
| @media (min-width: $screen-xs-min) and (max-width: $screen-xs-max) { | |
| .dashboard .widget-container.widget-small.no-padding.column { | |
| margin-right: 0px; | |
| @media (max-width: $screen-xs-min - 1) { | |
| .dashboard .widget-container.widget-small.no-padding.column { | |
| margin-right: 0px; | |
| } | |
| } | |
| @media (min-width: $screen-xs-min) and (max-width: $screen-xs-max) { | |
| .dashboard .widget-container.widget-small.no-padding.column { | |
| margin-right: 0px; |
hongquan
requested changes
Jan 10, 2025
| return nav | ||
|
|
||
|
|
||
| def get_event_navigation(request: HttpRequest) -> List[Dict[str, Any]]: |
There was a problem hiding this comment.
Define this function to receive event instead of request, so that this function is only called after we make sure request.event exists.
With current implementation, all the use of `request.event' inside the body is unsafe (lack of checking).
| ctx["is_talk_event_created"] = True | ||
|
|
||
| if organizer := getattr(request, "organizer", None): | ||
| ctx["nav_items"] = get_event_navigation(request) |
There was a problem hiding this comment.
The get_event_navigation() depends on request.event but here you only check request.organizer existence.
| ] | ||
|
|
||
| # Merge plugin-provided navigation items | ||
| plugin_nav_items = sum( |
There was a problem hiding this comment.
I remember I saw this code in previous PR and I asked why this sum() usage looks weird. Wonder if it is tested.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Event dashboard
Fix css conflict of info box
This PR resolves #455
Summary by Sourcery
New Features: