Fixes 404 and url rename #1235
Open
Fixes 404 and url rename #1235
+39
−34
Conversation
This file contains hidden or 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
Contributor
Reviewer's GuideRenames speaker/talk URL paths to pluralized and session-focused routes, updates all dependent URLs and templates, and introduces a scoped template tag to gate the online video link to users with paid orders while aligning pretalx and schedule editor URLs with the new structure. Sequence diagram for user_has_paid_ticket template tag gating online video linksequenceDiagram
participant T as DjangoTemplate
participant Tag as user_has_paid_ticket
participant Req as HttpRequest
participant Event as Event
participant Scope as django_scopes.scope
participant Order as OrderModel
participant DB as Database
T->>Tag: call user_has_paid_ticket(context)
Tag->>Req: get request from context
Tag->>Req: check request.event and request.user.is_authenticated
alt missing_event_or_user
Tag-->>T: return False
else has_event_and_user
Tag->>Event: get organizer from request.event
alt no_organizer
Tag-->>T: return False
else has_organizer
Tag->>Scope: enter scope(organizer)
activate Scope
Tag->>Order: objects.filter(event=request.event, email__iexact=request.user.email, status=STATUS_PAID)
Order->>DB: query paid orders for event and email
DB-->>Order: result set
Order-->>Tag: exists() boolean
Tag->>Scope: exit scope
deactivate Scope
Tag-->>T: return exists
end
end
T->>T: if has_ticket is True, render Join online video link
Class diagram for updated event and profile URL helpersclassDiagram
class EventUrls {
}
class Event {
+urls urls
+talk_schedule_url
+talk_session_url
+talk_speaker_url
}
class Event_urls {
+base
+schedule
+schedule_nojs
+featured
+talks
+speakers
+changelog
+feed
+export
+changelog_feed
+version
+version_feed
+version_frab
+version_frab_xcal
+version_frab_ical
+version_guidebook
+review
}
class Profile {
+urls urls
}
class Profile_urls {
+public
+social_image
+talks_ical
}
Event_urls --|> EventUrls
Profile_urls --|> EventUrls
Event *-- Event_urls : urls
Profile *-- Profile_urls : urls
Event : talk_schedule_url returns self.urls.schedule.full
Event : talk_session_url returns self.urls.talks.full
Event : talk_speaker_url returns self.urls.speakers.full
Event_urls : talks = "{base}sessions/"
Event_urls : speakers = "{base}speakers/"
Profile_urls : public = "{self.event.urls.base}speakers/{self.user.code}/"
File-Level Changes
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}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The non-live schedule checks in your SpeakerList, SpeakerView, and ScheduleView dispatch methods are identical—extract them into a shared mixin to reduce duplication and ease future updates.
- Relying on substring matching of request.path_info in templates for active tabs is brittle; consider passing an explicit context flag or comparing against reversed URL names instead.
- The legacy redirect logic in multidomain/redirects.py manually splits and remaps path segments—refactor to use a configurable mapping and Django's reverse() to keep it in sync with your URL patterns.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The non-live schedule checks in your SpeakerList, SpeakerView, and ScheduleView dispatch methods are identical—extract them into a shared mixin to reduce duplication and ease future updates.
- Relying on substring matching of request.path_info in templates for active tabs is brittle; consider passing an explicit context flag or comparing against reversed URL names instead.
- The legacy redirect logic in multidomain/redirects.py manually splits and remaps path segments—refactor to use a configurable mapping and Django's reverse() to keep it in sync with your URL patterns.
## Individual Comments
### Comment 1
<location> `app/eventyay/agenda/templates/agenda/base.html:42-43` </location>
<code_context>
+ {% endif %}
{% endif %}
{% if schedule or request.event.current_schedule %}
- <a href="{{ request.event.urls.speakers }}" class="header-tab {% if "/speaker/" in request.path_info %} active{% endif %}">
- <i class="fa fa-group"></i> {{ phrases.schedule.speakers }}
- </a>
+ {% with request.event.speakers.all|length as speaker_count %}
+ {% if speaker_count %}
+ <a href="{{ request.event.urls.speakers }}" class="header-tab {% if "/speakers/" in request.path_info %} active{% endif %}">
+ <i class="fa fa-group"></i> {{ phrases.schedule.speakers }}
+ </a>
</code_context>
<issue_to_address>
**suggestion:** The active tab logic may not cover legacy URLs.
Since legacy URLs ('/speaker/') are still accessible via redirects, please update the check to include both '/speakers/' and '/speaker/' to ensure the tab is highlighted correctly for all users.
```suggestion
<a href="{{ request.event.urls.speakers }}" class="header-tab {% if "/speakers/" in request.path_info or "/speaker/" in request.path_info %} active{% endif %}">
<i class="fa fa-group"></i> {{ phrases.schedule.speakers }}
</a>
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
|
Please make sure that links to schedules, sessions, CfS or speaker profiles are not visible if the talk component is not live or if they are not yet active, e.g. because the schedule is not yet released. |
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR renames URL patterns from "talk" to "sessions" and "speaker" to "speakers" across the codebase, adds backward-compatible redirects for legacy URLs, and implements error handling to prevent 404 errors when schedules or speaker data are unavailable.
- URL patterns updated from
talk/tosessions/andspeaker/tospeakers/ - Legacy URL redirects added for backward compatibility
- Error handling added to redirect users when schedule/speaker data is not available
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| app/eventyay/webapp/src/views/schedule/schedule-components/Session.vue | Updated session link URL generation from talk/ to sessions/ |
| app/eventyay/static/agenda/js/pretalx-schedule.min.js | Updated minified JavaScript to use sessions/ path instead of talk/ |
| app/eventyay/multidomain/redirects.py | Extended legacy redirect function to map old talk/ and speaker/ paths to new sessions/ and speakers/ paths |
| app/eventyay/base/models/submission.py | Updated submission URL properties to use sessions/ instead of talk/ |
| app/eventyay/base/models/profile.py | Updated speaker profile URL to use speakers/ instead of speaker/ |
| app/eventyay/base/models/event.py | Updated event URL properties and fixed help_text string quoting |
| app/eventyay/agenda/views/speaker.py | Added dispatch checks to redirect when schedule/speakers are unavailable |
| app/eventyay/agenda/views/schedule.py | Added null check for schedule object and updated path detection for sessions list view |
| app/eventyay/agenda/urls.py | Updated all URL patterns to use new paths and added RedirectView entries for legacy paths |
| app/eventyay/agenda/templates/agenda/base.html | Updated header tab path detection and added visibility checks based on data availability |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mariobehling
requested changes
Nov 14, 2025
Member
mariobehling
left a comment
mariobehling
left a comment
There was a problem hiding this comment.
You lost track of what are you doing in regards to the /talk and /cfp page. Instead of going back where you started you are considering your last PR as the start point.
Check this: I am also confused. I think you renamed the /talk URL to /cfp. Instead the page should stay as is.
In the issue it was written
old URL: https://next.eventyay.com/{eventslug}/
new URL: https://next.eventyay.com/{orgname}/{eventslug}/talk
Please go back to the original before your PRs and check this out.
Where is the original cfp page?
e.g. https://wikimedia.eventyay.com/talk/wikiindaba-2025/cfp
As described in the issue it should go to {orgname}/{eventslug}/cfp
Saksham-Sirohi
force-pushed
the
404-urls-fix
branch
from
dfc5391 to
c61d42a
Compare
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- In
frontend/schedule-editor/src/api.ts,saveTalknow relies onthis.eventSlugandbasePathrather than deriving the URL fromwindow.location; ifsaveTalkis ever called with a lostthiscontext (e.g. passed as a callback),this.eventSlugwill be undefined, so consider avoidingthis-binding (e.g. passeventSlugexplicitly) or ensure all call sites preserve the object context. - The URL renames from
/talk/→/sessions/and/speaker/→/speakers/inagenda/urls.pyand the correspondingEvent.urlsproperties remove the old paths entirely; if existing external links or bookmarks use the old URLs, consider adding explicit redirect routes to preserve backward compatibility. - The updated
pretalxScheduleUrlgetter inwebapp/src/store/schedule.jsnow requirespretalx.organizer; if this field is not set in all environments, the schedule URL will silently become an empty string, so it may be safer to fall back to the old{event}/...path whenorganizeris missing or validate/populateorganizerwhereverworld.pretalxis constructed.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `frontend/schedule-editor/src/api.ts`, `saveTalk` now relies on `this.eventSlug` and `basePath` rather than deriving the URL from `window.location`; if `saveTalk` is ever called with a lost `this` context (e.g. passed as a callback), `this.eventSlug` will be undefined, so consider avoiding `this`-binding (e.g. pass `eventSlug` explicitly) or ensure all call sites preserve the object context.
- The URL renames from `/talk/`→`/sessions/` and `/speaker/`→`/speakers/` in `agenda/urls.py` and the corresponding `Event.urls` properties remove the old paths entirely; if existing external links or bookmarks use the old URLs, consider adding explicit redirect routes to preserve backward compatibility.
- The updated `pretalxScheduleUrl` getter in `webapp/src/store/schedule.js` now requires `pretalx.organizer`; if this field is not set in all environments, the schedule URL will silently become an empty string, so it may be safer to fall back to the old `{event}/...` path when `organizer` is missing or validate/populate `organizer` wherever `world.pretalx` is constructed.
## Individual Comments
### Comment 1
<location> `app/eventyay/common/templatetags/event_tags.py:42-51` </location>
<code_context>
return f"{base}?{query.urlencode()}"
+
+
+@register.simple_tag(takes_context=True)
+def user_has_paid_ticket(context):
+ """Check if user has a paid order for the event."""
+ request = context.get('request')
+ if not request or not getattr(request, 'event', None) or not getattr(request.user, 'is_authenticated', False):
+ return False
+
+ organizer = getattr(request.event, 'organizer', None)
+ if not organizer:
+ return False
+
+ with scope(organizer=organizer):
+ return Order.objects.filter(
+ event=request.event, email__iexact=request.user.email, status=Order.STATUS_PAID
+ ).exists()
</code_context>
<issue_to_address>
**issue:** Using email to determine access to the online video may fail for users whose account email differs from their order email.
This query ties access to `request.user.email`, so users who purchased with a different email (e.g., forwarding/organizational addresses, or after changing their account email) will be denied. If possible, use a user–order relationship (e.g., a foreign key or related email model) instead of the current email string match to ensure all valid paid orders are recognized.
</issue_to_address>
### Comment 2
<location> `app/eventyay/webapp/src/store/schedule.js:22-31` </location>
<code_context>
return pretalx.url
}
- if (!pretalx.domain || !pretalx.event) return ''
+ if (!pretalx.domain || !pretalx.event || !pretalx.organizer) return ''
if (!pretalx.domain.endsWith('/')) {
pretalx.domain += '/'
}
- const url = new URL(`${pretalx.event}/schedule/widgets/schedule.json`, pretalx.domain)
+ // New unified URL structure: {orgname}/{event}/schedule/widgets/schedule.json
+ const path = `${pretalx.organizer}/${pretalx.event}/schedule/widgets/schedule.json`
+ const url = new URL(path, pretalx.domain)
return url.toString()
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The new URL construction depends on `pretalx.organizer` and still mutates `pretalx.domain` inside a getter.
Two concerns:
1. Returning `''` when `pretalx.organizer` is missing will break the schedule widget URL for any existing configs that don’t set `organizer`, whereas it previously worked.
2. This getter still mutates `pretalx.domain` by appending `'/'`. Vuex getters should be pure; this risks subtle issues (e.g. accumulating slashes, surprising state changes). Use a local `domain` variable with the trailing slash instead of modifying `pretalx.domain` directly.
```suggestion
if (pretalx.url) {
return pretalx.url
}
// Require domain and event; organizer is optional for backwards compatibility
if (!pretalx.domain || !pretalx.event) return ''
// Do not mutate state: work with a local domain variable
const domain = pretalx.domain.endsWith('/') ? pretalx.domain : pretalx.domain + '/'
// If organizer is present, use unified URL {organizer}/{event}/schedule/widgets/schedule.json
// Otherwise, fall back to the legacy structure {event}/schedule/widgets/schedule.json
const path = pretalx.organizer
? `${pretalx.organizer}/${pretalx.event}/schedule/widgets/schedule.json`
: `${pretalx.event}/schedule/widgets/schedule.json`
const url = new URL(path, domain)
```
</issue_to_address>
### Comment 3
<location> `app/eventyay/frontend/schedule-editor/src/api.ts:93-94` </location>
<code_context>
- const url = new URL(window.location.href);
- url.pathname = `${url.pathname}api/talks/${talk.id ? `${talk.id}/` : ''}`;
- url.search = window.location.search;
+ const talksBase = `${basePath}/orga/event/${this.eventSlug}/schedule/api/talks/`;
+ const url = talk.id ? `${talksBase}${talk.id}/` : talksBase;
let payload: HttpRequestBody = null;
</code_context>
<issue_to_address>
**question (bug_risk):** The new talks API URL no longer preserves query parameters and assumes a specific basePath layout.
Previously, the URL reused `window.location.search`, so query params (e.g. language, preview flags) were always forwarded to the talks API. The new `basePath` construction drops them and hard-codes `/orga/event/${this.eventSlug}/schedule/api/talks/`, which could break any backend logic that expects those params or relies on a specific deployment path (subpaths, reverse proxies, etc.). Please verify that no API consumers depend on these query parameters and that `basePath` is always aligned with the actual deployed URL structure.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Saksham-Sirohi
force-pushed
the
404-urls-fix
branch
from
f9e85de to
e602b64
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Fixes left of #1014
Summary by Sourcery
Update agenda and presale navigation, URLs, and integrations to use the new speakers/sessions structure and restrict online video access to paid attendees.
New Features:
Bug Fixes:
Enhancements: