Skip to content

Commit

Permalink
[ENG-5805] Fixes for files page integration (CenterForOpenScience#10750)
Browse files Browse the repository at this point in the history
  • Loading branch information
adlius authored Oct 1, 2024
1 parent 88b8892 commit 8484057
Show file tree
Hide file tree
Showing 8 changed files with 42 additions and 22 deletions.
11 changes: 9 additions & 2 deletions addons/base/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from furl import furl
import jwe
import jwt
from osf.external.gravy_valet.translations import EphemeralNodeSettings
import waffle
from django.db import transaction
from django.contrib.contenttypes.models import ContentType
Expand Down Expand Up @@ -837,8 +838,14 @@ def addon_view_or_download_file(auth, path, provider, **kwargs):
if hasattr(target, 'get_addon'):

node_addon = target.get_addon(provider)

if not isinstance(node_addon, BaseStorageAddon):
if flag_is_active(request, features.ENABLE_GV):
if not isinstance(node_addon, EphemeralNodeSettings):
object_text = markupsafe.escape(getattr(target, 'project_or_component', 'this object'))
raise HTTPError(http_status.HTTP_400_BAD_REQUEST, data={
'message_short': 'Bad Request',
'message_long': f'The {provider_safe} add-on containing {path_safe} is no longer connected to {object_text}.'
})
elif not isinstance(node_addon, BaseStorageAddon):
object_text = markupsafe.escape(getattr(target, 'project_or_component', 'this object'))
raise HTTPError(http_status.HTTP_400_BAD_REQUEST, data={
'message_short': 'Bad Request',
Expand Down
6 changes: 5 additions & 1 deletion api/nodes/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -1456,7 +1456,11 @@ class Meta:

@staticmethod
def get_id(obj):
return f'{obj.node._id}:{obj.provider}'
from addons.base.models import BaseAddonSettings
if not obj.provider_settings or issubclass(type(obj.provider_settings), BaseAddonSettings):
return f'{obj.node._id}:{obj.provider}'
else:
return obj.provider_settings.gv_data.resource_id

def get_absolute_url(self, obj):
return absolute_reverse(
Expand Down
4 changes: 2 additions & 2 deletions api/nodes/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@
re_path(r'^(?P<node_id>\w+)/draft_registrations/(?P<draft_id>\w+)/$', views.NodeDraftRegistrationDetail.as_view(), name=views.NodeDraftRegistrationDetail.view_name),
re_path(r'^(?P<node_id>\w+)/files/$', views.NodeStorageProvidersList.as_view(), name=views.NodeStorageProvidersList.view_name),
re_path(r'^(?P<node_id>\w+)/files/providers/(?P<provider>\w+)/?$', views.NodeStorageProviderDetail.as_view(), name=views.NodeStorageProviderDetail.view_name),
re_path(r'^(?P<node_id>\w+)/files/(?P<provider>\w+)(?P<path>/(?:.*/)?)$', views.NodeFilesList.as_view(), name=views.NodeFilesList.view_name),
re_path(r'^(?P<node_id>\w+)/files/(?P<provider>\w+)(?P<path>/.+[^/])$', views.NodeFileDetail.as_view(), name=views.NodeFileDetail.view_name),
re_path(r'^(?P<node_id>\w+)/files/(?P<provider>[a-zA-Z0-9\-]*)(?P<path>/(?:.*/)?)$', views.NodeFilesList.as_view(), name=views.NodeFilesList.view_name),
re_path(r'^(?P<node_id>\w+)/files/(?P<provider>[a-zA-Z0-9\-]*)(?P<path>/.+[^/])$', views.NodeFileDetail.as_view(), name=views.NodeFileDetail.view_name),
re_path(r'^(?P<node_id>\w+)/forks/$', views.NodeForksList.as_view(), name=views.NodeForksList.view_name),
re_path(r'^(?P<node_id>\w+)/groups/$', views.NodeGroupsList.as_view(), name=views.NodeGroupsList.view_name),
re_path(r'^(?P<node_id>\w+)/groups/(?P<group_id>\w+)/$', views.NodeGroupsDetail.as_view(), name=views.NodeGroupsDetail.view_name),
Expand Down
7 changes: 4 additions & 3 deletions osf/external/gravy_valet/request_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
# {{placeholder}} format allows f-string to return a formatable string
ACCOUNT_ENDPOINT = f'{API_BASE}authorized-storage-accounts/{{pk}}'
ADDON_ENDPOINT = f'{API_BASE}configured-storage-addons/{{pk}}'
WB_CONFIG_ENDPOINT = f'{ADDON_ENDPOINT}/waterbutler-config'
WB_CONFIG_ENDPOINT = f'{ADDON_ENDPOINT}/waterbutler-credentials'

USER_LIST_ENDPOINT = f'{API_BASE}user-references'
USER_DETAIL_ENDPOINT = f'{API_BASE}user-references/{{pk}}'
Expand Down Expand Up @@ -145,7 +145,7 @@ def _make_gv_request(
params: dict = None,
):
'''Generates HMAC-Signed auth headers and makes a request to GravyValet, returning the result.'''
full_url = urlunparse(urlparse(endpoint_url)._replace(query=urlencode(params)))
full_url = urlunparse(urlparse(endpoint_url)._replace(query=urlencode(params or {})))
auth_headers = auth_helpers.make_gravy_valet_hmac_headers(
request_url=full_url,
request_method=request_method,
Expand Down Expand Up @@ -189,7 +189,8 @@ def __init__(self, result_entry: dict, included_entities_lookup: dict = None):
self.resource_type = result_entry['type']
self.resource_id = result_entry['id']
self._attributes = dict(result_entry['attributes'])
self._relationships = _extract_relationships(result_entry['relationships'])
if 'relationships' in result_entry:
self._relationships = _extract_relationships(result_entry['relationships'])
self._includes = {}
if included_entities_lookup:
self.extract_included_relationships(included_entities_lookup)
Expand Down
18 changes: 17 additions & 1 deletion osf/external/gravy_valet/translations.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,28 @@ def configured(self):
attribute_name='credentials_available'
)

@property
def deleted(self):
return False

@property
def id(self):
return self.gv_id

@property
def has_auth(self):
return True

@property
def complete(self):
return True

def before_page_load(self, *args, **kwargs):
pass

@property
def folder_id(self):
return self.gv_data.get_attribute('root_folder')
return self.gv_data.get_attribute('root_folder').split(':')[1]

def serialize_waterbutler_credentials(self):
# sufficient for most OAuth services, including Box
Expand Down
14 changes: 3 additions & 11 deletions osf/models/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -2431,17 +2431,9 @@ def _remove_from_associated_collections(self, auth=None, force=False):
)

def _get_addon_from_gv(self, gv_pk, requesting_user_id):
requesting_user = OSFUser.load(requesting_user_id)
gv_addon_data = gv_requests.get_addon(
gv_addon_pk=gv_pk,
requested_resource=self,
requesting_user=requesting_user,
)
return gv_translations.make_ephemeral_node_settings(
gv_addon_data=gv_addon_data,
requested_resource=self,
requesting_user=requesting_user
)
for item in self._get_addons_from_gv(requesting_user_id):
if item.short_name == gv_pk:
return item

def _get_addons_from_gv(self, requesting_user_id):
requesting_user = OSFUser.load(requesting_user_id)
Expand Down
2 changes: 1 addition & 1 deletion osf_tests/external/gravy_valet/gv_fakes.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ class _FakeAddon(_FakeGVEntity):
resource_pk: int
base_account: _FakeAccount
display_name: str = ''
root_folder: str = '/'
root_folder: str = '0:1'

def _serialize_attributes(self):
return {
Expand Down
2 changes: 1 addition & 1 deletion osf_tests/test_gv_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,6 @@ def test_make_ephemeral_node_settings(self, contributor, project, fake_box_addon
assert ephemeral_config.gv_id == fake_box_addon.pk
assert ephemeral_config.config.name == 'addons.box'
assert ephemeral_config.serialize_waterbutler_settings() == {
'folder': fake_box_addon.root_folder,
'folder': fake_box_addon.root_folder.split(':')[1],
'service': 'box'
}

0 comments on commit 8484057

Please sign in to comment.