From d741ec8a8e05f3459e10e1cac9978648840f84f4 Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Tue, 10 Jul 2018 11:45:16 -0500 Subject: [PATCH 01/18] Begin adding Node Settings to APIv2. - Url added, view and serializer started. - New permission class IsContributor added. --- api/nodes/permissions.py | 7 +++++ api/nodes/serializers.py | 55 ++++++++++++++++++++++++++++++++++ api/nodes/urls.py | 1 + api/nodes/views.py | 22 ++++++++++++++ framework/auth/oauth_scopes.py | 7 +++-- 5 files changed, 90 insertions(+), 2 deletions(-) diff --git a/api/nodes/permissions.py b/api/nodes/permissions.py index e429e9e2076..22af2d18e3c 100644 --- a/api/nodes/permissions.py +++ b/api/nodes/permissions.py @@ -51,6 +51,13 @@ def has_object_permission(self, request, view, obj): return obj.has_permission(auth.user, osf_permissions.ADMIN) +class IsContributor(permissions.BasePermission): + def has_object_permission(self, request, view, obj): + assert isinstance(obj, AbstractNode), 'obj must be an Node, got {}'.format(obj) + auth = get_user_auth(request) + return obj.is_contributor(auth.user) + + class IsAdminOrReviewer(permissions.BasePermission): """ Prereg admins can update draft registrations. diff --git a/api/nodes/serializers.py b/api/nodes/serializers.py index 99925772216..bf5639f3564 100644 --- a/api/nodes/serializers.py +++ b/api/nodes/serializers.py @@ -1314,3 +1314,58 @@ def update(self, link, validated_data): link.save() return link + + +class NodeSettingsSerializer(JSONAPISerializer): + id = IDField(source='_id', read_only=True) + type = TypeField() + # Do we need to wrap these fields in HideIfWithdrawal so this serializer + # can be used for registrations? + + # TODO deprecate field from NodeSerializer + allow_access_requests = ser.BooleanField(read_only=True, source='access_requests_enabled') + anyone_can_comment = ser.SerializerMethodField() + anyone_can_edit_wiki = ser.SerializerMethodField() + wiki_enabled = ser.SerializerMethodField() + redirect_link_enabled = ser.SerializerMethodField() + redirect_link_url = ser.SerializerMethodField() + redirect_link_label = ser.SerializerMethodField() + + links = LinksField({ + 'self': 'get_absolute_url' + }) + + # TODO add view_only_links RelationshipField + + def get_anyone_can_comment(self, obj): + return obj.comment_level == 'public' + + def get_anyone_can_edit_wiki(self, obj): + wiki_addon = obj.get_addon('wiki') + return wiki_addon.is_publicly_editable if wiki_addon else None + + def get_wiki_enabled(self, obj): + return 'wiki' in obj.get_addon_names() + + def get_redirect_link_enabled(self, obj): + return 'forward' in obj.get_addon_names() + + def get_redirect_link_url(self, obj): + forward_addon = obj.get_addon('forward') + return forward_addon.url if forward_addon else None + + def get_redirect_link_label(self, obj): + forward_addon = obj.get_addon('forward') + return forward_addon.label if forward_addon else None + + def get_absolute_url(self, obj): + return absolute_reverse( + 'nodes:node-settings', + kwargs={ + 'node_id': self.context['request'].parser_context['kwargs']['node_id'], + 'version': self.context['request'].parser_context['kwargs']['version'] + } + ) + + class Meta: + type_ = 'node-settings' diff --git a/api/nodes/urls.py b/api/nodes/urls.py index 33e4d4f79fa..41e574b0046 100644 --- a/api/nodes/urls.py +++ b/api/nodes/urls.py @@ -42,6 +42,7 @@ url(r'^(?P\w+)/relationships/linked_nodes/$', views.NodeLinkedNodesRelationship.as_view(), name=views.NodeLinkedNodesRelationship.view_name), url(r'^(?P\w+)/relationships/linked_registrations/$', views.NodeLinkedRegistrationsRelationship.as_view(), name=views.NodeLinkedRegistrationsRelationship.view_name), url(r'^(?P\w+)/requests/$', views.NodeRequestListCreate.as_view(), name=views.NodeRequestListCreate.view_name), + url(r'^(?P\w+)/settings/$', views.NodeSettings.as_view(), name=views.NodeSettings.view_name), url(r'^(?P\w+)/view_only_links/$', views.NodeViewOnlyLinksList.as_view(), name=views.NodeViewOnlyLinksList.view_name), url(r'^(?P\w+)/view_only_links/(?P\w+)/$', views.NodeViewOnlyLinkDetail.as_view(), name=views.NodeViewOnlyLinkDetail.view_name), url(r'^(?P\w+)/wikis/$', views.NodeWikiList.as_view(), name=views.NodeWikiList.view_name), diff --git a/api/nodes/views.py b/api/nodes/views.py index bc18eae2e25..6338386e820 100644 --- a/api/nodes/views.py +++ b/api/nodes/views.py @@ -68,6 +68,7 @@ ContributorDetailPermissions, ReadOnlyIfRegistration, IsAdminOrReviewer, + IsContributor, WriteOrPublicForRelationshipInstitutions, ExcludeWithdrawals, NodeLinksShowIfVersion, @@ -88,6 +89,7 @@ NodeContributorsCreateSerializer, NodeViewOnlyLinkSerializer, NodeViewOnlyLinkUpdateSerializer, + NodeSettingsSerializer, NodeCitationSerializer, NodeCitationStyleSerializer ) @@ -1909,3 +1911,23 @@ def get_default_queryset(self): def get_queryset(self): return self.get_queryset_from_request() + + +class NodeSettings(JSONAPIBaseView, generics.RetrieveUpdateAPIView, NodeMixin): + permission_classes = ( + drf_permissions.IsAuthenticatedOrReadOnly, + base_permissions.TokenHasScope, + IsContributor, + ) + + required_read_scopes = [CoreScopes.NODE_SETTINGS_READ] + required_write_scopes = [CoreScopes.NODE_SETTINGS_WRITE] + + serializer_class = NodeSettingsSerializer + + view_category = 'nodes' + view_name = 'node-settings' + + # overrides RetrieveUpdateAPIView + def get_object(self): + return self.get_node() diff --git a/framework/auth/oauth_scopes.py b/framework/auth/oauth_scopes.py index 9d9c9a5bf87..b5c3f650c9a 100644 --- a/framework/auth/oauth_scopes.py +++ b/framework/auth/oauth_scopes.py @@ -112,6 +112,9 @@ class CoreScopes(object): NODE_REQUESTS_READ = 'node_requests_read' NODE_REQUESTS_WRITE = 'node_requests_write' + NODE_SETTINGS_READ = 'node_settings_read' + NODE_SETTINGS_WRITE = 'node_settings_write' + PROVIDERS_WRITE = 'providers_write' WAFFLE_READ = 'waffle_read' @@ -203,11 +206,11 @@ class ComposedScopes(object): # Privileges relating to who can access a node (via contributors or registrations) NODE_ACCESS_READ = (CoreScopes.NODE_CONTRIBUTORS_READ, CoreScopes.NODE_REGISTRATIONS_READ, CoreScopes.NODE_VIEW_ONLY_LINKS_READ, CoreScopes.REGISTRATION_VIEW_ONLY_LINKS_READ, - CoreScopes.NODE_REQUESTS_READ) + CoreScopes.NODE_REQUESTS_READ, CoreScopes.NODE_SETTINGS_READ) NODE_ACCESS_WRITE = NODE_ACCESS_READ + \ (CoreScopes.NODE_CONTRIBUTORS_WRITE, CoreScopes.NODE_REGISTRATIONS_WRITE, CoreScopes.NODE_VIEW_ONLY_LINKS_WRITE, CoreScopes.REGISTRATION_VIEW_ONLY_LINKS_WRITE, - CoreScopes.NODE_REQUESTS_WRITE) + CoreScopes.NODE_REQUESTS_WRITE, CoreScopes.NODE_SETTINGS_WRITE) # Combine all sets of node permissions into one convenience level NODE_ALL_READ = NODE_METADATA_READ + NODE_DATA_READ + NODE_ACCESS_READ From e4bde4de83f4a16610bb76bebd09d7f0b8e02e13 Mon Sep 17 00:00:00 2001 From: erinspace Date: Tue, 10 Jul 2018 14:58:34 -0400 Subject: [PATCH 02/18] Make sure user is viewing or has WRITE permissions in IsContributor --- api/nodes/permissions.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/api/nodes/permissions.py b/api/nodes/permissions.py index 22af2d18e3c..f75d4273593 100644 --- a/api/nodes/permissions.py +++ b/api/nodes/permissions.py @@ -55,7 +55,10 @@ class IsContributor(permissions.BasePermission): def has_object_permission(self, request, view, obj): assert isinstance(obj, AbstractNode), 'obj must be an Node, got {}'.format(obj) auth = get_user_auth(request) - return obj.is_contributor(auth.user) + if request.method in permissions.SAFE_METHODS: + return obj.is_contributor(auth.user) + else: + return obj.has_permission(user, 'write') class IsAdminOrReviewer(permissions.BasePermission): From 60031ad7ad14b39444c84f476ee0beea4d515da1 Mon Sep 17 00:00:00 2001 From: erinspace Date: Tue, 10 Jul 2018 15:00:24 -0400 Subject: [PATCH 03/18] Add NodeSettingsUpdateSerializer for updating fields on NodeSettings - Use this serializer for PUT or PATCH requests - Update the view to choose the right serializer --- api/nodes/serializers.py | 87 ++++++++++++++++++++++++++++++++++++++-- api/nodes/views.py | 6 +++ 2 files changed, 90 insertions(+), 3 deletions(-) diff --git a/api/nodes/serializers.py b/api/nodes/serializers.py index bf5639f3564..fe479e5a48d 100644 --- a/api/nodes/serializers.py +++ b/api/nodes/serializers.py @@ -1315,6 +1315,85 @@ def update(self, link, validated_data): link.save() return link +class NodeSettingsUpdateSerializer(NodeSettingsSerializer): + anyone_can_comment = ser.BooleanField(write_only=True, required=False) + anyone_can_edit_wiki = ser.BooleanField(write_only=True, required=False) + wiki_enabled = ser.BooleanField(write_only=True, required=False) + redirect_link_enabled = ser.BooleanField(write_only=True, required=False) + redirect_link_url = ser.URLField(read_only=True) + # TODO - add a length restriction + redirect_link_label = ser.CharField() + + def update(self, obj, validated_data): + user = self.context['request'].user + auth = get_user_auth(self.context['request']) + is_admin = obj.has_permission(user, 'admin') + is_read_write = obj.has_permission(user, 'write') + + access_requests_enabled = validated_data.get('access_requests_enabled') + anyone_can_comment = validated_data.get('anyone_can_comment') + anyone_can_edit_wiki = validated_data.get('anyone_can_edit_wiki') + wiki_enabled = validated_data.get('wiki_enabled') + redirect_link_enabled = validated_data.get('redirect_link_enabled') + redirect_link_url = validated_data.get('redirect_link_url') + redirect_link_label = validated_data.get('redirect_link_label') + + admin_only_field_names = [ + 'access_requests_enabled', + 'anyone_can_comment', + 'anyone_can_edit_wiki', + 'wiki_enabled', + ] + save_node = False + if set(validated_data.keys()).intersection(set(admin_only_field_names)) and not is_admin: + raise exceptions.PermissionDenied + if access_requests_enabled is not None: + obj.access_requests_enabled = access_requests_enabled + save_node = True + if anyone_can_comment is not None: + if anyone_can_comment is True: + obj.comment_level = 'public' + else: + obj.comment_level = 'private' + save_node = True + if save_node: + obj.save() + + if anyone_can_edit_wiki is not None: + wiki_addon = obj.get_addon('wiki') + if wiki_addon + if anyone_can_edit_wiki is True: + wiki_addon.is_publicly_editable = True + else: + wiki_addon.is_publicly_editable = False + wiki_addon.save() + # TODO: test me well! + if redirect_link_enabled is not None: + if not redirect_link_url and redirect_link_enabled: + raise ValidationError('You must include a redirect URL to enable a redirect.') + if redirect_link_enabled: + forward_addon = obj.get_or_add_addon('forward') + forward_addon.url = redirect_link_url + if redirect_link_label: + forward_addon.label = redirect_link_label + forward_addon.save() + if redirect_link_url is not None: + forward_addon = obj.get_addon('forward') + if not forward_addon: + raise ValidationError('You must first set redirect_link_enabled to True before specifying a redirect link URL.') + elif not redirect_link_enabled: + forward_addon.url = redirect_link_url + forward_addon.save() + if redirect_link_label is not None: + forward_addon = obj.get_addon('forward') + if not forward_addon: + raise ValidationError('You must first set redirect_link_enabled to True before specifying a redirect link URL.') + elif not redirect_link_enabled: + forward_addon.label = redirect_link_label + forward_addon.save() + + return obj + class NodeSettingsSerializer(JSONAPISerializer): id = IDField(source='_id', read_only=True) @@ -1323,20 +1402,22 @@ class NodeSettingsSerializer(JSONAPISerializer): # can be used for registrations? # TODO deprecate field from NodeSerializer - allow_access_requests = ser.BooleanField(read_only=True, source='access_requests_enabled') + access_requests_enabled = ser.BooleanField() anyone_can_comment = ser.SerializerMethodField() anyone_can_edit_wiki = ser.SerializerMethodField() wiki_enabled = ser.SerializerMethodField() redirect_link_enabled = ser.SerializerMethodField() redirect_link_url = ser.SerializerMethodField() redirect_link_label = ser.SerializerMethodField() + view_only_links = RelationshipField( + related_view='nodes:node-view-only-links', + related_view_kwargs={'node_id': '<_id>'}, + ) links = LinksField({ 'self': 'get_absolute_url' }) - # TODO add view_only_links RelationshipField - def get_anyone_can_comment(self, obj): return obj.comment_level == 'public' diff --git a/api/nodes/views.py b/api/nodes/views.py index 6338386e820..0a999addff0 100644 --- a/api/nodes/views.py +++ b/api/nodes/views.py @@ -90,6 +90,7 @@ NodeViewOnlyLinkSerializer, NodeViewOnlyLinkUpdateSerializer, NodeSettingsSerializer, + NodeSettingsUpdateSerializer, NodeCitationSerializer, NodeCitationStyleSerializer ) @@ -1931,3 +1932,8 @@ class NodeSettings(JSONAPIBaseView, generics.RetrieveUpdateAPIView, NodeMixin): # overrides RetrieveUpdateAPIView def get_object(self): return self.get_node() + + def get_serializer_class(self): + if self.request.method == 'PUT' or self.request.method == 'PATCH': + return NodeSettingsUpdateSerializer + return NodeSettingsSerializer From 010492a2a8ec4382860da26f0e838c87fa13a2de Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Tue, 10 Jul 2018 14:34:36 -0500 Subject: [PATCH 04/18] Add node settings test file. --- api_tests/nodes/views/test_node_settings.py | 1 + 1 file changed, 1 insertion(+) create mode 100644 api_tests/nodes/views/test_node_settings.py diff --git a/api_tests/nodes/views/test_node_settings.py b/api_tests/nodes/views/test_node_settings.py new file mode 100644 index 00000000000..8b137891791 --- /dev/null +++ b/api_tests/nodes/views/test_node_settings.py @@ -0,0 +1 @@ + From 0d03e562ca704c64d9d74a4e2462dc51b983a3ed Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Tue, 10 Jul 2018 14:57:48 -0500 Subject: [PATCH 05/18] Add missed items to IsContributor permission class and NodeSettings serializer to get tests to run. --- api/nodes/permissions.py | 2 +- api/nodes/serializers.py | 117 ++++++++++---------- api_tests/nodes/views/test_node_settings.py | 72 ++++++++++++ 3 files changed, 132 insertions(+), 59 deletions(-) diff --git a/api/nodes/permissions.py b/api/nodes/permissions.py index f75d4273593..ff87e4fc0bc 100644 --- a/api/nodes/permissions.py +++ b/api/nodes/permissions.py @@ -58,7 +58,7 @@ def has_object_permission(self, request, view, obj): if request.method in permissions.SAFE_METHODS: return obj.is_contributor(auth.user) else: - return obj.has_permission(user, 'write') + return obj.has_permission(auth.user, 'write') class IsAdminOrReviewer(permissions.BasePermission): diff --git a/api/nodes/serializers.py b/api/nodes/serializers.py index fe479e5a48d..0837e14b40a 100644 --- a/api/nodes/serializers.py +++ b/api/nodes/serializers.py @@ -1315,6 +1315,64 @@ def update(self, link, validated_data): link.save() return link + +class NodeSettingsSerializer(JSONAPISerializer): + id = IDField(source='_id', read_only=True) + type = TypeField() + # Do we need to wrap these fields in HideIfWithdrawal so this serializer + # can be used for registrations? + + # TODO deprecate field from NodeSerializer + access_requests_enabled = ser.BooleanField() + anyone_can_comment = ser.SerializerMethodField() + anyone_can_edit_wiki = ser.SerializerMethodField() + wiki_enabled = ser.SerializerMethodField() + redirect_link_enabled = ser.SerializerMethodField() + redirect_link_url = ser.SerializerMethodField() + redirect_link_label = ser.SerializerMethodField() + view_only_links = RelationshipField( + related_view='nodes:node-view-only-links', + related_view_kwargs={'node_id': '<_id>'}, + ) + + links = LinksField({ + 'self': 'get_absolute_url' + }) + + def get_anyone_can_comment(self, obj): + return obj.comment_level == 'public' + + def get_anyone_can_edit_wiki(self, obj): + wiki_addon = obj.get_addon('wiki') + return wiki_addon.is_publicly_editable if wiki_addon else None + + def get_wiki_enabled(self, obj): + return 'wiki' in obj.get_addon_names() + + def get_redirect_link_enabled(self, obj): + return 'forward' in obj.get_addon_names() + + def get_redirect_link_url(self, obj): + forward_addon = obj.get_addon('forward') + return forward_addon.url if forward_addon else None + + def get_redirect_link_label(self, obj): + forward_addon = obj.get_addon('forward') + return forward_addon.label if forward_addon else None + + def get_absolute_url(self, obj): + return absolute_reverse( + 'nodes:node-settings', + kwargs={ + 'node_id': self.context['request'].parser_context['kwargs']['node_id'], + 'version': self.context['request'].parser_context['kwargs']['version'] + } + ) + + class Meta: + type_ = 'node-settings' + + class NodeSettingsUpdateSerializer(NodeSettingsSerializer): anyone_can_comment = ser.BooleanField(write_only=True, required=False) anyone_can_edit_wiki = ser.BooleanField(write_only=True, required=False) @@ -1361,7 +1419,7 @@ def update(self, obj, validated_data): if anyone_can_edit_wiki is not None: wiki_addon = obj.get_addon('wiki') - if wiki_addon + if wiki_addon: if anyone_can_edit_wiki is True: wiki_addon.is_publicly_editable = True else: @@ -1393,60 +1451,3 @@ def update(self, obj, validated_data): forward_addon.save() return obj - - -class NodeSettingsSerializer(JSONAPISerializer): - id = IDField(source='_id', read_only=True) - type = TypeField() - # Do we need to wrap these fields in HideIfWithdrawal so this serializer - # can be used for registrations? - - # TODO deprecate field from NodeSerializer - access_requests_enabled = ser.BooleanField() - anyone_can_comment = ser.SerializerMethodField() - anyone_can_edit_wiki = ser.SerializerMethodField() - wiki_enabled = ser.SerializerMethodField() - redirect_link_enabled = ser.SerializerMethodField() - redirect_link_url = ser.SerializerMethodField() - redirect_link_label = ser.SerializerMethodField() - view_only_links = RelationshipField( - related_view='nodes:node-view-only-links', - related_view_kwargs={'node_id': '<_id>'}, - ) - - links = LinksField({ - 'self': 'get_absolute_url' - }) - - def get_anyone_can_comment(self, obj): - return obj.comment_level == 'public' - - def get_anyone_can_edit_wiki(self, obj): - wiki_addon = obj.get_addon('wiki') - return wiki_addon.is_publicly_editable if wiki_addon else None - - def get_wiki_enabled(self, obj): - return 'wiki' in obj.get_addon_names() - - def get_redirect_link_enabled(self, obj): - return 'forward' in obj.get_addon_names() - - def get_redirect_link_url(self, obj): - forward_addon = obj.get_addon('forward') - return forward_addon.url if forward_addon else None - - def get_redirect_link_label(self, obj): - forward_addon = obj.get_addon('forward') - return forward_addon.label if forward_addon else None - - def get_absolute_url(self, obj): - return absolute_reverse( - 'nodes:node-settings', - kwargs={ - 'node_id': self.context['request'].parser_context['kwargs']['node_id'], - 'version': self.context['request'].parser_context['kwargs']['version'] - } - ) - - class Meta: - type_ = 'node-settings' diff --git a/api_tests/nodes/views/test_node_settings.py b/api_tests/nodes/views/test_node_settings.py index 8b137891791..fcf4bba77a7 100644 --- a/api_tests/nodes/views/test_node_settings.py +++ b/api_tests/nodes/views/test_node_settings.py @@ -1 +1,73 @@ +# -*- coding: utf-8 -*- +import mock +import pytest +from api.base.settings.defaults import API_BASE +from osf_tests.factories import ( + AuthUserFactory, + ProjectFactory, +) + + +@pytest.mark.django_db +class TestNodeSettingsUpdate: + + @pytest.fixture() + def admin_contrib(self): + return AuthUserFactory() + + @pytest.fixture() + def write_contrib(self): + return AuthUserFactory() + + @pytest.fixture() + def read_contrib(self): + return AuthUserFactory() + + @pytest.fixture() + def project(self, admin_contrib, write_contrib, read_contrib): + project = ProjectFactory(creator=admin_contrib) + project.add_contributor(write_contrib, 'write') + project.add_contributor(read_contrib, 'write') + project.save() + return project + + @pytest.fixture() + def url(self, project): + return '/{}nodes/{}/settings/'.format(API_BASE, project._id) + + @pytest.fixture() + def payload(self, project): + return { + 'data': { + 'id': project._id, + 'type': 'node-settings', + 'attributes': { + } + } + } + + + def test_patch_permissions(self, app, project, payload, admin_contrib, write_contrib, read_contrib, url): + payload['data']['attributes']['redirect_link_enabled'] = True + payload['data']['attributes']['redirect_link_url'] = 'https://cos.io' + # Logged out + res = app.patch_json_api(url, payload, expect_errors=True) + assert res.status_code == 401 + + # Logged in, noncontrib + noncontrib = AuthUserFactory() + res = app.patch_json_api(url, payload, auth=noncontrib.auth, expect_errors=True) + assert res.status_code == 403 + + # Logged in read + res = app.patch_json_api(url, payload, auth=read_contrib.auth, expect_errors=True) + assert res.status_code == 403 + + # Logged in write (Write contribs can only change some node settings) + res = app.patch_json_api(url, payload, auth=write_contrib.auth, expect_errors=True) + assert res.status_code == 403 + + # Logged in admin + res = app.patch_json_api(url, payload, auth=admin_contrib.auth) + assert res.status_code == 200 From d5a861d2dc0924f8d77314958e20868c1d485b24 Mon Sep 17 00:00:00 2001 From: erinspace Date: Tue, 10 Jul 2018 16:36:57 -0400 Subject: [PATCH 06/18] Add tests for the NodeSettingsSerializer and GET requests --- api_tests/nodes/views/test_node_settings.py | 104 ++++++++++++++++---- 1 file changed, 86 insertions(+), 18 deletions(-) diff --git a/api_tests/nodes/views/test_node_settings.py b/api_tests/nodes/views/test_node_settings.py index fcf4bba77a7..c281411f874 100644 --- a/api_tests/nodes/views/test_node_settings.py +++ b/api_tests/nodes/views/test_node_settings.py @@ -1,4 +1,3 @@ - # -*- coding: utf-8 -*- import mock import pytest @@ -8,33 +7,102 @@ ProjectFactory, ) +from framework.auth import Auth + + +@pytest.fixture() +def admin_contrib(): + return AuthUserFactory() + +@pytest.fixture() +def write_contrib(): + return AuthUserFactory() + +@pytest.fixture() +def read_contrib(): + return AuthUserFactory() + +@pytest.fixture() +def project(admin_contrib, write_contrib, read_contrib): + project = ProjectFactory(creator=admin_contrib) + project.add_contributor(write_contrib, 'write') + project.add_contributor(read_contrib, 'write') + project.save() + return project + +@pytest.fixture() +def url(project): + return '/{}nodes/{}/settings/'.format(API_BASE, project._id) + @pytest.mark.django_db -class TestNodeSettingsUpdate: +class TestGetNodeSettingsGet: @pytest.fixture() - def admin_contrib(self): + def non_contrib(self): return AuthUserFactory() - @pytest.fixture() - def write_contrib(self): - return AuthUserFactory() + def test_node_settings_detail(self, app, admin_contrib, non_contrib, write_contrib, url, project): - @pytest.fixture() - def read_contrib(self): - return AuthUserFactory() + # non logged in uers can't access node settings + res = app.get(url, expect_errors=True) + assert res.status_code == 401 - @pytest.fixture() - def project(self, admin_contrib, write_contrib, read_contrib): - project = ProjectFactory(creator=admin_contrib) - project.add_contributor(write_contrib, 'write') - project.add_contributor(read_contrib, 'write') + # non_contrib can't access node settings + res = app.get(url, auth=non_contrib.auth, expect_errors=True) + assert res.status_code == 403 + + # read contrib can access node settings + res = app.get(url, auth=write_contrib.auth) + assert res.status_code == 200 + + # admin can access node settings + res = app.get(url, auth=admin_contrib.auth) + assert res.status_code == 200 + + # allow_access_requests + project.allow_access_requests = True project.save() - return project + res = app.get(url, auth=admin_contrib.auth) + attributes = res.json['data']['attributes'] + assert attributes['access_requests_enabled'] == True - @pytest.fixture() - def url(self, project): - return '/{}nodes/{}/settings/'.format(API_BASE, project._id) + # anyone can comment + project.comment_level = 'public' + project.save() + res = app.get(url, auth=admin_contrib.auth) + attributes = res.json['data']['attributes'] + assert attributes['anyone_can_comment'] == True + + project.comment_level = 'private' + project.save() + res = app.get(url, auth=admin_contrib.auth) + attributes = res.json['data']['attributes'] + assert attributes['anyone_can_comment'] == False + + # wiki enabled + project.delete_addon('wiki', auth=Auth(admin_contrib)) + project.save() + res = app.get(url, auth=admin_contrib.auth) + attributes = res.json['data']['attributes'] + assert attributes['wiki_enabled'] == False + + # redirect link enabled + new_url = 'http://cool.com' + new_label = 'Test Label Woo' + forward = project.add_addon('forward', auth=Auth(admin_contrib)) + forward.url = new_url + forward.label = new_label + forward.save() + res = app.get(url, auth=admin_contrib.auth) + attributes = res.json['data']['attributes'] + assert attributes['redirect_link_enabled'] == True + assert attributes['redirect_link_url'] == new_url + assert attributes['redirect_link_label'] == new_label + + +@pytest.mark.django_db +class TestNodeSettingsUpdate: @pytest.fixture() def payload(self, project): From 98b94b5b823f644fc6e5f71fd14844e071b12d33 Mon Sep 17 00:00:00 2001 From: erinspace Date: Wed, 11 Jul 2018 11:02:29 -0400 Subject: [PATCH 07/18] Add test for view_only_links relationship --- api_tests/nodes/views/test_node_settings.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/api_tests/nodes/views/test_node_settings.py b/api_tests/nodes/views/test_node_settings.py index c281411f874..9d91e53c2e2 100644 --- a/api_tests/nodes/views/test_node_settings.py +++ b/api_tests/nodes/views/test_node_settings.py @@ -5,6 +5,7 @@ from osf_tests.factories import ( AuthUserFactory, ProjectFactory, + PrivateLinkFactory, ) from framework.auth import Auth @@ -100,6 +101,12 @@ def test_node_settings_detail(self, app, admin_contrib, non_contrib, write_contr assert attributes['redirect_link_url'] == new_url assert attributes['redirect_link_label'] == new_label + # view only links + view_only_link = PrivateLinkFactory(name='testlink') + view_only_link.nodes.add(project) + view_only_link.save() + res = app.get(url, auth=admin_contrib.auth) + assert 'view_only_links' in res.json['data']['relationships'].keys() @pytest.mark.django_db class TestNodeSettingsUpdate: From b6cd5f0e5ff81a8d149bc7edf25914edd4486c6f Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Tue, 10 Jul 2018 17:04:42 -0500 Subject: [PATCH 08/18] Add APIv2 update tests for NodeSettings. --- api/nodes/serializers.py | 107 ++++++------ api_tests/nodes/views/test_node_settings.py | 183 +++++++++++++++++++- 2 files changed, 237 insertions(+), 53 deletions(-) diff --git a/api/nodes/serializers.py b/api/nodes/serializers.py index 0837e14b40a..db83f4ef09b 100644 --- a/api/nodes/serializers.py +++ b/api/nodes/serializers.py @@ -1319,9 +1319,6 @@ def update(self, link, validated_data): class NodeSettingsSerializer(JSONAPISerializer): id = IDField(source='_id', read_only=True) type = TypeField() - # Do we need to wrap these fields in HideIfWithdrawal so this serializer - # can be used for registrations? - # TODO deprecate field from NodeSerializer access_requests_enabled = ser.BooleanField() anyone_can_comment = ser.SerializerMethodField() @@ -1375,79 +1372,91 @@ class Meta: class NodeSettingsUpdateSerializer(NodeSettingsSerializer): anyone_can_comment = ser.BooleanField(write_only=True, required=False) - anyone_can_edit_wiki = ser.BooleanField(write_only=True, required=False) wiki_enabled = ser.BooleanField(write_only=True, required=False) + anyone_can_edit_wiki = ser.BooleanField(write_only=True, required=False) redirect_link_enabled = ser.BooleanField(write_only=True, required=False) - redirect_link_url = ser.URLField(read_only=True) - # TODO - add a length restriction - redirect_link_label = ser.CharField() + redirect_link_url = ser.URLField(write_only=True, required=False) + redirect_link_label = ser.CharField(max_length=50, write_only=True, required=False) def update(self, obj, validated_data): user = self.context['request'].user auth = get_user_auth(self.context['request']) - is_admin = obj.has_permission(user, 'admin') - is_read_write = obj.has_permission(user, 'write') - - access_requests_enabled = validated_data.get('access_requests_enabled') - anyone_can_comment = validated_data.get('anyone_can_comment') - anyone_can_edit_wiki = validated_data.get('anyone_can_edit_wiki') - wiki_enabled = validated_data.get('wiki_enabled') - redirect_link_enabled = validated_data.get('redirect_link_enabled') - redirect_link_url = validated_data.get('redirect_link_url') - redirect_link_label = validated_data.get('redirect_link_label') - admin_only_field_names = [ 'access_requests_enabled', 'anyone_can_comment', 'anyone_can_edit_wiki', 'wiki_enabled', ] - save_node = False - if set(validated_data.keys()).intersection(set(admin_only_field_names)) and not is_admin: + + if set(validated_data.keys()).intersection(set(admin_only_field_names)) and not obj.has_permission(user, 'admin'): raise exceptions.PermissionDenied + + self.update_node_fields(obj, validated_data) + self.update_wiki_fields(obj, validated_data, auth) + self.update_forward_fields(obj, validated_data, auth) + return obj + + def update_node_fields(self, obj, validated_data): + access_requests_enabled = validated_data.get('access_requests_enabled') + anyone_can_comment = validated_data.get('anyone_can_comment') + save_node = False + if access_requests_enabled is not None: obj.access_requests_enabled = access_requests_enabled save_node = True if anyone_can_comment is not None: - if anyone_can_comment is True: - obj.comment_level = 'public' - else: - obj.comment_level = 'private' + obj.comment_level = 'public' if anyone_can_comment else 'private' save_node = True if save_node: obj.save() + def update_wiki_fields(self, obj, validated_data, auth): + wiki_enabled = validated_data.get('wiki_enabled') + anyone_can_edit_wiki = validated_data.get('anyone_can_edit_wiki') + wiki_addon = obj.get_addon('wiki') + + if wiki_enabled is not None: + wiki_addon = self.enable_or_disable_addon(obj, wiki_enabled, 'wiki', auth) + if anyone_can_edit_wiki is not None: - wiki_addon = obj.get_addon('wiki') + if not obj.is_public and anyone_can_edit_wiki: + raise exceptions.ValidationError('To allow all OSF users to edit the wiki, the project must be public.') if wiki_addon: - if anyone_can_edit_wiki is True: - wiki_addon.is_publicly_editable = True - else: - wiki_addon.is_publicly_editable = False + wiki_addon.is_publicly_editable = True if anyone_can_edit_wiki else False wiki_addon.save() - # TODO: test me well! + else: + raise exceptions.ValidationError('You must have the wiki enabled before changing wiki settings.') + + def update_forward_fields(self, obj, validated_data, auth): + redirect_link_enabled = validated_data.get('redirect_link_enabled') + redirect_link_url = validated_data.get('redirect_link_url') + redirect_link_label = validated_data.get('redirect_link_label') + + save_forward = False + forward_addon = obj.get_addon('forward') + if redirect_link_enabled is not None: if not redirect_link_url and redirect_link_enabled: - raise ValidationError('You must include a redirect URL to enable a redirect.') - if redirect_link_enabled: - forward_addon = obj.get_or_add_addon('forward') - forward_addon.url = redirect_link_url - if redirect_link_label: - forward_addon.label = redirect_link_label - forward_addon.save() + raise exceptions.ValidationError('You must include a redirect URL to enable a redirect.') + forward_addon = self.enable_or_disable_addon(obj, redirect_link_enabled, 'forward', auth) + if redirect_link_url is not None: - forward_addon = obj.get_addon('forward') if not forward_addon: - raise ValidationError('You must first set redirect_link_enabled to True before specifying a redirect link URL.') - elif not redirect_link_enabled: - forward_addon.url = redirect_link_url - forward_addon.save() + raise exceptions.ValidationError('You must first set redirect_link_enabled to True before specifying a redirect link URL.') + forward_addon.url = redirect_link_url + save_forward = True + if redirect_link_label is not None: - forward_addon = obj.get_addon('forward') if not forward_addon: - raise ValidationError('You must first set redirect_link_enabled to True before specifying a redirect link URL.') - elif not redirect_link_enabled: - forward_addon.label = redirect_link_label - forward_addon.save() - - return obj + raise exceptions.ValidationError('You must first set redirect_link_enabled to True before specifying a redirect link label.') + forward_addon.label = redirect_link_label + save_forward = True + + if save_forward: + forward_addon.save() + + def enable_or_disable_addon(self, obj, should_enable, addon_name, auth): + addon = obj.get_or_add_addon(addon_name, auth=auth) if should_enable else obj.delete_addon(addon_name, auth) + if type(addon) == bool: + addon = None + return addon diff --git a/api_tests/nodes/views/test_node_settings.py b/api_tests/nodes/views/test_node_settings.py index fcf4bba77a7..76c24a20dd1 100644 --- a/api_tests/nodes/views/test_node_settings.py +++ b/api_tests/nodes/views/test_node_settings.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- -import mock import pytest +from framework.auth import Auth from api.base.settings.defaults import API_BASE from osf_tests.factories import ( AuthUserFactory, @@ -27,8 +27,8 @@ def read_contrib(self): @pytest.fixture() def project(self, admin_contrib, write_contrib, read_contrib): project = ProjectFactory(creator=admin_contrib) - project.add_contributor(write_contrib, 'write') - project.add_contributor(read_contrib, 'write') + project.add_contributor(write_contrib, ['write', 'read']) + project.add_contributor(read_contrib, ['read']) project.save() return project @@ -47,7 +47,6 @@ def payload(self, project): } } - def test_patch_permissions(self, app, project, payload, admin_contrib, write_contrib, read_contrib, url): payload['data']['attributes']['redirect_link_enabled'] = True payload['data']['attributes']['redirect_link_url'] = 'https://cos.io' @@ -66,8 +65,184 @@ def test_patch_permissions(self, app, project, payload, admin_contrib, write_con # Logged in write (Write contribs can only change some node settings) res = app.patch_json_api(url, payload, auth=write_contrib.auth, expect_errors=True) + assert res.status_code == 200 + + # Logged in admin + res = app.patch_json_api(url, payload, auth=admin_contrib.auth) + assert res.status_code == 200 + + def test_patch_invalid_type(self, app, project, payload, admin_contrib, url): + payload['data']['type'] = 'Invalid Type' + + # Logged in admin + res = app.patch_json_api(url, payload, auth=admin_contrib.auth, expect_errors=True) + assert res.status_code == 409 + + def test_patch_access_requests_enabled(self, app, project, payload, admin_contrib, write_contrib, url): + assert project.access_requests_enabled is True + payload['data']['attributes']['access_requests_enabled'] = False + + # Write cannot modify this field + res = app.patch_json_api(url, payload, auth=write_contrib.auth, expect_errors=True) + assert res.status_code == 403 + + # Logged in admin + res = app.patch_json_api(url, payload, auth=admin_contrib.auth) + assert res.status_code == 200 + project.reload() + assert project.access_requests_enabled is False + + payload['data']['attributes']['access_requests_enabled'] = True + # Logged in admin + res = app.patch_json_api(url, payload, auth=admin_contrib.auth) + assert res.status_code == 200 + project.reload() + assert project.access_requests_enabled is True + + def test_patch_anyone_can_comment(self, app, project, payload, admin_contrib, write_contrib, url): + assert project.comment_level == 'public' + payload['data']['attributes']['anyone_can_comment'] = False + + # Write cannot modify this field + res = app.patch_json_api(url, payload, auth=write_contrib.auth, expect_errors=True) + assert res.status_code == 403 + + # Logged in admin + res = app.patch_json_api(url, payload, auth=admin_contrib.auth) + assert res.status_code == 200 + project.reload() + assert project.comment_level == 'private' + + payload['data']['attributes']['anyone_can_comment'] = True + # Logged in admin + res = app.patch_json_api(url, payload, auth=admin_contrib.auth) + assert res.status_code == 200 + project.reload() + assert project.comment_level == 'public' + + def test_patch_anyone_can_edit_wiki(self, app, project, payload, admin_contrib, write_contrib, url): + project.is_public = True + project.save() + wiki_addon = project.get_addon('wiki') + assert wiki_addon.is_publicly_editable is False + payload['data']['attributes']['anyone_can_edit_wiki'] = True + + # Write cannot modify this field + res = app.patch_json_api(url, payload, auth=write_contrib.auth, expect_errors=True) assert res.status_code == 403 # Logged in admin res = app.patch_json_api(url, payload, auth=admin_contrib.auth) assert res.status_code == 200 + wiki_addon.reload() + assert wiki_addon.is_publicly_editable is True + + payload['data']['attributes']['anyone_can_edit_wiki'] = False + # Logged in admin + res = app.patch_json_api(url, payload, auth=admin_contrib.auth) + assert res.status_code == 200 + wiki_addon.reload() + assert wiki_addon.is_publicly_editable is False + + # Test wiki disabled in same request so cannot change wiki_settings + payload['data']['attributes']['wiki_enabled'] = False + res = app.patch_json_api(url, payload, auth=admin_contrib.auth, expect_errors=True) + assert res.status_code == 400 + + # Test wiki disabled so cannot change wiki settings + project.delete_addon('wiki', Auth(admin_contrib)) + res = app.patch_json_api(url, payload, auth=admin_contrib.auth, expect_errors=True) + assert res.status_code == 400 + + # Test wiki enabled so can change wiki settings + payload['data']['attributes']['wiki_enabled'] = True + payload['data']['attributes']['anyone_can_edit_wiki'] = True + res = app.patch_json_api(url, payload, auth=admin_contrib.auth, expect_errors=True) + assert res.status_code == 200 + assert project.get_addon('wiki').is_publicly_editable is True + + # If project is private, cannot change settings to allow anyone to edit wiki + project.is_public = False + project.save() + res = app.patch_json_api(url, payload, auth=admin_contrib.auth, expect_errors=True) + assert res.status_code == 400 + assert res.json['errors'][0]['detail'] == 'To allow all OSF users to edit the wiki, the project must be public.' + + def test_patch_wiki_enabled(self, app, project, payload, admin_contrib, write_contrib, url): + assert project.get_addon('wiki') is not None + payload['data']['attributes']['wiki_enabled'] = False + + # Write cannot modify this field + res = app.patch_json_api(url, payload, auth=write_contrib.auth, expect_errors=True) + assert res.status_code == 403 + + # Logged in admin + res = app.patch_json_api(url, payload, auth=admin_contrib.auth) + assert res.status_code == 200 + assert project.get_addon('wiki') is None + + # Nothing happens if attempting to disable an already-disabled wiki + res = app.patch_json_api(url, payload, auth=admin_contrib.auth) + assert res.status_code == 200 + assert project.get_addon('wiki') is None + + payload['data']['attributes']['wiki_enabled'] = True + # Logged in admin + res = app.patch_json_api(url, payload, auth=admin_contrib.auth) + assert res.status_code == 200 + assert project.get_addon('wiki') is not None + + # Nothing happens if attempting to enable an already-enabled-wiki + res = app.patch_json_api(url, payload, auth=admin_contrib.auth) + assert res.status_code == 200 + assert project.get_addon('wiki') is not None + + def test_redirect_link_enabled(self, app, project, payload, admin_contrib, write_contrib, url): + assert project.get_addon('forward') is None + payload['data']['attributes']['redirect_link_enabled'] = True + + # Redirect link not included + res = app.patch_json_api(url, payload, auth=admin_contrib.auth, expect_errors=True) + assert res.status_code == 400 + assert res.json['errors'][0]['detail'] == 'You must include a redirect URL to enable a redirect.' + + payload['data']['attributes']['redirect_link_url'] = 'https://cos.io' + payload['data']['attributes']['redirect_link_label'] = 'My Link' + # Write contrib can modify forward related fields + res = app.patch_json_api(url, payload, auth=write_contrib.auth) + assert res.status_code == 200 + forward_addon = project.get_addon('forward') + assert forward_addon is not None + assert forward_addon.url == 'https://cos.io' + assert forward_addon.label == 'My Link' + + # Attempting to set redirect_link_url when redirect_link not enabled + payload['data']['attributes']['redirect_link_enabled'] = False + del payload['data']['attributes']['redirect_link_label'] + res = app.patch_json_api(url, payload, auth=admin_contrib.auth, expect_errors=True) + assert res.status_code == 400 + assert res.json['errors'][0]['detail'] == 'You must first set redirect_link_enabled to True before specifying a redirect link URL.' + + # Attempting to set redirect_link_label when redirect_link not enabled + payload['data']['attributes']['redirect_link_enabled'] = False + del payload['data']['attributes']['redirect_link_url'] + payload['data']['attributes']['redirect_link_label'] = 'My Link' + res = app.patch_json_api(url, payload, auth=admin_contrib.auth, expect_errors=True) + assert res.status_code == 400 + assert res.json['errors'][0]['detail'] == 'You must first set redirect_link_enabled to True before specifying a redirect link label.' + + payload['data']['attributes']['redirect_link_enabled'] = False + del payload['data']['attributes']['redirect_link_label'] + res = app.patch_json_api(url, payload, auth=admin_contrib.auth) + assert res.status_code == 200 + forward_addon = project.get_addon('forward') + assert forward_addon is None + + def test_redirect_link_label_char_limit(self, app, project, payload, admin_contrib, url): + project.add_addon('forward', ()) + project.save() + + payload['data']['attributes']['redirect_link_label'] = 'a' * 52 + res = app.patch_json_api(url, payload, auth=admin_contrib.auth, expect_errors=True) + assert res.status_code == 400 + assert res.json['errors'][0]['detail'] == 'Ensure this field has no more than 50 characters.' From 9c2026ef4e484238baca8f4e16dce305f5dfd803 Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Wed, 11 Jul 2018 10:53:06 -0500 Subject: [PATCH 09/18] Add version 2.9 to API and add to serializer context so addons aren't loaded multiple times. - Version 2.9 deprecates access_requests_enabled field from NodeSerializer and moves it to NodeSettings serializer. - Wiki and forward addons pushed to serializer context so they are not loaded multiple times in serializer method fields and in update method. --- api/base/settings/defaults.py | 1 + api/nodes/serializers.py | 25 ++++++++++++--------- api/nodes/views.py | 11 +++++++++ api_tests/nodes/views/test_node_settings.py | 25 +++++++++++++-------- 4 files changed, 42 insertions(+), 20 deletions(-) diff --git a/api/base/settings/defaults.py b/api/base/settings/defaults.py index cfa5cf546e6..777bfca46a5 100644 --- a/api/base/settings/defaults.py +++ b/api/base/settings/defaults.py @@ -154,6 +154,7 @@ '2.6', '2.7', '2.8', + '2.9', ), 'DEFAULT_FILTER_BACKENDS': ('api.base.filters.OSFOrderingFilter',), 'DEFAULT_PAGINATION_CLASS': 'api.base.pagination.JSONAPIPagination', diff --git a/api/nodes/serializers.py b/api/nodes/serializers.py index db83f4ef09b..0d152b094d1 100644 --- a/api/nodes/serializers.py +++ b/api/nodes/serializers.py @@ -209,7 +209,7 @@ class NodeSerializer(TaxonomizableSerializerMixin, JSONAPISerializer): fork = ser.BooleanField(read_only=True, source='is_fork') collection = ser.BooleanField(read_only=True, source='is_collection') tags = ValuesListField(attr_name='name', child=ser.CharField(), required=False) - access_requests_enabled = ser.BooleanField(read_only=False, required=False) + access_requests_enabled = ShowIfVersion(ser.BooleanField(read_only=False, required=False), min_version='2.0', max_version='2.8') node_license = NodeLicenseSerializer(required=False, source='license') analytics_key = ShowIfAdminScopeOrAnonymous(ser.CharField(read_only=True, source='keenio_read_key')) template_from = ser.CharField(required=False, allow_blank=False, allow_null=False, @@ -1319,7 +1319,6 @@ def update(self, link, validated_data): class NodeSettingsSerializer(JSONAPISerializer): id = IDField(source='_id', read_only=True) type = TypeField() - # TODO deprecate field from NodeSerializer access_requests_enabled = ser.BooleanField() anyone_can_comment = ser.SerializerMethodField() anyone_can_edit_wiki = ser.SerializerMethodField() @@ -1327,6 +1326,7 @@ class NodeSettingsSerializer(JSONAPISerializer): redirect_link_enabled = ser.SerializerMethodField() redirect_link_url = ser.SerializerMethodField() redirect_link_label = ser.SerializerMethodField() + view_only_links = RelationshipField( related_view='nodes:node-view-only-links', related_view_kwargs={'node_id': '<_id>'}, @@ -1339,22 +1339,22 @@ class NodeSettingsSerializer(JSONAPISerializer): def get_anyone_can_comment(self, obj): return obj.comment_level == 'public' + def get_wiki_enabled(self, obj): + return self.context['wiki_addon'] is not None + def get_anyone_can_edit_wiki(self, obj): - wiki_addon = obj.get_addon('wiki') + wiki_addon = self.context['wiki_addon'] return wiki_addon.is_publicly_editable if wiki_addon else None - def get_wiki_enabled(self, obj): - return 'wiki' in obj.get_addon_names() - def get_redirect_link_enabled(self, obj): - return 'forward' in obj.get_addon_names() + return self.context['forward_addon'] is not None def get_redirect_link_url(self, obj): - forward_addon = obj.get_addon('forward') + forward_addon = self.context['forward_addon'] return forward_addon.url if forward_addon else None def get_redirect_link_label(self, obj): - forward_addon = obj.get_addon('forward') + forward_addon = self.context['forward_addon'] return forward_addon.label if forward_addon else None def get_absolute_url(self, obj): @@ -1413,7 +1413,7 @@ def update_node_fields(self, obj, validated_data): def update_wiki_fields(self, obj, validated_data, auth): wiki_enabled = validated_data.get('wiki_enabled') anyone_can_edit_wiki = validated_data.get('anyone_can_edit_wiki') - wiki_addon = obj.get_addon('wiki') + wiki_addon = self.context['wiki_addon'] if wiki_enabled is not None: wiki_addon = self.enable_or_disable_addon(obj, wiki_enabled, 'wiki', auth) @@ -1433,7 +1433,7 @@ def update_forward_fields(self, obj, validated_data, auth): redirect_link_label = validated_data.get('redirect_link_label') save_forward = False - forward_addon = obj.get_addon('forward') + forward_addon = self.context['forward_addon'] if redirect_link_enabled is not None: if not redirect_link_url and redirect_link_enabled: @@ -1456,6 +1456,9 @@ def update_forward_fields(self, obj, validated_data, auth): forward_addon.save() def enable_or_disable_addon(self, obj, should_enable, addon_name, auth): + """ + Returns addon, if exists, otherwise returns None + """ addon = obj.get_or_add_addon(addon_name, auth=auth) if should_enable else obj.delete_addon(addon_name, auth) if type(addon) == bool: addon = None diff --git a/api/nodes/views.py b/api/nodes/views.py index 0a999addff0..1ec9e173ba3 100644 --- a/api/nodes/views.py +++ b/api/nodes/views.py @@ -1937,3 +1937,14 @@ def get_serializer_class(self): if self.request.method == 'PUT' or self.request.method == 'PATCH': return NodeSettingsUpdateSerializer return NodeSettingsSerializer + + def get_serializer_context(self): + """ + Extra context for NodeSettingsSerializer - this will prevent loading + addons multiple times in SerializerMethodFields + """ + context = super(NodeSettings, self).get_serializer_context() + node = self.get_node(check_object_permissions=False) + context['wiki_addon'] = node.get_addon('wiki') + context['forward_addon'] = node.get_addon('forward') + return context diff --git a/api_tests/nodes/views/test_node_settings.py b/api_tests/nodes/views/test_node_settings.py index 43d877a7353..e5bb40fc264 100644 --- a/api_tests/nodes/views/test_node_settings.py +++ b/api_tests/nodes/views/test_node_settings.py @@ -8,9 +8,6 @@ PrivateLinkFactory, ) -from framework.auth import Auth - - @pytest.fixture() def admin_contrib(): return AuthUserFactory() @@ -37,7 +34,7 @@ def url(project): @pytest.mark.django_db -class TestGetNodeSettingsGet: +class TestNodeSettingsGet: @pytest.fixture() def non_contrib(self): @@ -66,29 +63,39 @@ def test_node_settings_detail(self, app, admin_contrib, non_contrib, write_contr project.save() res = app.get(url, auth=admin_contrib.auth) attributes = res.json['data']['attributes'] - assert attributes['access_requests_enabled'] == True + assert attributes['access_requests_enabled'] is True # anyone can comment project.comment_level = 'public' project.save() res = app.get(url, auth=admin_contrib.auth) attributes = res.json['data']['attributes'] - assert attributes['anyone_can_comment'] == True + assert attributes['anyone_can_comment'] is True project.comment_level = 'private' project.save() res = app.get(url, auth=admin_contrib.auth) attributes = res.json['data']['attributes'] - assert attributes['anyone_can_comment'] == False + assert attributes['anyone_can_comment'] is False # wiki enabled + res = app.get(url, auth=admin_contrib.auth) + attributes = res.json['data']['attributes'] + assert attributes['wiki_enabled'] is True + project.delete_addon('wiki', auth=Auth(admin_contrib)) project.save() res = app.get(url, auth=admin_contrib.auth) attributes = res.json['data']['attributes'] - assert attributes['wiki_enabled'] == False + assert attributes['wiki_enabled'] is False # redirect link enabled + res = app.get(url, auth=admin_contrib.auth) + attributes = res.json['data']['attributes'] + assert attributes['redirect_link_enabled'] is False + assert attributes['redirect_link_url'] is None + assert attributes['redirect_link_label'] is None + new_url = 'http://cool.com' new_label = 'Test Label Woo' forward = project.add_addon('forward', auth=Auth(admin_contrib)) @@ -97,7 +104,7 @@ def test_node_settings_detail(self, app, admin_contrib, non_contrib, write_contr forward.save() res = app.get(url, auth=admin_contrib.auth) attributes = res.json['data']['attributes'] - assert attributes['redirect_link_enabled'] == True + assert attributes['redirect_link_enabled'] is True assert attributes['redirect_link_url'] == new_url assert attributes['redirect_link_label'] == new_label From 2e4330838cc58951ccfc6b6c3dea21cb8ad55bd5 Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Wed, 11 Jul 2018 11:21:50 -0500 Subject: [PATCH 10/18] Expose settings relationship on Node serializer only. --- api/nodes/serializers.py | 5 +++++ api/registrations/serializers.py | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/api/nodes/serializers.py b/api/nodes/serializers.py index 0d152b094d1..a2583b69ba9 100644 --- a/api/nodes/serializers.py +++ b/api/nodes/serializers.py @@ -270,6 +270,11 @@ class NodeSerializer(TaxonomizableSerializerMixin, JSONAPISerializer): related_view_kwargs={'node_id': '<_id>'} ) + settings = RelationshipField( + related_view='nodes:node-settings', + related_view_kwargs={'node_id': '<_id>'} + ) + wikis = HideIfWikiDisabled(RelationshipField( related_view='nodes:node-wikis', related_view_kwargs={'node_id': '<_id>'} diff --git a/api/registrations/serializers.py b/api/registrations/serializers.py index b4876bad99b..613c7d9fb52 100644 --- a/api/registrations/serializers.py +++ b/api/registrations/serializers.py @@ -180,6 +180,11 @@ class BaseRegistrationSerializer(NodeSerializer): related_view_kwargs={'metaschema_id': ''} ) + settings = HideIfRegistration(RelationshipField( + related_view='nodes:node-settings', + related_view_kwargs={'node_id': '<_id>'} + )) + registrations = HideIfRegistration(RelationshipField( related_view='nodes:node-registrations', related_view_kwargs={'node_id': '<_id>'} From ab339469d4c8a96b1ea4f131346e60eec846b1cd Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Wed, 11 Jul 2018 12:42:10 -0500 Subject: [PATCH 11/18] Modify is_deprecated so only a min_version or max_version can be specified, if desired. - Modify HideIfWikiDisabled so versions greater than 2.7 don't show wiki relationship on nodes if wiki is disabled. Formerly, this feature was just restricted to 2.8. - List settings field as field that doesn't appear on registration serializer. - Add test for deprecated access_requests_enabled field on NodeSerializer --- api/base/serializers.py | 4 ++-- api/base/utils.py | 6 ++++-- api_tests/base/test_serializers.py | 2 +- api_tests/nodes/views/test_node_detail.py | 7 +++++++ 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/api/base/serializers.py b/api/base/serializers.py index 33b7b9f7371..96bda4a36f4 100644 --- a/api/base/serializers.py +++ b/api/base/serializers.py @@ -186,12 +186,12 @@ def should_hide(self, instance): class HideIfWikiDisabled(ConditionalField): """ - If wiki is disabled, don't show relationship field, only available in version 2.8 + If wiki is disabled, don't show relationship field, only available after 2.7 """ def should_hide(self, instance): request = self.context.get('request') - return not utils.is_deprecated(request.version, '2.8', '2.8') and not instance.has_addon('wiki') + return not utils.is_deprecated(request.version, min_version='2.8') and not instance.has_addon('wiki') class HideIfNotNodePointerLog(ConditionalField): diff --git a/api/base/utils.py b/api/base/utils.py index 78cdceeef80..babad0b4ec6 100644 --- a/api/base/utils.py +++ b/api/base/utils.py @@ -171,8 +171,10 @@ def has_admin_scope(request): return set(ComposedScopes.ADMIN_LEVEL).issubset(normalize_scopes(token.attributes['accessTokenScope'])) -def is_deprecated(request_version, min_version, max_version): - if request_version < min_version or request_version > max_version: +def is_deprecated(request_version, min_version=None, max_version=None): + if not min_version and not max_version: + raise NotImplementedError('Must specify min or max version.') + if min_version and request_version < min_version or max_version and request_version > max_version: return True return False diff --git a/api_tests/base/test_serializers.py b/api_tests/base/test_serializers.py index c2bae063793..c67b2eaacf4 100644 --- a/api_tests/base/test_serializers.py +++ b/api_tests/base/test_serializers.py @@ -134,7 +134,7 @@ def test_registration_serializer(self): 'preprint', 'subjects'] # fields that do not appear on registrations - non_registration_fields = ['registrations', 'draft_registrations', 'templated_by_count'] + non_registration_fields = ['registrations', 'draft_registrations', 'templated_by_count', 'settings'] for field in NodeSerializer._declared_fields: assert_in(field, RegistrationSerializer._declared_fields) diff --git a/api_tests/nodes/views/test_node_detail.py b/api_tests/nodes/views/test_node_detail.py index 1e2a11d9f1f..641d2bf6fc5 100644 --- a/api_tests/nodes/views/test_node_detail.py +++ b/api_tests/nodes/views/test_node_detail.py @@ -310,6 +310,13 @@ def test_node_shows_wiki_relationship_based_on_disabled_status_and_version(self, res = app.get(url, auth=user.auth) assert 'wikis' in res.json['data']['relationships'] + def test_shows_access_requests_enabled_field_based_on_version(self, app, user, project_public, url_public): + url = url_public + '?version=latest' + res = app.get(url, auth=user.auth) + assert 'access_requests_enabled' not in res.json['data']['attributes'] + res = app.get(url_public + '?version=2.8', auth=user.auth) + assert 'access_requests_enabled' in res.json['data']['attributes'] + def test_node_shows_correct_templated_from_count(self, app, user, project_public, url_public): url = url_public res = app.get(url) From c21ac616a18830615decbfcb07e1459ecffddc20 Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Fri, 13 Jul 2018 11:52:32 -0500 Subject: [PATCH 12/18] Add PUT permissions tests for APIv2 NodeSettings. --- api_tests/nodes/views/test_node_settings.py | 40 +++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/api_tests/nodes/views/test_node_settings.py b/api_tests/nodes/views/test_node_settings.py index e5bb40fc264..6f0e0300421 100644 --- a/api_tests/nodes/views/test_node_settings.py +++ b/api_tests/nodes/views/test_node_settings.py @@ -116,6 +116,46 @@ def test_node_settings_detail(self, app, admin_contrib, non_contrib, write_contr assert 'view_only_links' in res.json['data']['relationships'].keys() +@pytest.mark.django_db +class TestNodeSettingsPUT: + @pytest.fixture() + def payload(self, project): + return { + 'data': { + 'id': project._id, + 'type': 'node-settings', + 'attributes': { + 'redirect_link_enabled': True, + 'redirect_link_url': 'https://cos.io' + } + } + } + + def test_put_permissions(self, app, project, payload, admin_contrib, write_contrib, read_contrib, url): + assert project.access_requests_enabled is True + payload['data']['attributes']['access_requests_enabled'] = False + # Logged out + res = app.put_json_api(url, payload, expect_errors=True) + assert res.status_code == 401 + + # Logged in, noncontrib + noncontrib = AuthUserFactory() + res = app.put_json_api(url, payload, auth=noncontrib.auth, expect_errors=True) + assert res.status_code == 403 + + # Logged in read + res = app.put_json_api(url, payload, auth=read_contrib.auth, expect_errors=True) + assert res.status_code == 403 + + # Logged in write (Write contribs can only change some node settings) + res = app.put_json_api(url, payload, auth=write_contrib.auth, expect_errors=True) + assert res.status_code == 403 + + # Logged in admin + res = app.put_json_api(url, payload, auth=admin_contrib.auth) + assert res.status_code == 200 + + @pytest.mark.django_db class TestNodeSettingsUpdate: From fb7db323dafac946225c28ec522afe9075fa234b Mon Sep 17 00:00:00 2001 From: erinspace Date: Wed, 18 Jul 2018 11:22:13 -0400 Subject: [PATCH 13/18] Move method for setting access requests to node model [#PLAT-824] - Keep old v1 route - Add model tests --- osf/models/node.py | 28 ++++++++++++++++++++++++++++ osf_tests/test_node.py | 16 ++++++++++++++++ website/project/views/node.py | 24 ++---------------------- 3 files changed, 46 insertions(+), 22 deletions(-) diff --git a/osf/models/node.py b/osf/models/node.py index 84c2098dad6..4e441fc933d 100644 --- a/osf/models/node.py +++ b/osf/models/node.py @@ -2378,6 +2378,34 @@ def update_contributor(self, user, permission, visible, auth, save=False): self.set_visible(user, visible, auth=auth) self.save_node_preprints() + def set_access_requests_enabled(self, access_requests_enabled, auth, save=False): + user = auth.user + if not self.has_permission(user, ADMIN): + raise PermissionsError('Only admins can modify access requests enabled') + self.access_requests_enabled = access_requests_enabled + if self.access_requests_enabled: + self.add_log( + NodeLog.NODE_ACCESS_REQUESTS_ENABLED, + { + 'project': self.parent_id, + 'node': self._id, + 'user': user._id, + }, + auth=auth + ) + else: + self.add_log( + NodeLog.NODE_ACCESS_REQUESTS_DISABLED, + { + 'project': self.parent_id, + 'node': self._id, + 'user': user._id, + }, + auth=auth + ) + if save: + self.save() + def save(self, *args, **kwargs): first_save = not bool(self.pk) if 'old_subjects' in kwargs.keys(): diff --git a/osf_tests/test_node.py b/osf_tests/test_node.py index 4d639c65aa9..7375ab7e7ee 100644 --- a/osf_tests/test_node.py +++ b/osf_tests/test_node.py @@ -3303,6 +3303,22 @@ def test_set_description(self, node, auth): assert latest_log.params['description_original'], old_desc assert latest_log.params['description_new'], 'new description' + def test_set_access_requests(self, node, auth): + assert node.access_requests_enabled is True + node.set_access_requests_enabled(False, auth=auth, save=True) + assert node.access_requests_enabled is False + assert node.logs.latest().action == NodeLog.NODE_ACCESS_REQUESTS_DISABLED + + node.set_access_requests_enabled(True, auth=auth, save=True) + assert node.access_requests_enabled is True + assert node.logs.latest().action == NodeLog.NODE_ACCESS_REQUESTS_ENABLED + + def test_set_access_requests_non_admin(self, node, auth): + contrib = AuthUserFactory() + Contributor.objects.create(user=contrib, node=node, write=True, read=True, visible=True) + with pytest.raises(PermissionsError): + node.set_access_requests_enabled(True, auth=Auth(contrib)) + def test_validate_categories(self): with pytest.raises(ValidationError): Node(category='invalid').save() # an invalid category diff --git a/website/project/views/node.py b/website/project/views/node.py index dc6241b0438..f9bed3e9a67 100644 --- a/website/project/views/node.py +++ b/website/project/views/node.py @@ -398,28 +398,8 @@ def configure_comments(node, **kwargs): @must_not_be_registration def configure_requests(node, **kwargs): access_requests_enabled = request.get_json().get('accessRequestsEnabled') - node.access_requests_enabled = access_requests_enabled - if node.access_requests_enabled: - node.add_log( - NodeLog.NODE_ACCESS_REQUESTS_ENABLED, - { - 'project': node.parent_id, - 'node': node._id, - 'user': kwargs.get('auth').user._id, - }, - auth=kwargs.get('auth', None) - ) - else: - node.add_log( - NodeLog.NODE_ACCESS_REQUESTS_DISABLED, - { - 'project': node.parent_id, - 'node': node._id, - 'user': kwargs.get('auth').user._id, - }, - auth=kwargs.get('auth', None) - ) - node.save() + auth = kwargs.get('auth', None) + node.set_access_requests_enabled(access_requests_enabled, auth, save=True) return {'access_requests_enabled': access_requests_enabled}, 200 From a3468e51557517858648916e587f034793392602 Mon Sep 17 00:00:00 2001 From: erinspace Date: Wed, 18 Jul 2018 13:42:18 -0400 Subject: [PATCH 14/18] Use new access_requests_enabled method on node, add tests for logging --- api/nodes/serializers.py | 3 ++- api_tests/nodes/views/test_node_settings.py | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/api/nodes/serializers.py b/api/nodes/serializers.py index a2583b69ba9..3a6e11fb001 100644 --- a/api/nodes/serializers.py +++ b/api/nodes/serializers.py @@ -1402,12 +1402,13 @@ def update(self, obj, validated_data): return obj def update_node_fields(self, obj, validated_data): + auth = get_user_auth(self.context['request']) access_requests_enabled = validated_data.get('access_requests_enabled') anyone_can_comment = validated_data.get('anyone_can_comment') save_node = False if access_requests_enabled is not None: - obj.access_requests_enabled = access_requests_enabled + obj.set_access_requests_enabled(access_requests_enabled, auth=auth) save_node = True if anyone_can_comment is not None: obj.comment_level = 'public' if anyone_can_comment else 'private' diff --git a/api_tests/nodes/views/test_node_settings.py b/api_tests/nodes/views/test_node_settings.py index 6f0e0300421..c832029c76a 100644 --- a/api_tests/nodes/views/test_node_settings.py +++ b/api_tests/nodes/views/test_node_settings.py @@ -7,6 +7,7 @@ ProjectFactory, PrivateLinkFactory, ) +from osf.models import NodeLog @pytest.fixture() def admin_contrib(): @@ -214,6 +215,7 @@ def test_patch_access_requests_enabled(self, app, project, payload, admin_contri assert res.status_code == 200 project.reload() assert project.access_requests_enabled is False + assert project.logs.latest().action == NodeLog.NODE_ACCESS_REQUESTS_DISABLED payload['data']['attributes']['access_requests_enabled'] = True # Logged in admin @@ -221,6 +223,7 @@ def test_patch_access_requests_enabled(self, app, project, payload, admin_contri assert res.status_code == 200 project.reload() assert project.access_requests_enabled is True + assert project.logs.latest().action == NodeLog.NODE_ACCESS_REQUESTS_ENABLED def test_patch_anyone_can_comment(self, app, project, payload, admin_contrib, write_contrib, url): assert project.comment_level == 'public' From a6733d0d71e7b3a33d4ad0f954fe41977ac3efa8 Mon Sep 17 00:00:00 2001 From: erinspace Date: Wed, 18 Jul 2018 15:21:21 -0400 Subject: [PATCH 15/18] Check for logging on other node and wiki actions that have associated logs Update tests to check for node logs --- api/nodes/serializers.py | 11 ++++++++++- api_tests/nodes/views/test_node_settings.py | 4 ++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/api/nodes/serializers.py b/api/nodes/serializers.py index 3a6e11fb001..68dcf663f5d 100644 --- a/api/nodes/serializers.py +++ b/api/nodes/serializers.py @@ -1428,7 +1428,7 @@ def update_wiki_fields(self, obj, validated_data, auth): if not obj.is_public and anyone_can_edit_wiki: raise exceptions.ValidationError('To allow all OSF users to edit the wiki, the project must be public.') if wiki_addon: - wiki_addon.is_publicly_editable = True if anyone_can_edit_wiki else False + wiki_addon.set_editing(permissions=anyone_can_edit_wiki, auth=auth, log=True) wiki_addon.save() else: raise exceptions.ValidationError('You must have the wiki enabled before changing wiki settings.') @@ -1450,6 +1450,15 @@ def update_forward_fields(self, obj, validated_data, auth): if not forward_addon: raise exceptions.ValidationError('You must first set redirect_link_enabled to True before specifying a redirect link URL.') forward_addon.url = redirect_link_url + obj.add_log( + action='forward_url_changed', + params=dict( + node=obj._id, + project=obj.parent_id, + forward_url=redirect_link_url, + ), + auth=auth + ) save_forward = True if redirect_link_label is not None: diff --git a/api_tests/nodes/views/test_node_settings.py b/api_tests/nodes/views/test_node_settings.py index c832029c76a..c6526dcd911 100644 --- a/api_tests/nodes/views/test_node_settings.py +++ b/api_tests/nodes/views/test_node_settings.py @@ -262,6 +262,7 @@ def test_patch_anyone_can_edit_wiki(self, app, project, payload, admin_contrib, assert res.status_code == 200 wiki_addon.reload() assert wiki_addon.is_publicly_editable is True + assert project.logs.latest().action == NodeLog.MADE_WIKI_PUBLIC payload['data']['attributes']['anyone_can_edit_wiki'] = False # Logged in admin @@ -269,6 +270,7 @@ def test_patch_anyone_can_edit_wiki(self, app, project, payload, admin_contrib, assert res.status_code == 200 wiki_addon.reload() assert wiki_addon.is_publicly_editable is False + assert project.logs.latest().action == NodeLog.MADE_WIKI_PRIVATE # Test wiki disabled in same request so cannot change wiki_settings payload['data']['attributes']['wiki_enabled'] = False @@ -286,6 +288,7 @@ def test_patch_anyone_can_edit_wiki(self, app, project, payload, admin_contrib, res = app.patch_json_api(url, payload, auth=admin_contrib.auth, expect_errors=True) assert res.status_code == 200 assert project.get_addon('wiki').is_publicly_editable is True + assert project.logs.latest().action == NodeLog.MADE_WIKI_PUBLIC # If project is private, cannot change settings to allow anyone to edit wiki project.is_public = False @@ -341,6 +344,7 @@ def test_redirect_link_enabled(self, app, project, payload, admin_contrib, write assert forward_addon is not None assert forward_addon.url == 'https://cos.io' assert forward_addon.label == 'My Link' + assert project.logs.latest().action == 'forward_url_changed' # Attempting to set redirect_link_url when redirect_link not enabled payload['data']['attributes']['redirect_link_enabled'] = False From d74bf6a523c9009b962bb8578e8be8a735e35d7c Mon Sep 17 00:00:00 2001 From: erinspace Date: Wed, 18 Jul 2018 15:22:41 -0400 Subject: [PATCH 16/18] Make update methods consistent in passing auth --- api/nodes/serializers.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/api/nodes/serializers.py b/api/nodes/serializers.py index 68dcf663f5d..526ad4f03cf 100644 --- a/api/nodes/serializers.py +++ b/api/nodes/serializers.py @@ -1396,13 +1396,12 @@ def update(self, obj, validated_data): if set(validated_data.keys()).intersection(set(admin_only_field_names)) and not obj.has_permission(user, 'admin'): raise exceptions.PermissionDenied - self.update_node_fields(obj, validated_data) + self.update_node_fields(obj, validated_data, auth) self.update_wiki_fields(obj, validated_data, auth) self.update_forward_fields(obj, validated_data, auth) return obj - def update_node_fields(self, obj, validated_data): - auth = get_user_auth(self.context['request']) + def update_node_fields(self, obj, validated_data, auth): access_requests_enabled = validated_data.get('access_requests_enabled') anyone_can_comment = validated_data.get('anyone_can_comment') save_node = False From bee8d6ab9ca7d07f0e74362354f0640f307e34f0 Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Thu, 19 Jul 2018 11:36:54 -0500 Subject: [PATCH 17/18] Override to_representation on NodeSettingsUpdateSerializer to use a different serializer for the response versus the request. --- api/nodes/serializers.py | 11 +++++++++ api_tests/nodes/views/test_node_settings.py | 26 +++++++++++++++++---- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/api/nodes/serializers.py b/api/nodes/serializers.py index 526ad4f03cf..fa2df46ead6 100644 --- a/api/nodes/serializers.py +++ b/api/nodes/serializers.py @@ -1383,6 +1383,17 @@ class NodeSettingsUpdateSerializer(NodeSettingsSerializer): redirect_link_url = ser.URLField(write_only=True, required=False) redirect_link_label = ser.CharField(max_length=50, write_only=True, required=False) + + def to_representation(self, instance): + """ + Overriding to_representation allows using different serializers for the request and response. + """ + context = self.context + node = self.context['node'] + context['wiki_addon'] = node.get_addon('wiki') + context['forward_addon'] = node.get_addon('forward') + return NodeSettingsSerializer(instance=instance, context=context).data + def update(self, obj, validated_data): user = self.context['request'].user auth = get_user_auth(self.context['request']) diff --git a/api_tests/nodes/views/test_node_settings.py b/api_tests/nodes/views/test_node_settings.py index c6526dcd911..0fc8a562f30 100644 --- a/api_tests/nodes/views/test_node_settings.py +++ b/api_tests/nodes/views/test_node_settings.py @@ -216,6 +216,7 @@ def test_patch_access_requests_enabled(self, app, project, payload, admin_contri project.reload() assert project.access_requests_enabled is False assert project.logs.latest().action == NodeLog.NODE_ACCESS_REQUESTS_DISABLED + assert res.json['data']['attributes']['access_requests_enabled'] is False payload['data']['attributes']['access_requests_enabled'] = True # Logged in admin @@ -224,6 +225,7 @@ def test_patch_access_requests_enabled(self, app, project, payload, admin_contri project.reload() assert project.access_requests_enabled is True assert project.logs.latest().action == NodeLog.NODE_ACCESS_REQUESTS_ENABLED + assert res.json['data']['attributes']['access_requests_enabled'] is True def test_patch_anyone_can_comment(self, app, project, payload, admin_contrib, write_contrib, url): assert project.comment_level == 'public' @@ -238,6 +240,7 @@ def test_patch_anyone_can_comment(self, app, project, payload, admin_contrib, wr assert res.status_code == 200 project.reload() assert project.comment_level == 'private' + assert res.json['data']['attributes']['anyone_can_comment'] is False payload['data']['attributes']['anyone_can_comment'] = True # Logged in admin @@ -245,13 +248,14 @@ def test_patch_anyone_can_comment(self, app, project, payload, admin_contrib, wr assert res.status_code == 200 project.reload() assert project.comment_level == 'public' + assert res.json['data']['attributes']['anyone_can_comment'] is False def test_patch_anyone_can_edit_wiki(self, app, project, payload, admin_contrib, write_contrib, url): project.is_public = True project.save() wiki_addon = project.get_addon('wiki') assert wiki_addon.is_publicly_editable is False - payload['data']['attributes']['anyone_can_edit_wiki'] = True + payload['data']['attributes']['anyone_can_edit_wiki'] = False # Write cannot modify this field res = app.patch_json_api(url, payload, auth=write_contrib.auth, expect_errors=True) @@ -263,6 +267,7 @@ def test_patch_anyone_can_edit_wiki(self, app, project, payload, admin_contrib, wiki_addon.reload() assert wiki_addon.is_publicly_editable is True assert project.logs.latest().action == NodeLog.MADE_WIKI_PUBLIC + assert res.json['data']['attributes']['anyone_can_edit_wiki'] is True payload['data']['attributes']['anyone_can_edit_wiki'] = False # Logged in admin @@ -271,6 +276,7 @@ def test_patch_anyone_can_edit_wiki(self, app, project, payload, admin_contrib, wiki_addon.reload() assert wiki_addon.is_publicly_editable is False assert project.logs.latest().action == NodeLog.MADE_WIKI_PRIVATE + assert res.json['data']['attributes']['anyone_can_edit_wiki'] is False # Test wiki disabled in same request so cannot change wiki_settings payload['data']['attributes']['wiki_enabled'] = False @@ -289,6 +295,7 @@ def test_patch_anyone_can_edit_wiki(self, app, project, payload, admin_contrib, assert res.status_code == 200 assert project.get_addon('wiki').is_publicly_editable is True assert project.logs.latest().action == NodeLog.MADE_WIKI_PUBLIC + assert res.json['data']['attributes']['anyone_can_edit_wiki'] is True # If project is private, cannot change settings to allow anyone to edit wiki project.is_public = False @@ -309,6 +316,7 @@ def test_patch_wiki_enabled(self, app, project, payload, admin_contrib, write_co res = app.patch_json_api(url, payload, auth=admin_contrib.auth) assert res.status_code == 200 assert project.get_addon('wiki') is None + assert res.json['data']['attributes']['wiki_enabled'] is False # Nothing happens if attempting to disable an already-disabled wiki res = app.patch_json_api(url, payload, auth=admin_contrib.auth) @@ -320,6 +328,7 @@ def test_patch_wiki_enabled(self, app, project, payload, admin_contrib, write_co res = app.patch_json_api(url, payload, auth=admin_contrib.auth) assert res.status_code == 200 assert project.get_addon('wiki') is not None + assert res.json['data']['attributes']['wiki_enabled'] is True # Nothing happens if attempting to enable an already-enabled-wiki res = app.patch_json_api(url, payload, auth=admin_contrib.auth) @@ -330,21 +339,27 @@ def test_redirect_link_enabled(self, app, project, payload, admin_contrib, write assert project.get_addon('forward') is None payload['data']['attributes']['redirect_link_enabled'] = True + label = 'My Link' + url = 'https://cos.io' + # Redirect link not included res = app.patch_json_api(url, payload, auth=admin_contrib.auth, expect_errors=True) assert res.status_code == 400 assert res.json['errors'][0]['detail'] == 'You must include a redirect URL to enable a redirect.' - payload['data']['attributes']['redirect_link_url'] = 'https://cos.io' - payload['data']['attributes']['redirect_link_label'] = 'My Link' + payload['data']['attributes']['redirect_link_url'] = url + payload['data']['attributes']['redirect_link_label'] = label # Write contrib can modify forward related fields res = app.patch_json_api(url, payload, auth=write_contrib.auth) assert res.status_code == 200 forward_addon = project.get_addon('forward') assert forward_addon is not None - assert forward_addon.url == 'https://cos.io' + assert forward_addon.url == url assert forward_addon.label == 'My Link' assert project.logs.latest().action == 'forward_url_changed' + assert res.json['data']['attributes']['redirect_link_enabled'] is True + assert res.json['data']['attributes']['redirect_link_url'] == url + assert res.json['data']['attributes']['redirect_link_label'] == label # Attempting to set redirect_link_url when redirect_link not enabled payload['data']['attributes']['redirect_link_enabled'] = False @@ -367,6 +382,9 @@ def test_redirect_link_enabled(self, app, project, payload, admin_contrib, write assert res.status_code == 200 forward_addon = project.get_addon('forward') assert forward_addon is None + assert res.json['data']['attributes']['redirect_link_enabled'] is False + assert res.json['data']['attributes']['redirect_link_url'] is None + assert res.json['data']['attributes']['redirect_link_label'] is None def test_redirect_link_label_char_limit(self, app, project, payload, admin_contrib, url): project.add_addon('forward', ()) From dd681e718f11dd8bc69e31caa0d440e6740e0311 Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Thu, 19 Jul 2018 13:33:50 -0500 Subject: [PATCH 18/18] Fix NodeSettings tests. --- api/nodes/serializers.py | 10 +++++----- api_tests/nodes/views/test_node_settings.py | 16 ++++++++-------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/api/nodes/serializers.py b/api/nodes/serializers.py index a78086948f2..fd5382672b4 100644 --- a/api/nodes/serializers.py +++ b/api/nodes/serializers.py @@ -1434,7 +1434,7 @@ def update_wiki_fields(self, obj, validated_data, auth): if anyone_can_edit_wiki is not None: if not obj.is_public and anyone_can_edit_wiki: - raise exceptions.ValidationError('To allow all OSF users to edit the wiki, the project must be public.') + raise exceptions.ValidationError(detail='To allow all OSF users to edit the wiki, the project must be public.') if wiki_addon: try: wiki_addon.set_editing(permissions=anyone_can_edit_wiki, auth=auth, log=True) @@ -1442,7 +1442,7 @@ def update_wiki_fields(self, obj, validated_data, auth): return wiki_addon.save() else: - raise exceptions.ValidationError('You must have the wiki enabled before changing wiki settings.') + raise exceptions.ValidationError(detail='You must have the wiki enabled before changing wiki settings.') def update_forward_fields(self, obj, validated_data, auth): redirect_link_enabled = validated_data.get('redirect_link_enabled') @@ -1454,12 +1454,12 @@ def update_forward_fields(self, obj, validated_data, auth): if redirect_link_enabled is not None: if not redirect_link_url and redirect_link_enabled: - raise exceptions.ValidationError('You must include a redirect URL to enable a redirect.') + raise exceptions.ValidationError(detail='You must include a redirect URL to enable a redirect.') forward_addon = self.enable_or_disable_addon(obj, redirect_link_enabled, 'forward', auth) if redirect_link_url is not None: if not forward_addon: - raise exceptions.ValidationError('You must first set redirect_link_enabled to True before specifying a redirect link URL.') + raise exceptions.ValidationError(detail='You must first set redirect_link_enabled to True before specifying a redirect link URL.') forward_addon.url = redirect_link_url obj.add_log( action='forward_url_changed', @@ -1474,7 +1474,7 @@ def update_forward_fields(self, obj, validated_data, auth): if redirect_link_label is not None: if not forward_addon: - raise exceptions.ValidationError('You must first set redirect_link_enabled to True before specifying a redirect link label.') + raise exceptions.ValidationError(detail='You must first set redirect_link_enabled to True before specifying a redirect link label.') forward_addon.label = redirect_link_label save_forward = True diff --git a/api_tests/nodes/views/test_node_settings.py b/api_tests/nodes/views/test_node_settings.py index 0fc8a562f30..9015cc5c491 100644 --- a/api_tests/nodes/views/test_node_settings.py +++ b/api_tests/nodes/views/test_node_settings.py @@ -248,14 +248,14 @@ def test_patch_anyone_can_comment(self, app, project, payload, admin_contrib, wr assert res.status_code == 200 project.reload() assert project.comment_level == 'public' - assert res.json['data']['attributes']['anyone_can_comment'] is False + assert res.json['data']['attributes']['anyone_can_comment'] is True def test_patch_anyone_can_edit_wiki(self, app, project, payload, admin_contrib, write_contrib, url): project.is_public = True project.save() wiki_addon = project.get_addon('wiki') assert wiki_addon.is_publicly_editable is False - payload['data']['attributes']['anyone_can_edit_wiki'] = False + payload['data']['attributes']['anyone_can_edit_wiki'] = True # Write cannot modify this field res = app.patch_json_api(url, payload, auth=write_contrib.auth, expect_errors=True) @@ -340,25 +340,25 @@ def test_redirect_link_enabled(self, app, project, payload, admin_contrib, write payload['data']['attributes']['redirect_link_enabled'] = True label = 'My Link' - url = 'https://cos.io' + link = 'https://cos.io' # Redirect link not included res = app.patch_json_api(url, payload, auth=admin_contrib.auth, expect_errors=True) assert res.status_code == 400 assert res.json['errors'][0]['detail'] == 'You must include a redirect URL to enable a redirect.' - payload['data']['attributes']['redirect_link_url'] = url + payload['data']['attributes']['redirect_link_url'] = link payload['data']['attributes']['redirect_link_label'] = label # Write contrib can modify forward related fields res = app.patch_json_api(url, payload, auth=write_contrib.auth) assert res.status_code == 200 forward_addon = project.get_addon('forward') assert forward_addon is not None - assert forward_addon.url == url - assert forward_addon.label == 'My Link' + assert forward_addon.url == link + assert forward_addon.label == label assert project.logs.latest().action == 'forward_url_changed' assert res.json['data']['attributes']['redirect_link_enabled'] is True - assert res.json['data']['attributes']['redirect_link_url'] == url + assert res.json['data']['attributes']['redirect_link_url'] == link assert res.json['data']['attributes']['redirect_link_label'] == label # Attempting to set redirect_link_url when redirect_link not enabled @@ -371,7 +371,7 @@ def test_redirect_link_enabled(self, app, project, payload, admin_contrib, write # Attempting to set redirect_link_label when redirect_link not enabled payload['data']['attributes']['redirect_link_enabled'] = False del payload['data']['attributes']['redirect_link_url'] - payload['data']['attributes']['redirect_link_label'] = 'My Link' + payload['data']['attributes']['redirect_link_label'] = label res = app.patch_json_api(url, payload, auth=admin_contrib.auth, expect_errors=True) assert res.status_code == 400 assert res.json['errors'][0]['detail'] == 'You must first set redirect_link_enabled to True before specifying a redirect link label.'