Merge pull request #27233 from edx/feanil/flag_management

feat!: Replace logging WaffleSwitch with a django settinge.
This commit is contained in:
Feanil Patel
2021-04-02 15:12:11 -04:00
committed by GitHub
4 changed files with 16 additions and 20 deletions

View File

@@ -387,7 +387,7 @@ class CourseListSearchViewTest(CourseApiTestViewMixin, ModuleStoreTestCase, Sear
self.setup_user(self.audit_user)
# These query counts were found empirically
query_counts = [55, 46, 46, 46, 46, 46, 46, 46, 46, 46, 16]
query_counts = [54, 46, 46, 46, 46, 46, 46, 46, 46, 46, 16]
ordered_course_ids = sorted([str(cid) for cid in (course_ids + [c.id for c in self.courses])])
self.clear_caches()

View File

@@ -76,21 +76,19 @@ from django.utils.crypto import get_random_string
from django.utils.deprecation import MiddlewareMixin
from django.utils.encoding import python_2_unicode_compatible
from edx_django_utils.monitoring import set_custom_attribute
from edx_toggles.toggles import WaffleSwitch
from openedx.core.lib.mobile_utils import is_request_from_mobile_app
# .. toggle_name: safe_session.log_request_user_changes
# .. toggle_implementation: WaffleSwitch
# .. toggle_name: LOG_REQUEST_USER_CHANGES
# .. toggle_implementation: SettingToggle
# .. toggle_default: False
# .. toggle_description: Turn this toggle on to log anytime the `user` attribute of the request object gets
# changed. This will also log the location where the change is coming from to quickly find issues.
# .. toggle_warnings: This logging will be very verbose and so should probably not be left on all the time.
# .. toggle_use_cases: temporary
# .. toggle_use_cases: opt_in
# .. toggle_creation_date: 2021-03-25
# .. toggle_target_removal_date: 2021-05-01
# .. toggle_tickets: https://openedx.atlassian.net/browse/ARCHBOM-1718
LOG_REQUEST_USER_CHANGES_FLAG = WaffleSwitch('safe_session.log_request_user_changes', __name__)
LOG_REQUEST_USER_CHANGES = getattr(settings, 'LOG_REQUEST_USER_CHANGES', False)
log = getLogger(__name__)
@@ -308,7 +306,7 @@ class SafeSessionMiddleware(SessionMiddleware, MiddlewareMixin):
user_id = self.get_user_id_from_session(request)
if safe_cookie_data.verify(user_id): # Step 4
request.safe_cookie_verified_user_id = user_id # Step 5
if LOG_REQUEST_USER_CHANGES_FLAG.is_enabled():
if LOG_REQUEST_USER_CHANGES:
log_request_user_changes(request)
else:
return self._on_user_authentication_failed(request)

View File

@@ -71,10 +71,9 @@ class TestSafeSessionProcessRequest(TestSafeSessionsLogMixin, TestCase):
"""
assert SafeSessionMiddleware.get_user_id_from_session(self.request) == self.user.id
@patch("openedx.core.djangoapps.safe_sessions.middleware.LOG_REQUEST_USER_CHANGES", False)
@patch("openedx.core.djangoapps.safe_sessions.middleware.log_request_user_changes")
@patch("openedx.core.djangoapps.safe_sessions.middleware.LOG_REQUEST_USER_CHANGES_FLAG")
def test_success(self, mock_log_request_user_changes_flag, mock_log_request_user_changes):
mock_log_request_user_changes_flag.is_enabled.return_value = False
def test_success(self, mock_log_request_user_changes):
self.client.login(username=self.user.username, password='test')
session_id = self.client.session.session_key
safe_cookie_data = SafeCookieData.create(session_id, self.user.id)
@@ -99,10 +98,9 @@ class TestSafeSessionProcessRequest(TestSafeSessionsLogMixin, TestCase):
# verify extra request_user_logging not called.
assert not mock_log_request_user_changes.called
@patch("openedx.core.djangoapps.safe_sessions.middleware.LOG_REQUEST_USER_CHANGES", True)
@patch("openedx.core.djangoapps.safe_sessions.middleware.log_request_user_changes")
@patch("openedx.core.djangoapps.safe_sessions.middleware.LOG_REQUEST_USER_CHANGES_FLAG")
def test_log_request_user_flag_on(self, mock_log_request_user_changes_flag, mock_log_request_user_changes):
mock_log_request_user_changes_flag.is_enabled.return_value = True
def test_log_request_user_on(self, mock_log_request_user_changes):
self.client.login(username=self.user.username, password='test')
session_id = self.client.session.session_key
safe_cookie_data = SafeCookieData.create(session_id, self.user.id)

View File

@@ -171,7 +171,7 @@ class TestOwnUsernameAPI(CacheIsolationTestCase, UserAPITestCase):
Test that a client (logged in) can get her own username.
"""
self.client.login(username=self.user.username, password=TEST_PASSWORD)
self._verify_get_own_username(18)
self._verify_get_own_username(17)
def test_get_username_inactive(self):
"""
@@ -181,7 +181,7 @@ class TestOwnUsernameAPI(CacheIsolationTestCase, UserAPITestCase):
self.client.login(username=self.user.username, password=TEST_PASSWORD)
self.user.is_active = False
self.user.save()
self._verify_get_own_username(18)
self._verify_get_own_username(17)
def test_get_username_not_logged_in(self):
"""
@@ -355,7 +355,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
"""
self.different_client.login(username=self.different_user.username, password=TEST_PASSWORD)
self.create_mock_profile(self.user)
with self.assertNumQueries(25):
with self.assertNumQueries(24):
response = self.send_get(self.different_client)
self._verify_full_shareable_account_response(response, account_privacy=ALL_USERS_VISIBILITY)
@@ -370,7 +370,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
"""
self.different_client.login(username=self.different_user.username, password=TEST_PASSWORD)
self.create_mock_profile(self.user)
with self.assertNumQueries(25):
with self.assertNumQueries(24):
response = self.send_get(self.different_client)
self._verify_private_account_response(response)
@@ -517,7 +517,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
assert data['accomplishments_shared'] is False
self.client.login(username=self.user.username, password=TEST_PASSWORD)
verify_get_own_information(23)
verify_get_own_information(22)
# Now make sure that the user can get the same information, even if not active
self.user.is_active = False
@@ -537,7 +537,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
legacy_profile.save()
self.client.login(username=self.user.username, password=TEST_PASSWORD)
with self.assertNumQueries(23):
with self.assertNumQueries(22):
response = self.send_get(self.client)
for empty_field in ("level_of_education", "gender", "country", "state", "bio",):
assert response.data[empty_field] is None