Skip to content

Conversation

@Saksham-Sirohi
Copy link
Contributor

@Saksham-Sirohi Saksham-Sirohi commented Nov 21, 2025

Fixes #1291 and solves 500 errors on sessions and review page

Summary by Sourcery

Improve UI and robustness of the featured session toggle and resolve server errors on session and review pages

Bug Fixes:

  • Fix 500 errors by sourcing available content locales from settings.LANGUAGES and merging in LANGUAGE_NAMES
  • Correct the CSRF cookie key from pretalx_csrftoken to eventyay_csrftoken

Enhancements:

  • Refine styling and centering for the featured session checkbox in CSS and HTML templates
  • Include dynamic data-url attribute in the featured toggle input and update JS to use it or fallback to a constructed path
  • Use the correct CSRF token name (eventyay_csrftoken) for toggle requests

@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Nov 21, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

This PR enhances the featured sessions UI and fixes server errors by updating CSS and templates for proper alignment and URL handling, refining locale lookups in the Event model, and correcting JS request construction and CSRF header naming.

Sequence diagram for toggling featured status with improved URL and CSRF handling

sequenceDiagram
    actor User
    participant "Session List Page"
    participant "JavaScript (main.js)"
    participant "Backend Server"
    User->>"Session List Page": Loads page
    "Session List Page"->>"JavaScript (main.js)": Renders featured checkbox with data-url and correct CSRF token
    User->>"JavaScript (main.js)": Clicks featured checkbox
    "JavaScript (main.js)"->>"Backend Server": POST to toggle_featured (using data-url, eventyay_csrftoken)
    Backend Server-->>"JavaScript (main.js)": Returns status
    "JavaScript (main.js)"->>User: Updates UI status
Loading

Class diagram for Event model locale lookup changes

classDiagram
    class Event {
        +available_content_locales() list
        +named_content_locales() list
        +named_plugin_locales() list
    }
    class settings {
        LANGUAGES
    }
    class LANGUAGE_NAMES
    Event --> settings : uses LANGUAGES
    Event --> LANGUAGE_NAMES : uses LANGUAGE_NAMES in named_content_locales
Loading

File-Level Changes

Change Details Files
Refactor featured session styling in CSS
  • Add vertical-align: middle to .submission_featured
  • Define .form-check as inline-block with zero margin
  • Center the icon vertically using top: 50% and transform
  • Reset margin on checkboxes
app/eventyay/static/orga/css/_layout.css
Align featured toggle in templates and supply toggle URL
  • Add text-center class to header and cell
  • Use d-inline-block instead of mt-1 on form-check container
  • Remove default margins from form elements
  • Introduce data-url attribute for toggle action
app/eventyay/orga/templates/orga/submission/list.html
Improve locale lookup in Event model
  • Switch to settings.LANGUAGES for available_content_locales
  • Merge LANGUAGE_NAMES into locale_names
  • Safely map content_locales with fallback values
app/eventyay/base/models/event.py
Fix featured toggle requests in JS
  • Prefer dataset.url over constructed path
  • Handle optional trailing slash when building URL
  • Update CSRF header key to eventyay_csrftoken
app/eventyay/static/orga/js/main.js

Assessment against linked issues

Issue Objective Addressed Explanation
#1291 Fix the 'Feature session' option under Sessions so that it functions correctly (i.e., allows users to feature/unfeature sessions as intended).

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Contributor

@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 there - I've reviewed your changes and they look great!

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `app/eventyay/static/orga/css/_layout.css:59-67` </location>
<code_context>
 .submission_featured {
   position: relative;
   text-align: center;
+  vertical-align: middle;
+
+  .form-check {
</code_context>

<issue_to_address>
**suggestion:** Consider if vertical-align is effective for block-level elements.

'vertical-align: middle;' does not work as expected on divs. For vertical centering, consider using flexbox or grid instead.

```suggestion
.submission_featured {
  position: relative;
  display: flex;
  align-items: center;
  justify-content: center;
  text-align: center;

  .form-check {
    display: inline-block;
    margin: 0;
  }
```
</issue_to_address>

### Comment 2
<location> `app/eventyay/base/models/event.py:2118` </location>
<code_context>
    @cached_property
    def named_content_locales(self) -> list:
        locale_names = dict(self.available_content_locales)
        # locale_names['en-us'] = locale_names['en']
        locale_names.update(LANGUAGE_NAMES)
        return [(code, locale_names.get(code, code)) for code in self.content_locales]

</code_context>

<issue_to_address>
**suggestion (code-quality):** Merge dictionary updates via the union operator ([`dict-assign-update-to-union`](https://docs.sourcery.ai/Reference/Default-Rules/suggestions/dict-assign-update-to-union/))

```suggestion
        locale_names |= LANGUAGE_NAMES
```
</issue_to_address>

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 and I'll use the feedback to improve your reviews.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes featured session functionality and resolves 500 errors occurring on session and review pages by correcting CSRF token references and improving locale handling.

Key Changes:

  • Updated CSRF token cookie name from pretalx_csrftoken to eventyay_csrftoken in JavaScript
  • Enhanced featured session toggle UI with improved centering and styling
  • Modified locale resolution logic to prevent 500 errors when content locales are not found

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
app/eventyay/static/orga/js/main.js Updates CSRF token name and adds support for data-url attribute with fallback URL construction for featured toggle requests
app/eventyay/static/orga/css/_layout.css Improves featured checkbox alignment with vertical centering and inline-flex layout
app/eventyay/orga/templates/orga/submission/list.html Adds data-url attribute to featured toggle input and centers the featured column header
app/eventyay/base/models/event.py Changes locale source from default_django_settings.LANGUAGES to settings.LANGUAGES and adds fallback for missing locale names

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

locale_names = dict(self.available_content_locales)
# locale_names['en-us'] = locale_names['en']
return [(code, locale_names[code]) for code in self.content_locales]
locale_names |= LANGUAGE_NAMES
Copy link

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

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

The merge operation locale_names |= LANGUAGE_NAMES is problematic because:

  1. locale_names is created from dict(self.available_content_locales) (line 2157)
  2. available_content_locales already uses dict(settings.LANGUAGES) (line 2151)
  3. LANGUAGE_NAMES is redefined at line 74 as {code: name for code, name in settings.LANGUAGES}

This means you're merging settings.LANGUAGES back into a dict that was already built from settings.LANGUAGES, making this operation redundant.

The intended behavior appears to be merging the imported LANGUAGE_NAMES from eventyay.common.language (which contains additional language names from settings.LANGUAGES_INFORMATION), but the redefinition at line 74 overwrites that import.

Suggestion: Remove the redefinition at line 74 to use the imported LANGUAGE_NAMES, which contains the enhanced language information from settings.LANGUAGES_INFORMATION.

Copilot uses AI. Check for mistakes.
Copy link
Member

@mariobehling mariobehling left a comment

Choose a reason for hiding this comment

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

Please check AI comments.

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.

Talk: Feature Session Option Does not Work

2 participants