-
Notifications
You must be signed in to change notification settings - Fork 135
Fix Speaker Acceptance Confirmation Link Returning 500 Error #1300
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
base: enext
Are you sure you want to change the base?
Fix Speaker Acceptance Confirmation Link Returning 500 Error #1300
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR patches the SubmissionConfirmView.dispatch method to manually initialize the view’s request context (request, args, kwargs) before accessing the cached submission, preventing an AttributeError and subsequent 500 error when loading the confirmation link. Sequence diagram for speaker submission confirmation link accesssequenceDiagram
actor User
participant "SubmissionConfirmView"
participant "get_login_redirect()"
participant "super().dispatch()"
User->>"SubmissionConfirmView": GET /event/me/submissions/CODE/confirm
alt User is anonymous
"SubmissionConfirmView"->>"get_login_redirect()": Redirect to login
"get_login_redirect()"-->>User: 302 Found (redirect to login)
else User is authenticated
"SubmissionConfirmView"->>"SubmissionConfirmView": Initialize request, args, kwargs
"SubmissionConfirmView"->>"SubmissionConfirmView": Check user permission on submission
alt User lacks permission
"SubmissionConfirmView"->>"SubmissionConfirmView": Set error template
end
"SubmissionConfirmView"->>"super().dispatch()": Continue dispatch
end
File-Level Changes
Assessment against linked issues
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:
- Replace manual assignment of self.request, self.args, and self.kwargs with a call to self.setup(request, *args, **kwargs) at the start of dispatch to align with Django CBV conventions.
- Consider using Django's built-in LoginRequiredMixin and PermissionRequiredMixin for cleaner authentication and authorization handling instead of custom checks in dispatch.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Replace manual assignment of self.request, self.args, and self.kwargs with a call to self.setup(request, *args, **kwargs) at the start of dispatch to align with Django CBV conventions.
- Consider using Django's built-in LoginRequiredMixin and PermissionRequiredMixin for cleaner authentication and authorization handling instead of custom checks in dispatch.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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.
Pull Request Overview
This PR fixes a bug where the speaker acceptance confirmation link was returning a 500 error due to an AttributeError. The issue occurred because SubmissionConfirmView.dispatch() tried to access self.submission (a cached property that depends on self.request and self.kwargs) before these attributes were initialized by the parent class's dispatch() method.
The fix manually initializes self.request, self.args, and self.kwargs before accessing self.submission, preventing the AttributeError.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fix Speaker Acceptance Confirmation Link Returning 500 Error
The
SubmissionConfirmView.dispatch()method tried to accessself.submission(a cached property) before callingsuper().dispatch(), which caused anAttributeErrorbecauseself.requestandself.kwargsweren't initialized yet.Modified
dispatch()to manually initializeself.request,self.args, andself.kwargsbefore accessingself.submission:Fixes #1276
Summary by Sourcery
Bug Fixes: