Use ServeDocsPermissionsMixin to isolate this methods and be more clear #6408
Conversation
humitos
force-pushed
the
humitos/use-doc-permission-mixin
branch
from
3689847 to
60b121f
Compare
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
|
||
| :rtype: django.http.HttpResponse | ||
| """ | ||
| return self._serve_401(request, project) |
There was a problem hiding this comment.
I don't quite understand what this is meant to be doing. Are we using these additional methods, or are they just meant to be overwritten?
There was a problem hiding this comment.
We are overriding these methods from corporate. They are meant to be overwritten.
This PR just move them outside the main class and put them into a mixin with better docstrings. Which is easier to understand where they come from. That's all.
I could move the docstrings to the Base class, otherwise.
There was a problem hiding this comment.
@ericholscher see https://github.com/readthedocs/readthedocs.org/pull/6572/files#r371770894
I'm fine with either. I think defaulting to False and force re-define it, it's safer, though.
There was a problem hiding this comment.
Yea, it should default False 👍
There was a problem hiding this comment.
Looks good, likely need to be updated w/ the changes from #6572.
|
|
||
| :rtype: django.http.HttpResponse | ||
| """ | ||
| return self._serve_401(request, project) |
…humitos/use-doc-permission-mixin
…humitos/use-doc-permission-mixin
|
|
||
| :rtype: django.http.HttpResponse | ||
| """ | ||
| return self._serve_401(request, project) |
There was a problem hiding this comment.
This _serve_401 method doesn't exist on this class. It shouldn't be called if we aren't implementing it.
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Small refactor to make more clear where these method come from.