From e2fc21bbdd8713e789162fa7420e07157ec0617d Mon Sep 17 00:00:00 2001 From: ihorsokhanexoft Date: Tue, 30 Sep 2025 04:34:30 +0300 Subject: [PATCH 01/13] [ENG-9020] Custom html host for legacy OSF (#11338) * custom html host for legacy OSF * fixed html links, added a test --- api/base/serializers.py | 7 +++++++ api_tests/nodes/views/test_node_list.py | 10 ++++++++++ 2 files changed, 17 insertions(+) diff --git a/api/base/serializers.py b/api/base/serializers.py index 60afedc1876..5f430fbf7e6 100644 --- a/api/base/serializers.py +++ b/api/base/serializers.py @@ -1183,6 +1183,13 @@ def to_representation(self, obj): if hasattr(obj, 'get_absolute_info_url'): ret['info'] = self._extend_url_with_vol_key(obj.get_absolute_info_url()) + request = self.context['request'] + referer = request.headers.get('Referer', '') + if 'html' in ret and 'legacy' in referer: + parsed_html_url = urlparse(ret['html']) + legacy_url = urlparse(referer) + ret['html'] = parsed_html_url._replace(scheme=legacy_url.scheme, netloc=legacy_url.netloc).geturl() + return ret diff --git a/api_tests/nodes/views/test_node_list.py b/api_tests/nodes/views/test_node_list.py index 35a8ebed143..62970a10920 100644 --- a/api_tests/nodes/views/test_node_list.py +++ b/api_tests/nodes/views/test_node_list.py @@ -259,6 +259,16 @@ def test_current_user_permissions(self, app, user, url, public_project, non_cont res = app.get(url_public, auth=superuser.auth) assert permissions.READ not in res.json['data'][0]['attributes']['current_user_permissions'] + def test_legacy_host_for_htmls(self, app, url, public_project): + settings.DOMAIN = 'https://staging3.osf.io' + current_domain_response = app.get(url).json['data'] + assert current_domain_response[-1]['links']['html'].startswith(settings.DOMAIN) + + # mock request from legacy OSF domain to staging3 backend + # so that backend uses it to generate html links instead of current domain + legacy_domain_response = app.get(url, headers={'Referer': 'http://legacy.osf.io'}).json['data'] + assert legacy_domain_response[-1]['links']['html'].startswith('http://legacy.osf.io') + @pytest.mark.django_db @pytest.mark.enable_bookmark_creation From efc103f6c4e66284cd463a8dc0ac1ed96990e810 Mon Sep 17 00:00:00 2001 From: Ihor Sokhan Date: Tue, 30 Sep 2025 19:05:12 +0300 Subject: [PATCH 02/13] anyone can duplicate public project (structure only) --- api/nodes/serializers.py | 2 +- api_tests/nodes/views/test_node_list.py | 112 ++++++++++++++++++++---- 2 files changed, 94 insertions(+), 20 deletions(-) diff --git a/api/nodes/serializers.py b/api/nodes/serializers.py index f3323d356cb..016a1e76c55 100644 --- a/api/nodes/serializers.py +++ b/api/nodes/serializers.py @@ -787,7 +787,7 @@ def create(self, validated_data): template_node = Node.load(template_from) if template_node is None: raise exceptions.NotFound - if not template_node.has_permission(user, osf_permissions.READ, check_parent=False): + if not template_node.is_public and not template_node.is_contributor(user): raise exceptions.PermissionDenied validated_data.pop('creator') changed_data = {template_from: validated_data} diff --git a/api_tests/nodes/views/test_node_list.py b/api_tests/nodes/views/test_node_list.py index 44502dd6e36..e8ab79c232c 100644 --- a/api_tests/nodes/views/test_node_list.py +++ b/api_tests/nodes/views/test_node_list.py @@ -1477,25 +1477,6 @@ def test_create_from_template_errors(self, app, user_one, user_two, url): expect_errors=True) assert res.status_code == 404 - # test_403_on_create_from_template_of_unauthorized_project - template_from = ProjectFactory(creator=user_two, is_public=True) - templated_project_data = { - 'data': { - 'type': 'nodes', - 'attributes': - { - 'title': 'No permission', - 'category': 'project', - 'template_from': template_from._id, - } - } - } - res = app.post_json_api( - url, templated_project_data, - auth=user_one.auth, - expect_errors=True) - assert res.status_code == 403 - def test_creates_project_from_template(self, app, user_one, category, url): template_from = ProjectFactory(creator=user_one, is_public=True) template_component = ProjectFactory( @@ -1527,6 +1508,99 @@ def test_creates_project_from_template(self, app, user_one, category, url): assert len(new_project.nodes) == len(template_from.nodes) assert new_project.nodes[0].title == template_component.title + def test_non_contributor_create_project_from_public_template_success(self, app, user_one, category, url): + template_from = ProjectFactory(creator=user_one, is_public=True) + user_without_permissions = AuthUserFactory() + templated_project_data = { + 'data': { + 'type': 'nodes', + 'attributes': + { + 'title': 'template from project', + 'category': category, + 'template_from': template_from._id, + } + } + } + with capture_notifications(): + res = app.post_json_api( + url, templated_project_data, + auth=user_without_permissions.auth + ) + assert res.status_code == 201 + + def test_non_contributor_create_project_from_private_template_no_permission_fails(self, app, user_one, category, url): + template_from = ProjectFactory(creator=user_one, is_public=False) + user_without_permissions = AuthUserFactory() + templated_project_data = { + 'data': { + 'type': 'nodes', + 'attributes': + { + 'title': 'template from project', + 'category': category, + 'template_from': template_from._id, + } + } + } + res = app.post_json_api( + url, templated_project_data, + auth=user_without_permissions.auth, + expect_errors=True + ) + assert res.status_code == 403 + + def test_contributor_create_project_from_private_template_with_permission_success(self, app, user_one, category, url): + template_from = ProjectFactory(creator=user_one, is_public=False) + user_without_permissions = AuthUserFactory() + template_from.add_contributor(user_without_permissions, permissions=permissions.READ, auth=Auth(user_one), save=True) + templated_project_data = { + 'data': { + 'type': 'nodes', + 'attributes': + { + 'title': 'template from project', + 'category': category, + 'template_from': template_from._id, + } + } + } + with capture_notifications(): + res = app.post_json_api( + url, templated_project_data, + auth=user_without_permissions.auth + ) + assert res.status_code == 201 + assert template_from.has_permission(user_without_permissions, permissions.READ) + + template_from.update_contributor( + user_without_permissions, + permission=permissions.WRITE, + auth=Auth(user_one), + save=True, + visible=True + ) + res = app.post_json_api( + url, templated_project_data, + auth=user_without_permissions.auth + ) + assert res.status_code == 201 + assert template_from.has_permission(user_without_permissions, permissions.WRITE) + + template_from.update_contributor( + user_without_permissions, + permission=permissions.ADMIN, + auth=Auth(user_one), + save=True, + visible=True + ) + res = app.post_json_api( + url, templated_project_data, + auth=user_without_permissions.auth + ) + assert res.status_code == 201 + assert template_from.has_permission(user_without_permissions, permissions.ADMIN) + def test_creates_project_creates_project_and_sanitizes_html( self, app, user_one, category, url): title = 'Cool Project' From 29e8cd7fd7a5b47848247e2e04c0986fa8c6a586 Mon Sep 17 00:00:00 2001 From: Ihor Sokhan Date: Tue, 30 Sep 2025 20:02:03 +0300 Subject: [PATCH 03/13] remove capture_notifications as the result of rebasing from pbs-25-19 --- api_tests/nodes/views/test_node_list.py | 78 ++++++++++++------------- 1 file changed, 38 insertions(+), 40 deletions(-) diff --git a/api_tests/nodes/views/test_node_list.py b/api_tests/nodes/views/test_node_list.py index e8ab79c232c..bbfa7870050 100644 --- a/api_tests/nodes/views/test_node_list.py +++ b/api_tests/nodes/views/test_node_list.py @@ -1522,11 +1522,10 @@ def test_non_contributor_create_project_from_public_template_success(self, app, } } } - with capture_notifications(): - res = app.post_json_api( - url, templated_project_data, - auth=user_without_permissions.auth - ) + res = app.post_json_api( + url, templated_project_data, + auth=user_without_permissions.auth + ) assert res.status_code == 201 def test_non_contributor_create_project_from_private_template_no_permission_fails(self, app, user_one, category, url): @@ -1565,41 +1564,40 @@ def test_contributor_create_project_from_private_template_with_permission_succes } } } - with capture_notifications(): - res = app.post_json_api( - url, templated_project_data, - auth=user_without_permissions.auth - ) - assert res.status_code == 201 - assert template_from.has_permission(user_without_permissions, permissions.READ) - - template_from.update_contributor( - user_without_permissions, - permission=permissions.WRITE, - auth=Auth(user_one), - save=True, - visible=True - ) - res = app.post_json_api( - url, templated_project_data, - auth=user_without_permissions.auth - ) - assert res.status_code == 201 - assert template_from.has_permission(user_without_permissions, permissions.WRITE) - - template_from.update_contributor( - user_without_permissions, - permission=permissions.ADMIN, - auth=Auth(user_one), - save=True, - visible=True - ) - res = app.post_json_api( - url, templated_project_data, - auth=user_without_permissions.auth - ) - assert res.status_code == 201 - assert template_from.has_permission(user_without_permissions, permissions.ADMIN) + res = app.post_json_api( + url, templated_project_data, + auth=user_without_permissions.auth + ) + assert res.status_code == 201 + assert template_from.has_permission(user_without_permissions, permissions.READ) + + template_from.update_contributor( + user_without_permissions, + permission=permissions.WRITE, + auth=Auth(user_one), + save=True, + visible=True + ) + res = app.post_json_api( + url, templated_project_data, + auth=user_without_permissions.auth + ) + assert res.status_code == 201 + assert template_from.has_permission(user_without_permissions, permissions.WRITE) + + template_from.update_contributor( + user_without_permissions, + permission=permissions.ADMIN, + auth=Auth(user_one), + save=True, + visible=True + ) + res = app.post_json_api( + url, templated_project_data, + auth=user_without_permissions.auth + ) + assert res.status_code == 201 + assert template_from.has_permission(user_without_permissions, permissions.ADMIN) def test_creates_project_creates_project_and_sanitizes_html( self, app, user_one, category, url): From 2bc27c56f90006cec6f5b3dae4cdbb3d058d692f Mon Sep 17 00:00:00 2001 From: Anton Krytskyi Date: Thu, 2 Oct 2025 16:02:20 +0300 Subject: [PATCH 04/13] allow READ users to create guid for files --- api/files/views.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/api/files/views.py b/api/files/views.py index bd2eb9979cd..197c1dd3cdb 100644 --- a/api/files/views.py +++ b/api/files/views.py @@ -30,7 +30,6 @@ FileDetailSerializer, FileVersionSerializer, ) -from osf.utils.permissions import ADMIN class FileMixin: @@ -87,11 +86,11 @@ def get_target(self): # overrides RetrieveAPIView def get_object(self): - user = utils.get_user_auth(self.request).user file = self.get_file() if self.request.GET.get('create_guid', False): - if (self.get_target().has_permission(user, ADMIN) and utils.has_admin_scope(self.request)): + auth = utils.get_user_auth(self.request) + if self.get_target().can_view(auth): file.get_guid(create=True) # We normally would pass this through `get_file` as an annotation, but the `select_for_update` feature prevents From 4f8b2c2e0d9a6bbb5266756e25a3c7850eacfa7c Mon Sep 17 00:00:00 2001 From: antkryt Date: Thu, 2 Oct 2025 17:19:54 +0300 Subject: [PATCH 05/13] validate only on permission change (#11336) --- api/preprints/serializers.py | 2 ++ osf/models/contributor.py | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/api/preprints/serializers.py b/api/preprints/serializers.py index ee109f852a6..d47cc9098dc 100644 --- a/api/preprints/serializers.py +++ b/api/preprints/serializers.py @@ -52,6 +52,7 @@ ) from osf.utils import permissions as osf_permissions from osf.utils.workflows import DefaultStates +from osf.models.contributor import get_user_permission class PrimaryFileRelationshipField(RelationshipField): @@ -653,6 +654,7 @@ def validate_permission(self, value): user # if user is None then probably we're trying to make bulk update and this validation is not relevant and preprint.machine_state == DefaultStates.INITIAL.value and preprint.creator_id == user.id + and get_user_permission(user, preprint) != value ): raise ValidationError( 'You cannot change your permission setting at this time. ' diff --git a/osf/models/contributor.py b/osf/models/contributor.py index a427a7e50f6..42944161a03 100644 --- a/osf/models/contributor.py +++ b/osf/models/contributor.py @@ -103,11 +103,15 @@ def get_contributor_permission(contributor, resource): """ Returns a contributor's permissions - perms through contributorship only. No permissions through osf group membership. """ + return get_user_permission(contributor.user, resource) + + +def get_user_permission(user, resource): read = resource.format_group(permissions.READ) write = resource.format_group(permissions.WRITE) admin = resource.format_group(permissions.ADMIN) # Checking for django group membership allows you to also get the intended permissions of unregistered contributors - user_groups = contributor.user.groups.filter(name__in=[read, write, admin]).values_list('name', flat=True) + user_groups = user.groups.filter(name__in=[read, write, admin]).values_list('name', flat=True) if admin in user_groups: return permissions.ADMIN elif write in user_groups: From 1824c23be3ec6e30f2153c31570d6761821c226b Mon Sep 17 00:00:00 2001 From: bodintsov Date: Thu, 2 Oct 2025 17:59:40 +0300 Subject: [PATCH 06/13] Fix fail from first attempt (#11342) --- osf/models/user.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osf/models/user.py b/osf/models/user.py index 79e11f34ebd..a989e5b9fd6 100644 --- a/osf/models/user.py +++ b/osf/models/user.py @@ -61,7 +61,7 @@ from website.project import new_bookmark_collection from website.util.metrics import OsfSourceTags, unregistered_created_source_tag from importlib import import_module -from osf.utils.requests import get_headers_from_request +from osf.utils.requests import string_type_request_headers SessionStore = import_module(settings.SESSION_ENGINE).SessionStore @@ -1026,7 +1026,7 @@ def save(self, *args, **kwargs): ret = super().save(*args, **kwargs) # must save BEFORE spam check, as user needs guid. if set(self.SPAM_USER_PROFILE_FIELDS.keys()).intersection(dirty_fields): request = get_current_request() - headers = get_headers_from_request(request) + headers = string_type_request_headers(request) self.check_spam(dirty_fields, request_headers=headers) dirty_fields = set(dirty_fields) From 38ad074ecd0ecf2e0d48237a1455f7fc9f97b4ac Mon Sep 17 00:00:00 2001 From: ihorsokhanexoft Date: Thu, 2 Oct 2025 21:43:54 +0300 Subject: [PATCH 07/13] only moderator and admin can access moderation tabs (#11343) --- api/providers/permissions.py | 6 ++- api/registrations/permissions.py | 6 +-- .../collections/views/test_permissions.py | 22 +++++++++++ api_tests/providers/mixins.py | 39 +++++++++++++++++++ .../preprints/views/test_permissions.py | 23 +++++++++++ .../registrations/views/test_permissions.py | 25 ++++++++++++ 6 files changed, 116 insertions(+), 5 deletions(-) create mode 100644 api_tests/providers/collections/views/test_permissions.py create mode 100644 api_tests/providers/preprints/views/test_permissions.py create mode 100644 api_tests/providers/registrations/views/test_permissions.py diff --git a/api/providers/permissions.py b/api/providers/permissions.py index cc2c9e4632c..52d40e75dc9 100644 --- a/api/providers/permissions.py +++ b/api/providers/permissions.py @@ -1,4 +1,3 @@ -from guardian.shortcuts import get_perms from rest_framework import permissions as drf_permissions from api.base.utils import get_user_auth @@ -36,4 +35,7 @@ def has_permission(self, request, view): class MustBeModerator(drf_permissions.BasePermission): def has_permission(self, request, view): auth = get_user_auth(request) - return bool(get_perms(auth.user, view.get_provider())) + provider = view.get_provider() + is_admin = provider.get_group('admin').user_set.filter(id=auth.user.id).exists() + is_moderator = provider.get_group('moderator').user_set.filter(id=auth.user.id).exists() + return is_moderator or is_admin diff --git a/api/registrations/permissions.py b/api/registrations/permissions.py index de4452ba6a3..526dbb4d702 100644 --- a/api/registrations/permissions.py +++ b/api/registrations/permissions.py @@ -7,10 +7,10 @@ class ContributorOrModerator(permissions.BasePermission): def has_object_permission(self, request, view, obj): auth = get_user_auth(request) + is_admin = obj.provider.get_group('admin').user_set.filter(id=auth.user.id).exists() + is_moderator = obj.provider.get_group('moderator').user_set.filter(id=auth.user.id).exists() - # If a user has perms on the provider, they must be a moderator or admin - is_moderator = bool(get_perms(auth.user, obj.provider)) - return obj.is_admin_contributor(auth.user) or is_moderator + return obj.is_admin_contributor(auth.user) or is_moderator or is_admin class ContributorOrModeratorOrPublic(permissions.BasePermission): diff --git a/api_tests/providers/collections/views/test_permissions.py b/api_tests/providers/collections/views/test_permissions.py new file mode 100644 index 00000000000..ed4f76efcde --- /dev/null +++ b/api_tests/providers/collections/views/test_permissions.py @@ -0,0 +1,22 @@ +import pytest + +from api.base.settings.defaults import API_BASE +from api_tests.providers.mixins import OnlyModeratorOrAdminPermissionsMixin + +from osf_tests.factories import CollectionProviderFactory + + +@pytest.mark.django_db +class TestOnlyModeratorOrAdmin(OnlyModeratorOrAdminPermissionsMixin): + + @pytest.fixture() + def urls(self, provider, moderator, admin): + return [ + f'/{API_BASE}providers/collections/{provider._id}/moderators/', + f'/{API_BASE}providers/collections/{provider._id}/moderators/{moderator._id}/', + f'/{API_BASE}providers/collections/{provider._id}/moderators/{admin._id}/', + ] + + @pytest.fixture() + def provider(self): + return CollectionProviderFactory() diff --git a/api_tests/providers/mixins.py b/api_tests/providers/mixins.py index d6e28678059..86f18c19718 100644 --- a/api_tests/providers/mixins.py +++ b/api_tests/providers/mixins.py @@ -1038,3 +1038,42 @@ def test_provider_has_both_acceptable_and_default_licenses(self, app, provider, assert license_one._id in license_ids assert license_three._id in license_ids assert license_two._id not in license_ids + + +@pytest.mark.django_db +class OnlyModeratorOrAdminPermissionsMixin: + + @pytest.fixture() + def provider(self): + raise NotImplementedError + + @pytest.fixture() + def user(self): + return AuthUserFactory() + + @pytest.fixture() + def moderator(self, provider): + mod = AuthUserFactory() + provider.get_group('moderator').user_set.add(mod) + return mod + + @pytest.fixture() + def admin(self, provider): + adm = AuthUserFactory() + provider.get_group('admin').user_set.add(adm) + return adm + + @pytest.fixture() + def urls(self): + raise NotImplementedError + + def test_moderator_or_admin_have_access_to_provider(self, app, provider, user, moderator, admin, urls): + for url in urls: + user_res = app.get(url, auth=user.auth, expect_errors=True) + assert user_res.status_code == 403 + + moderator_res = app.get(url, auth=moderator.auth) + assert moderator_res.status_code == 200 + + admin_res = app.get(url, auth=admin.auth) + assert admin_res.status_code == 200 diff --git a/api_tests/providers/preprints/views/test_permissions.py b/api_tests/providers/preprints/views/test_permissions.py new file mode 100644 index 00000000000..09b726a991f --- /dev/null +++ b/api_tests/providers/preprints/views/test_permissions.py @@ -0,0 +1,23 @@ +import pytest + +from api.base.settings.defaults import API_BASE +from api_tests.providers.mixins import OnlyModeratorOrAdminPermissionsMixin + +from osf_tests.factories import PreprintProviderFactory + + +@pytest.mark.django_db +class TestOnlyModeratorOrAdmin(OnlyModeratorOrAdminPermissionsMixin): + + @pytest.fixture() + def urls(self, provider, moderator, admin): + return [ + f'/{API_BASE}providers/preprints/{provider._id}/withdraw_requests/', + f'/{API_BASE}providers/preprints/{provider._id}/moderators/', + f'/{API_BASE}providers/preprints/{provider._id}/moderators/{moderator._id}/', + f'/{API_BASE}providers/preprints/{provider._id}/moderators/{admin._id}/', + ] + + @pytest.fixture() + def provider(self): + return PreprintProviderFactory() diff --git a/api_tests/providers/registrations/views/test_permissions.py b/api_tests/providers/registrations/views/test_permissions.py new file mode 100644 index 00000000000..6efd4d2808d --- /dev/null +++ b/api_tests/providers/registrations/views/test_permissions.py @@ -0,0 +1,25 @@ +import pytest + +from api.base.settings.defaults import API_BASE +from api_tests.providers.mixins import OnlyModeratorOrAdminPermissionsMixin + +from osf_tests.factories import RegistrationProviderFactory + + +@pytest.mark.django_db +class TestOnlyModeratorOrAdmin(OnlyModeratorOrAdminPermissionsMixin): + + @pytest.fixture() + def urls(self, provider, moderator, admin): + return [ + f'/{API_BASE}providers/registrations/{provider._id}/requests/', + f'/{API_BASE}providers/registrations/{provider._id}/registrations/', + f'/{API_BASE}providers/registrations/{provider._id}/actions/', + f'/{API_BASE}providers/registrations/{provider._id}/moderators/', + f'/{API_BASE}providers/registrations/{provider._id}/moderators/{moderator._id}/', + f'/{API_BASE}providers/registrations/{provider._id}/moderators/{admin._id}/', + ] + + @pytest.fixture() + def provider(self): + return RegistrationProviderFactory() From 797954094e6a0a85d332d79f88fd12849a73259d Mon Sep 17 00:00:00 2001 From: ihorsokhanexoft Date: Mon, 30 Jun 2025 20:25:34 +0300 Subject: [PATCH 08/13] switch to new UI when user views draft registration file (#11144) ## Purpose Throughout registration creation user can view attached files. However `resolve_guid` doesn't handle this case and renders the legacy UI that shows error because `get_rdf_type` raises `NotImplementedError`. So this case has no be handled by ember. However when we switch to the new UI, we get `draftnode is not a supported target type`. Also there will be some work for FE because for now ember relies on `node` relationship that draft node/registration don't have. Taking into account Futa's words, it's an unusual flow for ember ## Changes Added redirect to ember, updated `view_map` that referenced to the non-existing view `draft_nodes:node-detail` to use `draft_nodes:draft-node-detail` view ## Notes 1. I'm not confident that `draft-node` key is still used in `view_map`. From the `resolve` method of `TargetField` I see that keys are either model name in lowercase or `referent._name` that I couldn't find how is created (maybe automatically). However if it existed, there would be some errors that this view does not exist. So I left both keys but with the correct view name. In case you know the answer, I can remove the original key and leave only the correct one ## Ticket https://openscience.atlassian.net/jira/software/c/projects/ENG/boards/145?selectedIssue=ENG-5810 --- website/views.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/website/views.py b/website/views.py index a0b3daeec3a..ba141abb1e3 100644 --- a/website/views.py +++ b/website/views.py @@ -328,6 +328,9 @@ def resolve_guid(guid, suffix=None): elif isinstance(resource, OsfStorageFile) and isinstance(resource.target, DraftNode): return use_ember_app() + elif isinstance(resource, OsfStorageFile) and isinstance(resource.target, DraftNode): + return use_ember_app() + elif isinstance(resource, BaseFileNode) and resource.is_file and not isinstance(resource.target, Preprint): if isinstance(resource.target, Registration) and flag_is_active(request, features.EMBER_FILE_REGISTRATION_DETAIL): return use_ember_app() From 6eaac7487088db86c0cfa17a306f93e7c1df4a4f Mon Sep 17 00:00:00 2001 From: Ihor Sokhan Date: Wed, 4 Jun 2025 17:07:43 +0300 Subject: [PATCH 09/13] remove caching for ascendants --- osf/models/node.py | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/osf/models/node.py b/osf/models/node.py index 7aee1cb0880..a05735f58d4 100644 --- a/osf/models/node.py +++ b/osf/models/node.py @@ -56,7 +56,7 @@ ) from osf.utils.datetime_aware_jsonfield import DateTimeAwareJSONField from osf.utils.fields import NonNaiveDateTimeField -from osf.utils.requests import get_request_and_user_id, string_type_request_headers, get_current_request +from osf.utils.requests import get_request_and_user_id, string_type_request_headers from osf.utils.workflows import CollectionSubmissionStates from osf.utils import sanitize from website import language, settings @@ -2448,21 +2448,15 @@ def _remove_from_associated_collections(self, auth=None, force=False): ) def _get_addon_from_gv(self, gv_pk, requesting_user_id, auth=None): - request = get_current_request() - # This is to avoid making multiple requests to GV - # within the lifespan of one request on the OSF side - try: - gv_addons = request.gv_addons - except AttributeError: - requesting_user = OSFUser.load(requesting_user_id) - services = gv_translations.get_external_services(requesting_user) - for service in services: - if service.short_name == gv_pk: - break - else: - return None - gv_addons = request.gv_addons = self._get_addons_from_gv(requesting_user_id, service.type, auth=auth) + requesting_user = OSFUser.load(requesting_user_id) + services = gv_translations.get_external_services(requesting_user) + for service in services: + if service.short_name == gv_pk: + break + else: + return None + gv_addons = self._get_addons_from_gv(requesting_user_id, service.type, auth=auth) for item in gv_addons: if item.short_name == gv_pk: return item From 4dc225e40fe873a417dcc578c08876bcc46aa156 Mon Sep 17 00:00:00 2001 From: Ihor Sokhan Date: Tue, 10 Jun 2025 13:17:04 +0300 Subject: [PATCH 10/13] Revert "remove caching for ascendants" This reverts commit 15915b0ae60e8967eb4f8fccfb286a3bff04bbe7. --- osf/models/node.py | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/osf/models/node.py b/osf/models/node.py index a05735f58d4..7aee1cb0880 100644 --- a/osf/models/node.py +++ b/osf/models/node.py @@ -56,7 +56,7 @@ ) from osf.utils.datetime_aware_jsonfield import DateTimeAwareJSONField from osf.utils.fields import NonNaiveDateTimeField -from osf.utils.requests import get_request_and_user_id, string_type_request_headers +from osf.utils.requests import get_request_and_user_id, string_type_request_headers, get_current_request from osf.utils.workflows import CollectionSubmissionStates from osf.utils import sanitize from website import language, settings @@ -2448,15 +2448,21 @@ def _remove_from_associated_collections(self, auth=None, force=False): ) def _get_addon_from_gv(self, gv_pk, requesting_user_id, auth=None): - requesting_user = OSFUser.load(requesting_user_id) - services = gv_translations.get_external_services(requesting_user) - for service in services: - if service.short_name == gv_pk: - break - else: - return None + request = get_current_request() + # This is to avoid making multiple requests to GV + # within the lifespan of one request on the OSF side + try: + gv_addons = request.gv_addons + except AttributeError: + requesting_user = OSFUser.load(requesting_user_id) + services = gv_translations.get_external_services(requesting_user) + for service in services: + if service.short_name == gv_pk: + break + else: + return None + gv_addons = request.gv_addons = self._get_addons_from_gv(requesting_user_id, service.type, auth=auth) - gv_addons = self._get_addons_from_gv(requesting_user_id, service.type, auth=auth) for item in gv_addons: if item.short_name == gv_pk: return item From a6162b6997cb721eb1543d5397b07958b8e4919f Mon Sep 17 00:00:00 2001 From: Ihor Sokhan Date: Tue, 10 Jun 2025 17:01:21 +0300 Subject: [PATCH 11/13] use GV addons caching when it's needed --- osf/models/archive.py | 2 +- osf/models/mixins.py | 4 ++-- osf/models/node.py | 31 +++++++++++++++++++------------ 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/osf/models/archive.py b/osf/models/archive.py index 052d918a99c..9e622764ca7 100644 --- a/osf/models/archive.py +++ b/osf/models/archive.py @@ -146,7 +146,7 @@ def _set_target(self, addon_short_name): def set_targets(self): addons = [] - for addon in [self.src_node.get_addon(name) + for addon in [self.src_node.get_addon(name, cached=False) for name in settings.ADDONS_ARCHIVABLE if settings.ADDONS_ARCHIVABLE[name] != 'none']: if not addon or not isinstance(addon, BaseStorageAddon) or not addon.complete: diff --git a/osf/models/mixins.py b/osf/models/mixins.py index 0e351edcd1b..7839e77c641 100644 --- a/osf/models/mixins.py +++ b/osf/models/mixins.py @@ -554,14 +554,14 @@ def get_or_add_addon(self, name, *args, **kwargs): return addon return self.add_addon(name, *args, **kwargs) - def get_addon(self, name, is_deleted=False, auth=None): + def get_addon(self, name, is_deleted=False, auth=None, cached=True): # Avoid test-breakages by avoiding early access to the request context if name not in self.OSF_HOSTED_ADDONS: request, user_id = get_request_and_user_id() if not user_id and auth and auth.user: user_id = auth.user._id if flag_is_active(request, features.ENABLE_GV): - return self._get_addon_from_gv(gv_pk=name, requesting_user_id=user_id, auth=auth) + return self._get_addon_from_gv(gv_pk=name, requesting_user_id=user_id, auth=auth, cached=cached) try: settings_model = self._settings_model(name) diff --git a/osf/models/node.py b/osf/models/node.py index 7aee1cb0880..2cd351c34d7 100644 --- a/osf/models/node.py +++ b/osf/models/node.py @@ -2447,21 +2447,28 @@ def _remove_from_associated_collections(self, auth=None, force=False): force=True ) - def _get_addon_from_gv(self, gv_pk, requesting_user_id, auth=None): + def _get_addons_from_gv_without_caching(self, gv_pk, requesting_user_id, auth=None): + requesting_user = OSFUser.load(requesting_user_id) + services = gv_translations.get_external_services(requesting_user) + for service in services: + if service.short_name == gv_pk: + break + else: + return None + + return self._get_addons_from_gv(requesting_user_id, service.type, auth=auth) + + def _get_addon_from_gv(self, gv_pk, requesting_user_id, auth=None, cached=True): request = get_current_request() # This is to avoid making multiple requests to GV # within the lifespan of one request on the OSF side - try: - gv_addons = request.gv_addons - except AttributeError: - requesting_user = OSFUser.load(requesting_user_id) - services = gv_translations.get_external_services(requesting_user) - for service in services: - if service.short_name == gv_pk: - break - else: - return None - gv_addons = request.gv_addons = self._get_addons_from_gv(requesting_user_id, service.type, auth=auth) + if cached: + try: + gv_addons = request.gv_addons + except AttributeError: + gv_addons = request.gv_addons = self._get_addons_from_gv_without_caching(gv_pk, requesting_user_id, auth=auth) + else: + gv_addons = self._get_addons_from_gv_without_caching(gv_pk, requesting_user_id, auth=auth) for item in gv_addons: if item.short_name == gv_pk: From a535864b21efc30056fb75705de7f8e36ddf4b47 Mon Sep 17 00:00:00 2001 From: Ihor Sokhan Date: Fri, 20 Jun 2025 17:25:53 +0300 Subject: [PATCH 12/13] fixed None issue when iterate --- osf/models/node.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osf/models/node.py b/osf/models/node.py index 2cd351c34d7..642a40ff269 100644 --- a/osf/models/node.py +++ b/osf/models/node.py @@ -2454,7 +2454,7 @@ def _get_addons_from_gv_without_caching(self, gv_pk, requesting_user_id, auth=No if service.short_name == gv_pk: break else: - return None + return [] return self._get_addons_from_gv(requesting_user_id, service.type, auth=auth) From 3e96220cb8bd0e4a0b081b6e5da76faead4b1d81 Mon Sep 17 00:00:00 2001 From: Ihor Sokhan Date: Fri, 3 Oct 2025 12:12:11 +0300 Subject: [PATCH 13/13] removed duplicate --- website/views.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/website/views.py b/website/views.py index ba141abb1e3..a0b3daeec3a 100644 --- a/website/views.py +++ b/website/views.py @@ -328,9 +328,6 @@ def resolve_guid(guid, suffix=None): elif isinstance(resource, OsfStorageFile) and isinstance(resource.target, DraftNode): return use_ember_app() - elif isinstance(resource, OsfStorageFile) and isinstance(resource.target, DraftNode): - return use_ember_app() - elif isinstance(resource, BaseFileNode) and resource.is_file and not isinstance(resource.target, Preprint): if isinstance(resource.target, Registration) and flag_is_active(request, features.EMBER_FILE_REGISTRATION_DETAIL): return use_ember_app()