Eventyay Common: Create an event dashboard #484
Conversation
Reviewer's Guide by SourceryThis pull request introduces a new event dashboard to Eventyay Common. It provides an overview of event information and actions, including tickets, talks, and videos. The dashboard is accessible from the event page and includes widgets for managing different aspects of the event. Class diagram for the event dashboard views and navigationclassDiagram
class EventDashboard {
+event_index(request, organizer, event)
+event_index_widgets_lazy(request, organizer, event)
+rearrange(widgets: list)
}
class Navigation {
+get_event_navigation(request)
+get_global_navigation(request)
}
class Context {
+_default_context(request)
}
EventDashboard --> Navigation: uses
EventDashboard --> Context: uses
note for EventDashboard "Main dashboard controller"
note for Navigation "Handles navigation structure"
note for Context "Provides context data"
File-Level Changes
Assessment against linked issues
Possibly 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: 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.
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: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 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.
There was a problem hiding this comment.
Please add screenshots with all components enabled and add spacing below the menu in the mobile view.
What happens if the a component is not enabled, e.g. talk or video? What happens when you click on the "dashboard" options in that case?
There should probably be a pop up "You need to enable this component first. You can do this here."
Or what is your suggestion?
If the component is not enable, the button will not display. I render the event template based on this condition. |
src/pretix/eventyay_common/templates/eventyay_common/event/index.html
Outdated
Show resolved
Hide resolved
| ctx['complain_testmode_orders'] = False | ||
| if not request.event.live and ctx['has_domain']: | ||
| child_sess = request.session.get(f'child_session_{request.event.pk}') | ||
| s = SessionStore() |
There was a problem hiding this comment.
Could you explain why you access the session this way, whereas the request is already available.
Note that, the documentation says that this way is for the code outside a view, like in Django commands, in background job, in unittest, etc...
But this code has request, meaning it is inside a view.
|
|
||
| ] | ||
|
|
||
| merge_in(nav, sorted( |
There was a problem hiding this comment.
Is this code tested? It looks weird because:
sorted()requires the 1st argument to be iterable, but you pass a scalar value.sum()accepts only one positional parameter, the 2nd parameter must be keyword one, but you pass two positional ones.
| {% endfor %} | ||
| </ul> | ||
| <div class="panel-footer"> | ||
| <a href="{% url "control:event.requiredactions" event=request.event.slug organizer=request.event.organizer.slug %}"> |
There was a problem hiding this comment.
Use different quote styles for nesting.
| if request.GET.get("subevent", "") != "" and request.event.has_subevents: | ||
| i = request.GET.get("subevent", "") | ||
| with contextlib.suppress(SubEvent.DoesNotExist): | ||
| subevent = request.event.subevents.get(pk=i) |
There was a problem hiding this comment.
Overkill, if the SubEvent.DoesNotExist can be ignored and you want to return None in that case, the code can be as simple as (without with block):
subevent = request.event.subevents.filter(pk=i).first()
Create dashboard the common event
Mobile view
Alert when component is not enabled
This PR resolves #455
Summary by Sourcery
New Features:
Summary by Sourcery
New Features: