#140 Clone redirects app #141
Conversation
|
@artemiy312 , this is single interesting commit for review. All other is about cloning and bunch renaming |
There was a problem hiding this comment.
@duker33 , As I understood you inherited a whole redirect django app to override only one middleware's method, itsn't it? If it's so, why did not you inherit only related middleware class and override the needed method.
redirects/admin.py
Outdated
| @@ -0,0 +1,10 @@ | |||
| from django.contrib import admin | |||
| from redirects.models import Redirect | |||
There was a problem hiding this comment.
Let's add one line before project imports
generic_admin/mixins.py
Outdated
| @@ -1,5 +1,5 @@ | |||
| from django.contrib import admin | |||
| from django.contrib.redirects.models import Redirect | |||
| from redirects.models import Redirect | |||
There was a problem hiding this comment.
An project import should be separate
@artemiy312 , oops ... You are right of course. We can inject cusom Middleware, i missed this django feature. I'll fix this PR |
duker33
added
the
refine
duker33
force-pushed
the
140_clone_redirects_app
branch
from
42455ee to
2a4d664
Compare
There was a problem hiding this comment.
@duker33 , please, pay attention on the comment about a name of middleware class
refarm_redirects/middleware.py
Outdated
| from django.contrib.sites.shortcuts import get_current_site | ||
|
|
||
|
|
||
| class RedirectFallbackMiddleware(DjangoRedirectFallbackMiddleware): |
There was a problem hiding this comment.
Fallback is the wrong part of the class name here, because the PR removes that fallback-like logic in favor of precondition check an existence of redirects
There was a problem hiding this comment.
@artemiy312 , i'll change it to RedirectAllMiddleware, y
TO-DO
Closes #140