Add sessions to sidebar and show filter and export options #215
Conversation
Reviewer's Guide by SourceryThis pull request enhances the user interface by adding sessions to the sidebar and providing filter and export options. The changes include the addition of a new sessions view, export functionality, and corresponding backend support. Translations for the new 'Sessions' label have been added in multiple languages. File-Level Changes
Tips
|
|
new page sessions with filter and export
export is added for schedule as well
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @odkhang - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider optimizing QR code generation to improve performance, possibly by generating on-demand or implementing caching.
- The export functionality is duplicated in both schedule and sessions views. Consider extracting this into a shared component or mixin to reduce code duplication.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟡 Security: 1 issue found
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| @@ -112,6 +165,34 @@ export default { | |||
| } else { | |||
| this.$store.dispatch('schedule/filter', {type: 'fav'}) | |||
| } | |||
| }, | |||
| async makeExport() { | |||
There was a problem hiding this comment.
suggestion: Consider refactoring the makeExport function for better separation of concerns and error handling.
The makeExport function is handling both API calls and file downloads. Consider separating these concerns for better maintainability. Improve error handling to provide better user feedback instead of just logging to console. Use let or const instead of var for variable declarations. Consider using a dedicated service for API calls to keep component logic cleaner.
async makeExport() {
try {
this.isExporting = true;
const exportData = await this.fetchExportData();
await this.downloadExportFile(exportData);
} catch (error) {
this.handleExportError(error);
} finally {
this.isExporting = false;
}
},
| @@ -165,6 +166,33 @@ def get_uid_from_token(request, world_id): | |||
| return token_decode.get("uid") | |||
|
|
|||
|
|
|||
| class ExportView(APIView): | |||
There was a problem hiding this comment.
🚨 issue (security): Enhance security and error handling in ExportView.
The ExportView lacks authentication, which could be a security risk. Implement proper authentication checks. Use Django's URL reverse function for safer and more maintainable URL construction. Improve error handling to properly communicate failures back to the client.
| @@ -0,0 +1,161 @@ | |||
| <template> | |||
There was a problem hiding this comment.
suggestion (performance): Optimize QR code generation and improve component structure.
Consider moving QR code generation to the server for better performance and reduced client-side bundle size. Clarify the usage of the outsideClick method, ensuring proper addition and removal of event listeners. Scope the CSS styles to avoid potential conflicts with other components. Consider using a Vue-specific QR code library for better optimization.
<template>
<div class="export-select">
<ServerSideQRCode v-if="showQRCode" :data="scheduleData" />
<button @click="toggleQRCode">Toggle QR Code</button>
This PR closes/references issue #190 .
How has this been tested?
Checklist
Summary by Sourcery
Add a new 'Sessions' section to the sidebar with filtering and export options, including a custom dropdown for export formats and QR code generation for certain formats. Enhance the schedule view and update the router to support the new section.
New Features:
Enhancements:
Documentation:
Tests: