Skip to content
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

Add Video-Component Logic to User's Dashboard #314

Closed
wants to merge 3 commits into from

Conversation

AviGawande
Copy link
Contributor

@AviGawande AviGawande commented Sep 6, 2024

This PR fixes #301 for implementing the Eventyay-video-component to User Dashboard.

Summary by Sourcery

Add a new 'Components' view to the user dashboard, allowing users to check the installation status of Eventyay-Talk and Eventyay-Video components. Refactor the HTML template structure for better maintainability.

New Features:

  • Introduce a new 'Components' view in the user dashboard to display the availability of Eventyay-Talk and Eventyay-Video components.

Enhancements:

  • Refactor the base HTML template to extend from a common base and streamline the inclusion of static resources.

@AviGawande AviGawande changed the base branch from development to interconnection September 10, 2024 18:25
@AviGawande AviGawande marked this pull request as ready for review September 16, 2024 06:57
Copy link

sourcery-ai bot commented Sep 16, 2024

Reviewer's Guide by Sourcery

This pull request implements the Eventyay-video-component in the User Dashboard. It adds a new Components view, updates the authentication base template, and modifies related Python files to support the new functionality.

File-Level Changes

Change Details Files
Added a new Components view to display installed Eventyay components
  • Created a new ComponentsView function in auth.py
  • Added a new URL route for the Components view
  • Created a new components.html template
src/pretix/control/views/auth.py
src/pretix/control/urls.py
src/pretix/control/templates/pretixcontrol/auth/components.html
Updated the authentication base template to include component information
  • Removed unnecessary HTML structure and meta tags
  • Added conditional rendering for Eventyay-Talk and Eventyay-Video components
  • Updated the template to extend from the base authentication template
src/pretix/control/templates/pretixcontrol/auth/base.html
Modified settings to support component detection
  • Added BASE_DIR setting to locate the project's base directory
src/pretix/settings.py

Tips
  • Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
  • Continue your discussion with Sourcery by replying directly to review comments.
  • You can change your review settings at any time by accessing your dashboard:
    • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
    • Change the review language;
  • You can always contact us if you have any questions or feedback.

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @AviGawande - I've reviewed your changes - here's some feedback:

Overall Comments:

  • Consider catching more specific exceptions in ComponentsView instead of a general Exception to avoid masking potential issues.
  • The template for Eventyay-Talk and Eventyay-Video components contains similar code. Consider refactoring this into a reusable template snippet to reduce duplication.
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

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

src/pretix/control/views/auth.py Show resolved Hide resolved
src/pretix/settings.py Show resolved Hide resolved
@mariobehling
Copy link
Member

The approach to providing access to the video component extends beyond the dashboard and requires a more comprehensive solution. A draft PR addressing this issue with a different method is available here: PR #376. Therefore, I am closing this PR in favor of that approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Eventay-Video Component to User-Dashboard
2 participants