-
-
Notifications
You must be signed in to change notification settings - Fork 340
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 when Session, Authentication or Message middlewares are not present #667
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #667 +/- ##
==========================================
+ Coverage 86.51% 87.07% +0.56%
==========================================
Files 52 52
Lines 2091 2113 +22
==========================================
+ Hits 1809 1840 +31
+ Misses 282 273 -9 ☔ View full report in Codecov by Sentry. |
Appears to resolve the issue on the ticket. |
silk/views/requests.py
Outdated
@@ -130,7 +130,7 @@ def _get_objects(self, show=None, order_by=None, order_dir=None, path=None, filt | |||
return query_set[:show] | |||
|
|||
def _create_context(self, request): | |||
raw_filters = request.session.get(self.session_key_request_filters, {}).copy() | |||
raw_filters = request.session.get(self.session_key_request_filters, {}).copy() if hasattr(request, 'session') else {} |
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.
Thanks for your work, it's a great beginning, but if I'm not mistaking, the filters are not working when selecting them. They should work if you fall back to request.GET or request.POST
here
silk/views/summary.py
Outdated
@@ -54,7 +54,7 @@ def _num_queries_by_view(self, filters): | |||
return sorted(requests, key=lambda item: item.t, reverse=True) | |||
|
|||
def _create_context(self, request): | |||
raw_filters = request.session.get(self.filters_key, {}) | |||
raw_filters = request.session.get(self.filters_key, {}) if hasattr(request, 'session') else {} |
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.
And also a fallback to request.GET or request.POST
here
014bbf8
to
753ced8
Compare
753ced8
to
8bb96b5
Compare
@SebCorbin First, thank you for your quick responses and the pacience 🙏 Regarding the review:
I followed the With that + the custom filter manager we can get filters from What do you think? Other question: would it be worth adding selenium tests? |
Hi @albertyw! Sorry to bother you, just wondering if there is something I can do to push forward the pull request. As I mentioned in my last comment, perhaps add selenium tests could give more confident to the change? Thank you! |
Hi! Sorry to bother you, but any progress on the review? We would really appreciate this feature, since it would allow us to use silk without the middleware needed. Thank you very much |
Hi @MattyCZ! This pull request was marked as part of the last release but never merged. I think we should wait. I'm available in case of changes required! |
Hi @albertyw! I just want to let you know that this PR was marked as part of the release |
Hello!
In this PR I try fix the issue #347
Doesn't fail when one of these middlewares are not present:
and if
SILKY_AUTHENTICATION
isTrue
, an exception is raised: SILKY_AUTHENTICATION can not be enabled without Session, Authentication or Message Django's middlewaresLet me know any change required!
Thank you!