-
Notifications
You must be signed in to change notification settings - Fork 57
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
Avoid using hardcoded localhost in Redirects #380
Avoid using hardcoded localhost in Redirects #380
Conversation
8eeb2ab
to
f642999
Compare
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.
A great start, though there's some choices that will cause issues that need addressing.
3356150
to
a003b5a
Compare
75707b2
to
c5feca2
Compare
grapple/types/redirects.py
Outdated
class Meta: | ||
name = "Redirect" |
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.
Can this be moved to just below the fields? I appreciate there isn't a standard way of doing that within this project (sometimes it's above the fields and sometimes it's below), though it's never at the end :).
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.
Addressed in c2a5365
@JakubMastalerz this is looking GREAT! As discussed earlier, can you review the performance of these changes to check we're not adversely affecting performance? In particular, I'm interested in the number of queries being made to ensure we're not causing any N+1 queries. |
6950794
to
12fd694
Compare
This PR changes
old_url
handling inRedirectType
to replace hardcoded URL generation with dynamically generated URLs based on wagtail'ssites
properties.Additionally:
Redirect
had no specified target.The changes were tested for performance and did not increase the number of queries when querying
Redirects
.