-
Notifications
You must be signed in to change notification settings - Fork 143
chore: added talk endpoint #1362
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
chore: added talk endpoint #1362
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds a new /talk endpoint under the main domain event routing that serves the event start page, ensuring event URLs include the organizer segment and /talk suffix, and wires it to the existing EventStartpage view. Sequence diagram for resolving the new event talk endpointsequenceDiagram
actor User
participant Browser
participant DjangoMainDomainURLconf
participant EventStartpage
User->>Browser: Enter URL /{orgname}/{eventslug}/talk
Browser->>DjangoMainDomainURLconf: HTTP GET /{orgname}/{eventslug}/talk
DjangoMainDomainURLconf->>DjangoMainDomainURLconf: Match re_path ^talk/?$
DjangoMainDomainURLconf->>EventStartpage: Dispatch request to as_view
EventStartpage-->>DjangoMainDomainURLconf: Rendered event start page response
DjangoMainDomainURLconf-->>Browser: HTTP 200 HTML
Browser-->>User: Display event talk start page
Flow diagram for Django routing to the new talk endpointflowchart LR
A["HTTP GET /{orgname}/{eventslug}/talk"] --> B[Django main domain URLconf]
B --> C[Locale and organizer specific patterns]
C --> D[Match re_path ^talk/?$ name event.talk]
D --> E[EventStartpage.as_view]
E --> F[Render event start page response]
F --> G[Return HTTP response to client]
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this 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 - here's some feedback:
- Consider using
path('talk/', EventStartpage.as_view(), ...)instead ofre_path(r'^talk/?$'...)for consistency with the rest of the URL patterns unless there is a specific need for regex here. - Double-check that mounting
EventStartpageat/talkpicks up the correct event context in the multidomain setup (org + event slug) and doesn’t rely on URL parts that are no longer present at this level.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider using `path('talk/', EventStartpage.as_view(), ...)` instead of `re_path(r'^talk/?$'...)` for consistency with the rest of the URL patterns unless there is a specific need for regex here.
- Double-check that mounting `EventStartpage` at `/talk` picks up the correct event context in the multidomain setup (org + event slug) and doesn’t rely on URL parts that are no longer present at this level.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Fixes this problem: Previously https://next.eventyay.com/{eventslug}/ should be -> https://next.eventyay.com/{orgname}/{eventslug}/talk
Summary by Sourcery
New Features: