Skip to content

Commit 44ac4a0

Browse files
authored
[ENG-8414] Fix WB logging payloads (#11244)
1 parent ba195ee commit 44ac4a0

3 files changed

Lines changed: 117 additions & 1 deletion

File tree

osf/models/mixins.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,8 @@ def get_addon(self, name, is_deleted=False, auth=None):
558558
# Avoid test-breakages by avoiding early access to the request context
559559
if name not in self.OSF_HOSTED_ADDONS:
560560
request, user_id = get_request_and_user_id()
561+
if not user_id and auth and auth.user:
562+
user_id = auth.user._id
561563
if flag_is_active(request, features.ENABLE_GV):
562564
return self._get_addon_from_gv(gv_pk=name, requesting_user_id=user_id, auth=auth)
563565

osf/models/node.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2378,7 +2378,7 @@ def serialize_waterbutler_settings(self, provider_name):
23782378
def create_waterbutler_log(self, auth, action, payload):
23792379
try:
23802380
metadata = payload['metadata']
2381-
node_addon = self.get_addon(payload['provider'])
2381+
node_addon = self.get_addon(payload['provider'], auth=auth)
23822382
except KeyError:
23832383
raise HTTPError(http_status.HTTP_400_BAD_REQUEST)
23842384

osf_tests/test_gv_session_optimization.py

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import pytest
22
from unittest.mock import patch, MagicMock, PropertyMock
33

4+
from waffle.testutils import override_flag
45
from osf.external.gravy_valet import translations as gv_translations
56
from osf.external.gravy_valet import request_helpers as gv_requests
67
from osf_tests.factories import ProjectFactory, UserFactory
@@ -354,6 +355,119 @@ def test_integration_get_addon_skips_gv_for_osf_hosted_addons(
354355

355356
mock_get_addon_from_gv.assert_not_called()
356357

358+
@patch('osf.models.mixins.get_request_and_user_id')
359+
@patch('osf.models.node.AbstractNode._get_addon_from_gv')
360+
def test_get_addon_uses_auth_user_id_when_request_user_id_is_none(
361+
self, mock_get_addon_from_gv, mock_get_request_user,
362+
node, user
363+
):
364+
with override_flag(features.ENABLE_GV, active=True):
365+
mock_request = MagicMock()
366+
mock_get_request_user.return_value = (mock_request, None)
367+
368+
auth = Auth(user=user)
369+
370+
mock_addon = MagicMock()
371+
mock_addon.short_name = 'github'
372+
mock_get_addon_from_gv.return_value = mock_addon
373+
374+
result = node.get_addon('github', auth=auth)
375+
376+
mock_get_addon_from_gv.assert_called_once_with(
377+
gv_pk='github',
378+
requesting_user_id=user._id,
379+
auth=auth
380+
)
381+
assert result == mock_addon
382+
383+
@patch('osf.models.mixins.get_request_and_user_id')
384+
@patch('osf.models.node.AbstractNode._get_addon_from_gv')
385+
def test_get_addon_uses_request_user_id_when_available(
386+
self, mock_get_addon_from_gv, mock_get_request_user,
387+
node, user
388+
):
389+
with override_flag(features.ENABLE_GV, active=True):
390+
mock_request = MagicMock()
391+
request_user_id = 'request_user_123'
392+
mock_get_request_user.return_value = (mock_request, request_user_id)
393+
394+
auth = Auth(user=user)
395+
396+
mock_addon = MagicMock()
397+
mock_addon.short_name = 'github'
398+
mock_get_addon_from_gv.return_value = mock_addon
399+
400+
result = node.get_addon('github', auth=auth)
401+
402+
mock_get_addon_from_gv.assert_called_once_with(
403+
gv_pk='github',
404+
requesting_user_id=request_user_id,
405+
auth=auth
406+
)
407+
assert result == mock_addon
408+
409+
@patch('osf.models.mixins.get_request_and_user_id')
410+
@patch('osf.models.node.AbstractNode._get_addon_from_gv')
411+
def test_get_addon_handles_none_auth_gracefully(
412+
self, mock_get_addon_from_gv, mock_get_request_user,
413+
node, user
414+
):
415+
with override_flag(features.ENABLE_GV, active=True):
416+
mock_request = MagicMock()
417+
mock_get_request_user.return_value = (mock_request, None)
418+
419+
mock_addon = MagicMock()
420+
mock_addon.short_name = 'github'
421+
mock_get_addon_from_gv.return_value = mock_addon
422+
423+
result = node.get_addon('github', auth=None)
424+
425+
mock_get_addon_from_gv.assert_called_once_with(
426+
gv_pk='github',
427+
requesting_user_id=None,
428+
auth=None
429+
)
430+
assert result == mock_addon
431+
432+
@patch('osf.models.mixins.get_request_and_user_id')
433+
def test_get_addon_skips_gv_for_osf_hosted_addons_with_auth(
434+
self, mock_get_request_user, node, user
435+
):
436+
with override_flag(features.ENABLE_GV, active=True):
437+
mock_request = MagicMock()
438+
mock_get_request_user.return_value = (mock_request, None)
439+
440+
auth = Auth(user=user)
441+
442+
node.get_addon('osfstorage', auth=auth)
443+
mock_get_request_user.assert_not_called()
444+
445+
@patch('osf.models.mixins.get_request_and_user_id')
446+
@patch('osf.models.node.AbstractNode._get_addon_from_gv')
447+
def test_get_addon_with_auth_user_none_falls_back_to_request_user_id(
448+
self, mock_get_addon_from_gv, mock_get_request_user,
449+
node, user
450+
):
451+
with override_flag(features.ENABLE_GV, active=True):
452+
mock_request = MagicMock()
453+
request_user_id = 'request_user_123'
454+
mock_get_request_user.return_value = (mock_request, request_user_id)
455+
456+
auth = Auth(user=None)
457+
458+
mock_addon = MagicMock()
459+
mock_addon.short_name = 'github'
460+
mock_get_addon_from_gv.return_value = mock_addon
461+
462+
result = node.get_addon('github', auth=auth)
463+
464+
mock_get_addon_from_gv.assert_called_once_with(
465+
gv_pk='github',
466+
requesting_user_id=request_user_id,
467+
auth=auth
468+
)
469+
assert result == mock_addon
470+
357471

358472
@pytest.mark.django_db
359473
class TestGVTranslationsOptimization:

0 commit comments

Comments
 (0)