Merge pull request #10072 from cslzchen/the-rest-of-the-fixes

[Django Upgrade] [ENG-4072] The Rest - Part 2

admin_tests/institutions/test_views.py

@@ -3,16 +3,16 @@
 from nose import tools as nt
 from django.test import RequestFactory
 from django.contrib.auth.models import Permission
+from django.contrib.contenttypes.models import ContentType
 from django.core.exceptions import PermissionDenied
 
-
 from tests.base import AdminTestCase
 from osf_tests.factories import (
     AuthUserFactory,
     InstitutionFactory,
     ProjectFactory
 )
-from osf.models import Institution, Node
+from osf.models import Institution, Node, AbstractNode
 
 from admin_tests.utilities import setup_form_view, setup_user_view
 
@@ -208,7 +208,10 @@ def setUp(self):
         self.institution = InstitutionFactory()
 
         self.user = AuthUserFactory()
-        self.view_node = Permission.objects.get(codename='view_node')
+        self.view_node = Permission.objects.filter(
+            codename='view_node',
+            content_type_id=ContentType.objects.get_for_model(AbstractNode).id
+        ).first()
         self.user.user_permissions.add(self.view_node)
         self.user.affiliated_institutions.add(self.institution)
         self.user.save()

admin_tests/nodes/test_views.py

@@ -6,7 +6,7 @@
 import datetime
 import responses
 
-from osf.models import AdminLogEntry, NodeLog
+from osf.models import AdminLogEntry, NodeLog, AbstractNode
 from admin.nodes.views import (
     NodeDeleteView,
     NodeRemoveContributorView,
@@ -29,6 +29,7 @@
 from django.urls import reverse
 from django.core.exceptions import PermissionDenied
 from django.contrib.auth.models import Permission
+from django.contrib.contenttypes.models import ContentType
 from framework.auth.core import Auth
 
 from tests.base import AdminTestCase
@@ -97,7 +98,10 @@ def test_correct_view_permissions(self):
         node = ProjectFactory()
         guid = node._id
 
-        change_permission = Permission.objects.get(codename='view_node')
+        change_permission = Permission.objects.filter(
+            codename='view_node',
+            content_type_id=ContentType.objects.get_for_model(AbstractNode).id
+        ).first()
         user.user_permissions.add(change_permission)
         user.save()
 
@@ -153,7 +157,10 @@ def test_correct_view_permissions(self):
         guid = self.node._id
 
         change_permission = Permission.objects.get(codename='delete_node')
-        view_permission = Permission.objects.get(codename='view_node')
+        view_permission = Permission.objects.filter(
+            codename='view_node',
+            content_type_id=ContentType.objects.get_for_model(AbstractNode).id
+        ).first()
         user.user_permissions.add(change_permission)
         user.user_permissions.add(view_permission)
         user.save()
@@ -226,7 +233,10 @@ def test_no_user_permissions_raises_error(self):
 
     def test_correct_view_permissions(self):
         change_permission = Permission.objects.get(codename='change_node')
-        view_permission = Permission.objects.get(codename='view_node')
+        view_permission = Permission.objects.filter(
+            codename='view_node',
+            content_type_id=ContentType.objects.get_for_model(AbstractNode).id
+        ).first()
         self.user.user_permissions.add(change_permission)
         self.user.user_permissions.add(view_permission)
         self.user.save()

admin_tests/preprints/test_views.py

@@ -103,8 +103,8 @@ def test_get_object(self, req, preprint, plain_view):
     def test_no_user_permissions_raises_error(self, user, preprint, plain_view):
         request = RequestFactory().get(reverse('preprints:preprint', kwargs={'guid': preprint._id}))
         request.user = user
-        resp = plain_view.as_view()(request, guid=preprint._id)
-        assert resp._headers['location'][1] == f'/accounts/login/?next=/preprints/{preprint._id}/'
+        with pytest.raises(PermissionDenied):
+            plain_view.as_view()(request, guid=preprint._id)
 
     def test_get_flagged_spam(self, superuser, preprint, ham_preprint, spam_preprint, flagged_preprint):
         request = RequestFactory().get(reverse('preprints:flagged-spam'))
@@ -175,16 +175,16 @@ def test_confirm_ham(self, preprint, superuser, mock_akismet):
         assert preprint.spam_status == SpamStatus.HAM
         assert preprint.is_public
 
-    def test_correct_view_permissions(self, user, preprint, plain_view):
+    def test_valid_but_insufficient_view_permissions(self, user, preprint, plain_view):
         view_permission = Permission.objects.get(codename='view_preprint')
         user.user_permissions.add(view_permission)
         user.save()
 
         request = RequestFactory().get(reverse('preprints:preprint', kwargs={'guid': preprint._id}))
         request.user = user
 
-        response = plain_view.as_view()(request, guid=preprint._id)
-        assert response.status_code == 302
+        with pytest.raises(PermissionDenied):
+            plain_view.as_view()(request, guid=preprint._id)
 
     def test_change_preprint_provider(self, user, preprint, plain_view):
         change_permission = Permission.objects.get(codename='change_preprint')

admin_tests/users/test_views.py

@@ -414,7 +414,7 @@ def test_search_user_by_guid(self):
         nt.assert_true(form.is_valid())
         response = self.view.form_valid(form)
         nt.assert_equal(response.status_code, 302)
-        nt.assert_equal(response._headers['location'][1], '/users/{}/'.format(self.user_1.guids.first()._id))
+        nt.assert_equal(response.headers['location'], '/users/{}/'.format(self.user_1.guids.first()._id))
 
     def test_search_user_by_name(self):
         form_data = {
@@ -424,7 +424,7 @@ def test_search_user_by_name(self):
         nt.assert_true(form.is_valid())
         response = self.view.form_valid(form)
         nt.assert_equal(response.status_code, 302)
-        nt.assert_equal(response._headers['location'][1], '/users/search/Hardy/')
+        nt.assert_equal(response.headers['location'], '/users/search/Hardy/')
 
     def test_search_user_by_name_with_punctuation(self):
         form_data = {
@@ -434,7 +434,7 @@ def test_search_user_by_name_with_punctuation(self):
         nt.assert_true(form.is_valid())
         response = self.view.form_valid(form)
         nt.assert_equal(response.status_code, 302)
-        nt.assert_equal(response._headers['location'][1], '/users/search/Dr.%20Sportello-Fay,%20PI%20@,%20%23,%20$,%20%25,%20%5E,%20&,%20*,%20(,%20),%20~/')
+        nt.assert_equal(response.headers['location'], '/users/search/Dr.%20Sportello-Fay,%20PI%20@,%20%23,%20$,%20%25,%20%5E,%20&,%20*,%20(,%20),%20~/')
 
     def test_search_user_by_username(self):
         form_data = {
@@ -444,7 +444,7 @@ def test_search_user_by_username(self):
         nt.assert_true(form.is_valid())
         response = self.view.form_valid(form)
         nt.assert_equal(response.status_code, 302)
-        nt.assert_equal(response._headers['location'][1], '/users/{}/'.format(self.user_1.guids.first()._id))
+        nt.assert_equal(response.headers['location'], '/users/{}/'.format(self.user_1.guids.first()._id))
 
     def test_search_user_by_alternate_email(self):
         form_data = {
@@ -454,7 +454,7 @@ def test_search_user_by_alternate_email(self):
         nt.assert_true(form.is_valid())
         response = self.view.form_valid(form)
         nt.assert_equal(response.status_code, 302)
-        nt.assert_equal(response._headers['location'][1], '/users/{}/'.format(self.user_2.guids.first()._id))
+        nt.assert_equal(response.headers['location'], '/users/{}/'.format(self.user_2.guids.first()._id))
 
     def test_search_user_list(self):
         view = views.UserSearchList()

api/draft_registrations/serializers.py

@@ -27,7 +27,12 @@
 class NodeRelationshipField(RelationshipField):
 
     def to_internal_value(self, node_id):
-        return {'branched_from': Node.load(node_id)}
+        context_view = self.context['view']
+        if hasattr(context_view, 'get_node'):
+            node = self.context['view'].get_node(node_id=node_id) if node_id else None
+        else:
+            node = Node.load(node_id)
+        return {'branched_from': node}
 
 
 class DraftRegistrationSerializer(DraftRegistrationLegacySerializer, TaxonomizableSerializerMixin):

api_tests/draft_registrations/views/test_draft_registration_list.py

@@ -72,10 +72,13 @@ def test_cannot_view_draft_list(
 
 
 class TestDraftRegistrationCreateWithNode(TestDraftRegistrationCreate):
+
+    # Overrides `url_draft_registrations` in `TestDraftRegistrationCreate`
     @pytest.fixture()
     def url_draft_registrations(self, project_public):
         return '/{}draft_registrations/?'.format(API_BASE)
 
+    # Overrides `payload` in TestDraftRegistrationCreate`
     @pytest.fixture()
     def payload(self, metaschema_open_ended, provider, project_public):
         return {
@@ -105,39 +108,55 @@ def payload(self, metaschema_open_ended, provider, project_public):
             }
         }
 
+    # Temporary alternative provider that supports `metaschema_open_ended` in `TestDraftRegistrationCreate`
+    # This provider is created to fix the first 3 tests in this test class due to test DB changes with the
+    # Django 3 Upgrade. A long-term solution is to create and/or use dedicated schemas for testing.
+    @pytest.fixture()
+    def provider_alt(self, metaschema_open_ended):
+        default_provider = RegistrationProvider.get_default()
+        default_provider.schemas.add(metaschema_open_ended)
+        default_provider.save()
+        return default_provider
+
+    # Similarly, this is a temporary alternative payload that uses the above `provider_alt`.
+    @pytest.fixture()
+    def payload_alt(self, payload, provider_alt):
+        new_payload = payload.copy()
+        new_payload['data']['relationships']['provider']['data']['id'] = provider_alt._id
+        return new_payload
+
     # Overrides TestDraftRegistrationList
-    def test_cannot_create_draft_errors(
-            self, app, user, project_public, payload, url_draft_registrations):
+    def test_cannot_create_draft_errors(self, app, user, payload_alt, project_public, url_draft_registrations):
         #   test_cannot_create_draft_from_a_registration
         registration = RegistrationFactory(
             project=project_public, creator=user)
-        payload['data']['relationships']['branched_from']['data']['id'] = registration._id
+        payload_alt['data']['relationships']['branched_from']['data']['id'] = registration._id
         res = app.post_json_api(
-            url_draft_registrations, payload, auth=user.auth,
+            url_draft_registrations, payload_alt, auth=user.auth,
             expect_errors=True)
         assert res.status_code == 404
 
     #   test_cannot_create_draft_from_deleted_node
         project = ProjectFactory(is_public=True, creator=user)
         project.is_deleted = True
         project.save()
-        payload['data']['relationships']['branched_from']['data']['id'] = project._id
+        payload_alt['data']['relationships']['branched_from']['data']['id'] = project._id
         res = app.post_json_api(
-            url_draft_registrations, payload,
+            url_draft_registrations, payload_alt,
             auth=user.auth, expect_errors=True)
         assert res.status_code == 410
         assert res.json['errors'][0]['detail'] == 'The requested node is no longer available.'
 
     #   test_cannot_create_draft_from_collection
         collection = CollectionFactory(creator=user)
-        payload['data']['relationships']['branched_from']['data']['id'] = collection._id
+        payload_alt['data']['relationships']['branched_from']['data']['id'] = collection._id
         res = app.post_json_api(
-            url_draft_registrations, payload, auth=user.auth,
+            url_draft_registrations, payload_alt, auth=user.auth,
             expect_errors=True)
         assert res.status_code == 404
 
     def test_draft_registration_attributes_copied_from_node(self, app, project_public,
-            url_draft_registrations, user, payload):
+            url_draft_registrations, user, payload_alt):
 
         write_contrib = AuthUserFactory()
         read_contrib = AuthUserFactory()
@@ -156,12 +175,12 @@ def test_draft_registration_attributes_copied_from_node(self, app, project_publi
         project_public.add_contributor(write_contrib, WRITE)
         project_public.add_contributor(read_contrib, READ)
 
-        res = app.post_json_api(url_draft_registrations, payload, auth=write_contrib.auth, expect_errors=True)
+        res = app.post_json_api(url_draft_registrations, payload_alt, auth=write_contrib.auth, expect_errors=True)
         assert res.status_code == 201
-        res = app.post_json_api(url_draft_registrations, payload, auth=read_contrib.auth, expect_errors=True)
+        res = app.post_json_api(url_draft_registrations, payload_alt, auth=read_contrib.auth, expect_errors=True)
         assert res.status_code == 403
 
-        res = app.post_json_api(url_draft_registrations, payload, auth=user.auth)
+        res = app.post_json_api(url_draft_registrations, payload_alt, auth=user.auth)
         assert res.status_code == 201
         attributes = res.json['data']['attributes']
         assert attributes['title'] == project_public.title
@@ -180,14 +199,14 @@ def test_draft_registration_attributes_copied_from_node(self, app, project_publi
     def test_cannot_create_draft(
             self, app, user_write_contrib,
             user_read_contrib, user_non_contrib,
-            project_public, payload, group,
+            project_public, payload_alt, group,
             url_draft_registrations, group_mem):
 
         #   test_write_only_contributor_cannot_create_draft
         assert user_write_contrib in project_public.contributors.all()
         res = app.post_json_api(
             url_draft_registrations,
-            payload,
+            payload_alt,
             auth=user_write_contrib.auth,
             expect_errors=True)
         assert res.status_code == 201
@@ -196,29 +215,29 @@ def test_cannot_create_draft(
         assert user_read_contrib in project_public.contributors.all()
         res = app.post_json_api(
             url_draft_registrations,
-            payload,
+            payload_alt,
             auth=user_read_contrib.auth,
             expect_errors=True)
         assert res.status_code == 403
 
     #   test_non_authenticated_user_cannot_create_draft
         res = app.post_json_api(
             url_draft_registrations,
-            payload, expect_errors=True)
+            payload_alt, expect_errors=True)
         assert res.status_code == 401
 
     #   test_logged_in_non_contributor_cannot_create_draft
         res = app.post_json_api(
             url_draft_registrations,
-            payload,
+            payload_alt,
             auth=user_non_contrib.auth,
             expect_errors=True)
         assert res.status_code == 403
 
     #   test_group_admin_cannot_create_draft
         res = app.post_json_api(
             url_draft_registrations,
-            payload,
+            payload_alt,
             auth=group_mem.auth,
             expect_errors=True)
         assert res.status_code == 201
@@ -228,7 +247,7 @@ def test_cannot_create_draft(
         project_public.add_osf_group(group, WRITE)
         res = app.post_json_api(
             url_draft_registrations,
-            payload,
+            payload_alt,
             auth=group_mem.auth,
             expect_errors=True)
         assert res.status_code == 201

api_tests/nodes/views/test_node_detail.py

@@ -1,9 +1,10 @@
 # -*- coding: utf-8 -*-
-import mock
-import pytest
+from django.utils import timezone
 from future.moves.urllib.parse import urlparse
+import mock
 from nose.tools import *  # noqa:
-
+import pytest
+from rest_framework import exceptions
 
 from addons.wiki.tests.factories import WikiFactory, WikiVersionFactory
 from api.base.settings.defaults import API_BASE
@@ -33,7 +34,6 @@
     WithdrawnRegistrationFactory,
     DraftNodeFactory,
 )
-from rest_framework import exceptions
 from tests.base import fake
 from tests.utils import assert_latest_log, assert_latest_log_not
 from website.views import find_bookmark_collection
@@ -480,8 +480,12 @@ def test_node_shows_related_count_for_linked_by_relationships(self, app, user, p
         assert res.json['data']['relationships']['linked_by_nodes']['links']['related']['meta']['count'] == 1
         assert res.json['data']['relationships']['linked_by_registrations']['links']['related']['meta']['count'] == 1
 
+        log_date = timezone.now()
+        project_private.deleted_date = log_date
+        project_private.deleted = log_date
         project_private.is_deleted = True
         project_private.save()
+        registration.reload()
         project_public.reload()
 
         res = app.get(url)