Feature/46 admin sessions #73
Conversation
Coperh
force-pushed
the
feature/46-admin-sessions
branch
2 times, most recently
from
2d0b8d0 to
8a65aee
Compare
Coperh
force-pushed
the
feature/46-admin-sessions
branch
from
8a65aee to
14afa0a
Compare
Coperh
force-pushed
the
feature/46-admin-sessions
branch
from
14afa0a to
bf1f086
Compare
Coperh
force-pushed
the
feature/46-admin-sessions
branch
from
bf1f086 to
5565720
Compare
| list_display = ["session_key", "user", "exists"] | ||
|
|
||
| @property | ||
| def SessionStore(self): |
There was a problem hiding this comment.
I originally set this in the _init_ but that is not called every time I ran a test and the cached version always used the database backend.
Is there a way to fix this?
There was a problem hiding this comment.
Is this caused by assigning the setting here?
There was a problem hiding this comment.
Probably because the pytest fixtures change the SESSION_ENGINE after the __init__ is done, I'm not sure if there is way around this tbh
There was a problem hiding this comment.
You can just use override_settings and it works as usual :/
| admin_user2 = admin_user | ||
|
|
||
|
|
||
| def test_only_session_profile_of_user_shown( |
There was a problem hiding this comment.
It might still be possible to see the detail or delete view for another users session
There was a problem hiding this comment.
I checked this and it is not possible to go to the detail view for other user's sessions, not sure about deleting though. Can you add tests for this?
There was a problem hiding this comment.
Is that intended behavior? Either way would be nice to have tests that verify this (whether it is intended or not).
There was a problem hiding this comment.
It is intended on my end that you can only see you're own sessions. IDK if this is the correct message, do we want users to know other sessions exist?
I do not think it is in scope that a super admin user can see all sessions.
There was a problem hiding this comment.
I think the current message is fine, because it does not give users extra information about other user's session. Just adding tests to verify that other users cant access/delete other session is enough in my opinion
There was a problem hiding this comment.
This still needs an assertion that deleting other users sessions is not possible, but might be nicer to move that to a separate test
|
Still has the issue where it saves the SessionProfile when sending in the request to delete |
| admin_user2 = admin_user | ||
|
|
||
|
|
||
| def test_only_session_profile_of_user_shown( |
There was a problem hiding this comment.
I checked this and it is not possible to go to the detail view for other user's sessions, not sure about deleting though. Can you add tests for this?
| list_display = ["session_key", "user", "exists"] | ||
|
|
||
| @property | ||
| def SessionStore(self): |
There was a problem hiding this comment.
Probably because the pytest fixtures change the SESSION_ENGINE after the __init__ is done, I'm not sure if there is way around this tbh
| list_display = ["session_key", "user", "exists"] | ||
|
|
||
| @property | ||
| def SessionStore(self): |
There was a problem hiding this comment.
Is this caused by assigning the setting here?
| admin_user2 = admin_user | ||
|
|
||
|
|
||
| def test_only_session_profile_of_user_shown( |
There was a problem hiding this comment.
Is that intended behavior? Either way would be nice to have tests that verify this (whether it is intended or not).
tests/conftest.py
Outdated
|
|
||
| @pytest.fixture | ||
| def cache_session_store(settings): | ||
| settings.SESSION_ENGINE = "django.contrib.sessions.backends.cache" |
There was a problem hiding this comment.
Are these settings set directly because they are ran outside of a testcase provided by django (e.g django.test.TestCase)?
There was a problem hiding this comment.
The db session is not really necessary because its set in the settings.
I've removed both fixtures and replaced them with cache override in the cached version
Coperh
force-pushed
the
feature/46-admin-sessions
branch
2 times, most recently
from
1ac629a to
25f3e5d
Compare
| admin_user2 = admin_user | ||
|
|
||
|
|
||
| def test_only_session_profile_of_user_shown( |
There was a problem hiding this comment.
This still needs an assertion that deleting other users sessions is not possible, but might be nicer to move that to a separate test
Coperh
force-pushed
the
feature/46-admin-sessions
branch
from
25f3e5d to
b631bbf
Compare
Fixes #46
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaah