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 an event dashboard, enhancing the user interface with a new overview page for event components (tickets, talks, videos). It also includes a fix for a CSS conflict related to the info box. Class diagram for EventIndexView and related componentsclassDiagram
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 TemplateView {
+get_context_data()
}
EventIndexView --|> TemplateView
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
hongquan
requested changes
Jan 14, 2025
There was a problem hiding this comment.
Hey @HungNgien - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider enhancing error handling in utility functions like get_subevent() to properly propagate errors to the UI rather than just logging them. This would improve the user experience when invalid data is provided.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 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.
hongquan
approved these changes
Jan 18, 2025
|
Please resolve conflicts. Also ensure the main eventyay-common dashboard "event box heading text", e.g Paypal event Jan 19... links to this event dashboard in the PR here.
|
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
Introduce an event dashboard to provide a comprehensive overview of event details, including tickets, talks, and videos, along with quick access to each component.
New Features:
Tests: