Merge branch 'release/19.24.0'

.travis.yml

@@ -4,8 +4,9 @@ language: python
 
 python:
     - "2.7"
-dist: trusty
+
 sudo: false
+dist: trusty
 
 # TODO: uncomment when https://github.com/travis-ci/travis-ci/issues/8836 is resolved
 # addons:

CHANGELOG

@@ -2,6 +2,21 @@
 
 We follow the CalVer (https://calver.org/) versioning scheme: YY.MINOR.MICRO.
 
+19.24.0 (2019-08-27)
+===================
+- APIv2: Allow creating a node with a license attached on creation
+- APIv2: Prevent creating a node link to the node itself, a parent, or a child
+- APIv2: Have the move/copy WB hooks update storage usage
+- Exclude collection groups from user page in Django's admin app
+- Have osfstorage move/copy hooks recursively update the latest file version's region
+- Fix password reset tokens
+- Do not reveal entire google drive file path in node logs
+- Allow logging in from password reset and forgot password pages
+- Fix broken social links on add contributors modal
+- Fix email typos in embargoed registration emails
+- Improve registration and retraction node log display text
+- Fix logs if cron job automatically approves a withdrawal
+
 19.23.0 (2019-08-19)
 ===================
 - Represents scopes as an m2m field on personal access tokens instead of a CharField

addons/osfstorage/models.py

@@ -166,16 +166,16 @@ def delete(self, user=None, parent=None, **kwargs):
         self._materialized_path = self.materialized_path
         return super(OsfStorageFileNode, self).delete(user=user, parent=parent) if self._check_delete_allowed() else None
 
+    def update_region_from_latest_version(self, destination_parent):
+        raise NotImplementedError
+
     def move_under(self, destination_parent, name=None):
         if self.is_preprint_primary:
             if self.target != destination_parent.target or self.provider != destination_parent.provider:
                 raise exceptions.FileNodeIsPrimaryFile()
         if self.is_checked_out:
             raise exceptions.FileNodeCheckedOutError()
-        most_recent_fileversion = self.versions.select_related('region').order_by('-created').first()
-        if most_recent_fileversion and most_recent_fileversion.region != destination_parent.target.osfstorage_region:
-            most_recent_fileversion.region = destination_parent.target.osfstorage_region
-            most_recent_fileversion.save()
+        self.update_region_from_latest_version(destination_parent)
         return super(OsfStorageFileNode, self).move_under(destination_parent, name)
 
     def check_in_or_out(self, user, checkout, save=False):
@@ -293,6 +293,12 @@ def serialize(self, include_full=None, version=None):
         })
         return ret
 
+    def update_region_from_latest_version(self, destination_parent):
+        most_recent_fileversion = self.versions.select_related('region').order_by('-created').first()
+        if most_recent_fileversion and most_recent_fileversion.region != destination_parent.target.osfstorage_region:
+            most_recent_fileversion.region = destination_parent.target.osfstorage_region
+            most_recent_fileversion.save()
+
     def create_version(self, creator, location, metadata=None):
         latest_version = self.get_version()
         version = FileVersion(identifier=self.versions.count() + 1, creator=creator, location=location)
@@ -451,6 +457,9 @@ def serialize(self, include_full=False, version=None):
             ret['fullPath'] = self.materialized_path
         return ret
 
+    def update_region_from_latest_version(self, destination_parent):
+        for child in self.children.all().prefetch_related('versions'):
+            child.update_region_from_latest_version(destination_parent)
 
 class Region(models.Model):
     _id = models.CharField(max_length=255, db_index=True)

addons/osfstorage/tests/test_models.py

@@ -334,6 +334,30 @@ def test_move_nested(self):
         assert_equal(new_project, move_to.target)
         assert_equal(new_project, child.target)
 
+    def test_move_nested_between_regions(self):
+        canada = RegionFactory()
+        new_component = NodeFactory(parent=self.project)
+        component_node_settings = new_component.get_addon('osfstorage')
+        component_node_settings.region = canada
+        component_node_settings.save()
+
+        move_to = component_node_settings.get_root()
+        to_move = self.node_settings.get_root().append_folder('Aaah').append_folder('Woop')
+        child = to_move.append_file('There it is')
+
+        for _ in range(2):
+            version = factories.FileVersionFactory(region=self.node_settings.region)
+            child.versions.add(version)
+        child.save()
+
+        moved = to_move.move_under(move_to)
+        child.reload()
+
+        assert new_component == child.target
+        versions = child.versions.order_by('-created')
+        assert versions.first().region == component_node_settings.region
+        assert versions.last().region == self.node_settings.region
+
     def test_copy_rename(self):
         to_copy = self.node_settings.get_root().append_file('Carp')
         copy_to = self.node_settings.get_root().append_folder('Cloud')

addons/wiki/tests/test_wiki.py

@@ -1238,7 +1238,6 @@ def test_serialize_wiki_settings(self):
         node = NodeFactory(parent=self.project, creator=self.user, is_public=True)
         node.get_addon('wiki').set_editing(
             permissions=True, auth=self.consolidate_auth, log=True)
-        node.add_pointer(self.project, Auth(self.user))
         node.save()
         data = serialize_wiki_settings(self.user, [node])
         expected = [{

admin_tests/base/test_utils.py

@@ -1,22 +1,30 @@
 from nose.tools import *  # noqa: F403
 import datetime as datetime
+import pytest
 
+from django.test import RequestFactory
 from django.db.models import Q
 from django.contrib.auth.models import Group
 from django.core.exceptions import ValidationError, PermissionDenied
+from django.contrib.admin.sites import AdminSite
+from django.forms.models import model_to_dict
+from django.http import QueryDict
+
 
 from tests.base import AdminTestCase
 
-from osf_tests.factories import SubjectFactory, UserFactory, RegistrationFactory
+from osf_tests.factories import SubjectFactory, UserFactory, RegistrationFactory, PreprintFactory
 
-from osf.models import Subject
+from osf.models import Subject, OSFUser, Collection
 from osf.models.provider import rules_to_subjects
 from admin.base.utils import get_subject_rules, change_embargo_date
+from osf.admin import OSFUserAdmin
 
 
 import logging
 logger = logging.getLogger(__name__)
 logging.basicConfig(level=logging.INFO)
+pytestmark = pytest.mark.django_db
 
 
 class TestSubjectRules(AdminTestCase):
@@ -158,3 +166,82 @@ def test_change_embargo_date(self):
         assert_almost_equal(self.registration.embargo.end_date, self.date_valid2, delta=datetime.timedelta(days=1))
 
         # Add a test to check privatizing
+
+site = AdminSite()
+
+class TestGroupCollectionsPreprints:
+    @pytest.mark.enable_bookmark_creation
+    @pytest.fixture()
+    def user(self):
+        return UserFactory()
+
+    @pytest.fixture()
+    def admin_url(self, user):
+        return '/admin/osf/osfuser/{}/change/'.format(user.id)
+
+    @pytest.fixture()
+    def preprint(self, user):
+        return PreprintFactory(creator=user)
+
+    @pytest.fixture()
+    def get_request(self, admin_url, user):
+        request = RequestFactory().get(admin_url)
+        request.user = user
+        return request
+
+    @pytest.fixture()
+    def post_request(self, admin_url, user):
+        request = RequestFactory().post(admin_url)
+        request.user = user
+        return request
+
+    @pytest.fixture()
+    def osf_user_admin(self):
+        return OSFUserAdmin(OSFUser, site)
+
+    @pytest.mark.enable_bookmark_creation
+    def test_admin_app_formfield_collections(self, preprint, user, get_request, osf_user_admin):
+        """ Testing OSFUserAdmin.formfield_many_to_many.
+        This should not return any bookmark collections or preprint groups, even if the user is a member.
+        """
+
+        formfield = (osf_user_admin.formfield_for_manytomany(OSFUser.groups.field, request=get_request))
+        queryset = formfield.queryset
+
+        collections_group = Collection.objects.filter(creator=user, is_bookmark_collection=True)[0].get_group('admin')
+        assert(collections_group not in queryset)
+
+        assert(preprint.get_group('admin') not in queryset)
+
+    @pytest.mark.enable_bookmark_creation
+    def test_admin_app_save_related_collections(self, post_request, osf_user_admin, user, preprint):
+        """ Testing OSFUserAdmin.save_related
+        This should maintain the bookmark collections and preprint groups the user is a member of
+        even though they aren't explicitly returned by the form.
+        """
+
+        form = osf_user_admin.get_form(request=post_request, obj=user)
+        data_dict = model_to_dict(user)
+        post_form = form(data_dict, instance=user)
+
+        # post_form.errors.keys() generates a list of fields causing JSON Related errors
+        # which are preventing the form from being valid (which is required for the form to be saved).
+        # By setting the field to '{}', this makes the form valid and resolves JSON errors.
+
+        for field in post_form.errors.keys():
+            if field == 'groups':
+                data_dict['groups'] = []
+            else:
+                data_dict[field] = '{}'
+        post_form = form(data_dict, instance=user)
+        assert(post_form.is_valid())
+        post_form.save(commit=False)
+        qdict = QueryDict('', mutable=True)
+        qdict.update(data_dict)
+        post_request.POST = qdict
+        osf_user_admin.save_related(request=post_request, form=post_form, formsets=[], change=True)
+
+        collections_group = Collection.objects.filter(creator=user, is_bookmark_collection=True)[0].get_group('admin')
+        assert(collections_group in user.groups.all())
+
+        assert(preprint.get_group('admin') in user.groups.all())

api/base/serializers.py

@@ -1674,7 +1674,12 @@ def update(self, instance, validated_data):
         for pointer in remove:
             collection.rm_pointer(pointer, auth)
         for node in add:
-            collection.add_pointer(node, auth)
+            try:
+                collection.add_pointer(node, auth)
+            except ValueError as e:
+                raise api_exceptions.InvalidModelValueError(
+                    detail=str(e),
+                )
 
         return self.make_instance_obj(collection)
 
@@ -1689,8 +1694,12 @@ def create(self, validated_data):
             raise api_exceptions.RelationshipPostMakesNoChanges
 
         for node in add:
-            collection.add_pointer(node, auth)
-
+            try:
+                collection.add_pointer(node, auth)
+            except ValueError as e:
+                raise api_exceptions.InvalidModelValueError(
+                    detail=str(e),
+                )
         return self.make_instance_obj(collection)
 
 
@@ -1747,7 +1756,12 @@ def update(self, instance, validated_data):
         for pointer in remove:
             collection.rm_pointer(pointer, auth)
         for node in add:
-            collection.add_pointer(node, auth)
+            try:
+                collection.add_pointer(node, auth)
+            except ValueError as e:
+                raise api_exceptions.InvalidModelValueError(
+                    detail=str(e),
+                )
 
         return self.make_instance_obj(collection)
 
@@ -1762,7 +1776,12 @@ def create(self, validated_data):
             raise api_exceptions.RelationshipPostMakesNoChanges
 
         for node in add:
-            collection.add_pointer(node, auth)
+            try:
+                collection.add_pointer(node, auth)
+            except ValueError as e:
+                raise api_exceptions.InvalidModelValueError(
+                    detail=str(e),
+                )
 
         return self.make_instance_obj(collection)
 

api/nodes/serializers.py

@@ -141,8 +141,8 @@ def to_internal_value(self, data):
 
 class NodeLicenseSerializer(BaseAPISerializer):
 
-    copyright_holders = ser.ListField(allow_empty=True)
-    year = ser.CharField(allow_blank=True)
+    copyright_holders = ser.ListField(allow_empty=True, required=False)
+    year = ser.CharField(allow_blank=True, required=False)
 
     class Meta:
         type_ = 'node_licenses'
@@ -206,8 +206,12 @@ class Meta:
         type_ = 'styled-citations'
 
 def get_license_details(node, validated_data):
-    license = node.license if isinstance(node, Preprint) else node.node_license
-
+    if node:
+        license = node.license if isinstance(node, Preprint) else node.node_license
+    else:
+        license = None
+    if ('license_type' not in validated_data and not (license and license.node_license.license_id)):
+        raise exceptions.ValidationError(detail='License ID must be provided for a Node License.')
     license_id = license.node_license.license_id if license else None
     license_year = license.year if license else None
     license_holders = license.copyright_holders if license else []
@@ -747,10 +751,18 @@ def create(self, validated_data):
         tag_instances = []
         affiliated_institutions = None
         region_id = None
+        license_details = None
         if 'affiliated_institutions' in validated_data:
             affiliated_institutions = validated_data.pop('affiliated_institutions')
         if 'region_id' in validated_data:
             region_id = validated_data.pop('region_id')
+        if 'license_type' in validated_data or 'license' in validated_data:
+            try:
+                license_details = get_license_details(None, validated_data)
+            except ValidationError as e:
+                raise InvalidModelValueError(detail=str(e.messages[0]))
+            validated_data.pop('license', None)
+            validated_data.pop('license_type', None)
         if 'tags' in validated_data:
             tags = validated_data.pop('tags')
             for tag in tags:
@@ -806,6 +818,20 @@ def create(self, validated_data):
             node.subjects.add(parent.subjects.all())
             node.save()
 
+        if license_details:
+            try:
+                node.set_node_license(
+                    {
+                        'id': license_details.get('id') if license_details.get('id') else 'NONE',
+                        'year': license_details.get('year'),
+                        'copyrightHolders': license_details.get('copyrightHolders') or license_details.get('copyright_holders', []),
+                    },
+                    auth=get_user_auth(request),
+                    save=True,
+                )
+            except ValidationError as e:
+                raise InvalidModelValueError(detail=str(e.message))
+
         if not region_id:
             region_id = self.context.get('region_id')
         if region_id:
@@ -1314,10 +1340,10 @@ def create(self, validated_data):
         try:
             pointer = node.add_pointer(pointer_node, auth, save=True)
             return pointer
-        except ValueError:
+        except ValueError as e:
             raise InvalidModelValueError(
                 source={'pointer': '/data/relationships/node_links/data/id'},
-                detail='Target Node \'{}\' already pointed to by \'{}\'.'.format(target_node_id, node._id),
+                detail=str(e),
             )
 
     def update(self, instance, validated_data):