improve tests for gravyvalet feature flagging

addons/base/utils.py

@@ -68,7 +68,6 @@ def __init__(self, resource, auth, legacy_config):
 
     @staticmethod
     def get_configured_storage_addons_data(config_id, auth):
-        print(auth)
         resp = requests.get(
             settings.GV_NODE_ADDON_ENDPOINT.format(config_id=config_id),
         )

api/base/views.py

@@ -635,10 +635,13 @@ def bulk_get_file_nodes_from_wb_resp(self, files_list):
 
         for item in files_list:
             attrs = item['attributes']
-            config = GravyValetAddonAppConfig(self, attrs['provider'], self.request)
+            if waffle.flag_is_active(self.request, features.ENABLE_GV):
+                short_name = GravyValetAddonAppConfig(self, attrs['provider'], self.request).legacy_config.short_name
+            else:
+                short_name = attrs['provider']
 
             base_class = BaseFileNode.resolve_class(
-                config.legacy_config.short_name,
+                short_name,
                 BaseFileNode.FOLDER if attrs['kind'] == 'folder'
                 else BaseFileNode.FILE,
             )
@@ -665,7 +668,11 @@ def bulk_get_file_nodes_from_wb_resp(self, files_list):
             else:
                 file_objs.append(file_obj)
 
-            file_obj.provider = config.config_id
+            # TODO: Improve robustness
+            if waffle.flag_is_active(self.request, features.ENABLE_GV):
+                config = GravyValetAddonAppConfig(self, attrs['provider'], self.request).legacy_config
+                file_obj.provider = config.config_id
+
             file_obj.update(None, attrs, user=self.request.user, save=False)
 
         bulk_update(file_objs)

api/nodes/serializers.py

@@ -1452,7 +1452,7 @@ class Meta:
     def get_id(obj):
         from osf.utils.requests import get_current_request
         if waffle.flag_is_active(get_current_request(), features.ENABLE_GV):
-            return obj.id
+            return obj.provider
         else:
             return f'{obj.node._id}:{obj.provider}'
 

api/nodes/utils.py

@@ -58,7 +58,7 @@ def get_file_object(target, path, provider, request):
         headers={'Authorization': request.META.get('HTTP_AUTHORIZATION')},
     )
 
-    if waterbutler_request.status_code == 401:
+    if waterbutler_request.status_code in (401, 403):
         raise PermissionDenied
 
     if waterbutler_request.status_code == 404:

api_tests/providers/test_files_with_gv.py

@@ -9,10 +9,14 @@
 )
 from waffle.testutils import override_flag
 from django.shortcuts import reverse
+from api_tests.draft_nodes.views.test_draft_node_files_lists import prepare_mock_wb_response
 
 
 @pytest.mark.django_db
 class TestWaffleFilesProviderView:
+    """
+    Just passing id as name, no need to mock GV
+    """
 
     @pytest.fixture(autouse=True)
     def override_flag(self):
@@ -31,7 +35,6 @@ def provider_gv_id(self):
     def node(self, user):
         return ProjectFactory(
             creator=user,
-            comment_level='public'
         )
 
     @pytest.fixture()
@@ -88,17 +91,13 @@ def test_must_be_contributor(self, app, addon_files_url):
         assert res.status_code == 403
 
     def test_get_file_provider(self, app, user, addon_files_url, file, provider_gv_id):
-
-        res = app.get(
-            addon_files_url,
-            auth=user.auth
-        )
+        res = app.get(addon_files_url, auth=user.auth)
 
         assert res.status_code == 200
         attributes = res.json['data']['attributes']
         assert attributes['provider'] == str(provider_gv_id)
         assert attributes['name'] == str(provider_gv_id)
-        assert res.json['data']['id'] == provider_gv_id
+        assert res.json['data']['id'] == str(provider_gv_id)
 
 
 @pytest.mark.django_db
@@ -142,18 +141,42 @@ def file_url(self, node, provider_gv_id, file):
                 'version': 'v2',
                 'node_id': node._id,
                 'provider': f'{provider_gv_id}/',
-                'path': f'{file._id}/'
+                'path':  f'{file._id}/'
             }
         )
 
-    def test_must_have_auth(self, app, file_url):
+    @responses.activate
+    def test_must_have_auth(self, app, user, file_url, file, provider_gv_id, node):
+        prepare_mock_wb_response(
+            node=node,
+            path=file.path + '/',
+            provider='1',
+            status_code=505  # invalid auth should not reach mock
+        )
+
+        responses.add_passthru('http://192.168.168.167:8004/v1/configured-storage-addons/1/base_account/')
+        responses.add_passthru('http://192.168.168.167:8004/v1/authorized-storage-accounts/2/external_storage_service/')
+
         res = app.get(
             file_url,
+            auth=('invaid', 'auth'),
             expect_errors=True
         )
+        assert res.status_code != 505   # invalid auth should not reach mock response
         assert res.status_code == 401
 
-    def test_must_be_contributor(self, app, file_url):
+    @responses.activate
+    def test_must_be_contributor(self, app, user, file_url, file, provider_gv_id, node):
+        prepare_mock_wb_response(
+            node=node,
+            path=file.path + '/',
+            provider='1',
+            status_code=403
+        )
+
+        responses.add_passthru('http://192.168.168.167:8004/v1/configured-storage-addons/1/base_account/')
+        responses.add_passthru('http://192.168.168.167:8004/v1/authorized-storage-accounts/2/external_storage_service/')
+
         res = app.get(
             file_url,
             auth=AuthUserFactory().auth,
@@ -163,8 +186,6 @@ def test_must_be_contributor(self, app, file_url):
 
     @responses.activate
     def test_get_file_provider(self, app, user, file_url, file, provider_gv_id, node):
-        from api_tests.draft_nodes.views.test_draft_node_files_lists import prepare_mock_wb_response
-
         prepare_mock_wb_response(
             path=file.path + '/',
             node=node,