-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
fix: add a nice sharing 404 #28065
fix: add a nice sharing 404 #28065
Conversation
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.
PR Summary
This PR enhances the user experience by replacing the default DRF API error page with a custom 404 page for invalid or expired sharing URLs.
- Added new
shared_resource_404.html
template with a space-themed design and PostHog branding - Modified
SharingViewerPageViewSet
inposthog/api/sharing.py
to handle NotFound exceptions with custom 404 response - Improved error handling for shared resources that don't exist or are no longer accessible
2 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile
def custom_404_response(request): | ||
"""Returns a custom 404 page.""" | ||
return render(request, "shared_resource_404.html", status=404) |
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.
style: consider adding request type annotation and docstring with more details about the template context
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.
LGTM, but I think we could load a font to make it slightly nicer
{% include "head.html" %} | ||
</head> | ||
<style> | ||
html, body, iframe { |
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.
It would be cool if you could load our font using @font
, will look more professional compared to the Times New Roman thing we have going on
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.
I'm kinda desgner myself
if someone visits a sharing URL that doesn't exist or doesn't exist any more
they get a DRF API style error page, yuck!