Work through regressions #3168

Author: joemull

src/core/admin.py

@@ -451,11 +451,11 @@ class AffiliationAdmin(admin.ModelAdmin):
         'pk',
         'title',
         'department',
-        'organization__ror_display_for',
-        'organization__custom_label_for',
-        'organization__label_for',
-        'organization__alias_for',
-        'organization__acronym_for'
+        'organization__ror_display__value',
+        'organization__custom_label__value',
+        'organization__labels__value',
+        'organization__aliases__value',
+        'organization__acronyms__value',
         'account__first_name',
         'account__last_name',
         'account__email',

src/core/models.py

@@ -566,8 +566,6 @@ def snapshot_self(self, article, force_update=True):
             'middle_name': self.middle_name,
             'last_name': self.last_name,
             'name_suffix': self.suffix,
-            'institution': self.institution,
-            'department': self.department,
             'display_email': True if self == article.correspondence_author else False,
         }
 
@@ -576,6 +574,8 @@ def snapshot_self(self, article, force_update=True):
         if frozen_author and force_update:
             for k, v in frozen_dict.items():
                 setattr(frozen_author, k, v)
+            frozen_author.institution = self.institution
+            frozen_author.department = self.department
             frozen_author.save()
 
         else:
@@ -589,11 +589,15 @@ def snapshot_self(self, article, force_update=True):
                     defaults={'order': order_integer}
                 )
 
-            submission_models.FrozenAuthor.objects.get_or_create(
+            frozen_author, created = submission_models.FrozenAuthor.objects.get_or_create(
                 author=self,
                 article=article,
                 defaults=dict(order=order_object.order, **frozen_dict)
             )
+            if created:
+                frozen_author.institution = self.institution
+                frozen_author.department = self.department
+                frozen_author.save()
 
     def frozen_author(self, article):
         try:
@@ -2074,40 +2078,39 @@ def naive_get_or_create(
         value,
         account=None,
         frozen_author=None,
+        preprint_author=None,
     ):
         """
         Backwards-compatible API for finding a matching organization name.
-        Intended for use in batch importers where ROR data is not available.
+        Intended for use in batch importers where ROR data is not available
+        in the data being imported.
         Does not support ROR ids, ROR name types, or location data.
         """
-        no_exact_match = (
-            cls.DoesNotExist or cls.MultipleObjectsReturned
-        )
-
         # Is there a single exact match in the
         # canonical name data from ROR (e.g. labels)?
         try:
             organization = cls.objects.get(labels__value=value)
             created = False
-        except no_exact_match:
+        except cls.DoesNotExist or cls.MultipleObjectsReturned:
             # Or maybe one in the past or alternate
             # name data from ROR (e.g. aliases)?
             try:
                 organization = cls.objects.get(aliases__value=value)
                 created = False
-            except no_exact_match:
+            except cls.DoesNotExist or cls.MultipleObjectsReturned:
                 # Or maybe an organization has already been
                 # entered without a ROR for this
                 # account / frozen author / preprint author?
                 try:
                     organization = cls.objects.get(
                         affiliation__account=account,
                         affiliation__frozen_author=frozen_author,
+                        affiliation__preprint_author=preprint_author,
                         ror__exact='',
                     )
                     created = False
-                except no_exact_match:
-                    # Otherwise, create a naive, disconnected record
+                except cls.DoesNotExist or cls.MultipleObjectsReturned:
+                    # Otherwise, create a naive, disconnected record.
                     organization = cls.objects.create()
                     created = True
 
@@ -2350,17 +2353,24 @@ def naive_get_or_create(
             value,
             account=account,
             frozen_author=frozen_author,
-        )
-
-        # Create or update the actual affiliation
-        affiliation, created = Affiliation.objects.get_or_create(
-            account=account,
-            frozen_author=frozen_author,
             preprint_author=preprint_author,
-            organization=organization,
         )
-        affiliation.is_primary = True
-        affiliation.save()
+
+        # Create or update the actual affiliation if the associated
+        # account / frozen author / preprint author has been saved already
+        try:
+            affiliation, created = Affiliation.objects.get_or_create(
+                account=account,
+                frozen_author=frozen_author,
+                preprint_author=preprint_author,
+                organization=organization,
+            )
+            affiliation.is_primary = True
+            affiliation.save()
+        except ValueError:
+            logger.warn('The affiliation could not be created.')
+            affiliation = None
+            created = False
         return affiliation, created
 
     @classmethod
@@ -2375,13 +2385,18 @@ def naive_set_primary_department(
         Intended for use in batch importers where ROR data is not available.
         Does not support RORs or multiple affiliations.
         """
-        affiliation, _created = Affiliation.objects.get_or_create(
-            account=account,
-            frozen_author=frozen_author,
-            is_primary=True,
-        )
-        affiliation.department = value
-        affiliation.save()
+        # Create or update an affiliation if the associated
+        # account / frozen author / preprint author has been saved already
+        try:
+            affiliation, _created = Affiliation.objects.get_or_create(
+                account=account,
+                frozen_author=frozen_author,
+                is_primary=True,
+            )
+            affiliation.department = value
+            affiliation.save()
+        except ValueError:
+            logger.warn('The department could not be set.')
 
     @classmethod
     def naive_set_primary_country(

src/core/tests/test_app.py

@@ -206,7 +206,6 @@ def test_orcid_registration(self, record_mock):
         self.assertEqual(response.status_code, 200)
         self.assertContains(response, "Campbell")
         self.assertContains(response, "Kasey")
-        self.assertContains(response, "Elk Valley University")
         self.assertContains(response, "[email protected]")
         self.assertNotContains(response, "Register with ORCiD")
         self.assertContains(response, "http://sandbox.orcid.org/0000-0000-0000-0000")

src/core/views.py

@@ -329,11 +329,6 @@ def register(request):
         initial["last_name"] = orcid_details.get("last_name", "")
         if orcid_details.get("emails"):
             initial["email"] = orcid_details["emails"][0]
-        if orcid_details.get("affiliation"):
-            initial['institution'] = orcid_details['affiliation']
-        if orcid_details.get("country"):
-            if models.Country.objects.filter(code=orcid_details['country']).exists():
-                initial["country"] = models.Country.objects.get(code=orcid_details['country'])
 
     form = forms.RegistrationForm(
         journal=request.journal,
@@ -355,6 +350,10 @@ def register(request):
         if form.is_valid():
             if token_obj:
                 new_user = form.save()
+                if new_user.orcid:
+                    orcid_details = orcid.get_orcid_record_details(token_obj.orcid)
+                    if orcid_details.get("affiliation"):
+                        new_user.institution = orcid_details['affiliation']
                 token_obj.delete()
                 # If the email matches the user email on ORCID, log them in
                 if new_user.email == initial.get("email"):

src/repository/views.py

@@ -377,7 +377,7 @@ def repository_search(request, search_term=None):
                 Q(account__first_name__in=split_search_term) |
                 Q(account__middle_name__in=split_search_term) |
                 Q(account__last_name__in=split_search_term) |
-                Q(account__institution__icontains=search_term)
+                Q(account__affiliation__organization__labels__value__icontains=search_term)
             )
             &
             (

src/review/logic.py

@@ -824,24 +824,22 @@ def process_reviewer_csv(path, request, article, form):
         reader = csv.DictReader(csv_file)
         reviewers = []
         for row in reader:
-            try:
-                country = core_models.Country.objects.get(code=row.get('country'))
-            except core_models.Country.DoesNotExist:
-                country = None
-
             reviewer, created = core_models.Account.objects.get_or_create(
                 email=row.get('email_address'),
                 defaults={
                     'salutation': row.get('salutation', ''),
                     'first_name': row.get('firstname', ''),
                     'middle_name': row.get('middlename', ''),
                     'last_name': row.get('lastname', ''),
-                    'department': row.get('department', ''),
-                    'institution': row.get('institution', ''),
-                    'country': country,
                     'is_active': True,
                 }
             )
+            if row.get('department', ''):
+                reviewer.department = row.get('department', '')
+                reviewer.save()
+            if row.get('institution', ''):
+                reviewer.institution = row.get('institution', '')
+                reviewer.save()
 
             try:
                 review_interests = row.get('interests')

src/submission/tests.py

@@ -66,28 +66,32 @@ def create_journal():
 
         return journal_one
 
-    @staticmethod
-    def create_authors():
+    @classmethod
+    def create_authors(cls):
         author_1_data = {
+            'email': '[email protected]',
             'is_active': True,
             'password': 'this_is_a_password',
             'salutation': 'Prof.',
             'first_name': 'Martin',
+            'middle_name': '',
             'last_name': 'Eve',
             'department': 'English & Humanities',
             'institution': 'Birkbeck, University of London',
         }
         author_2_data = {
+            'email': '[email protected]',
             'is_active': True,
             'password': 'this_is_a_password',
             'salutation': 'Sr.',
             'first_name': 'Mauro',
+            'middle_name': '',
             'last_name': 'Sanchez',
             'department': 'English & Humanities',
             'institution': 'Birkbeck, University of London',
         }
-        author_1 = Account.objects.create(email="[email protected]", **author_1_data)
-        author_2 = Account.objects.create(email="[email protected]", **author_2_data)
+        author_1 = helpers.create_author(cls.journal_one, **author_1_data)
+        author_2 = helpers.create_author(cls.journal_one, **author_2_data)
 
         return author_1, author_2
 
@@ -105,12 +109,15 @@ def create_sections(self):
             journal=self.journal_one,
         )
 
+    @classmethod
+    def setUpTestData(cls):
+        cls.journal_one = cls.create_journal()
+
     def setUp(self):
         """
         Setup the test environment.
         :return: None
         """
-        self.journal_one = self.create_journal()
         self.editor = helpers.create_editor(self.journal_one)
         self.press = helpers.create_press()
         self.create_sections()
@@ -273,7 +280,9 @@ def test_snapshot_author_metadata_override(self):
 
         article.snapshot_authors()
         new_department = "New department"
-        article.frozen_authors().update(department=new_department)
+        for frozen in article.frozen_authors():
+            frozen.department = new_department
+            frozen.save()
         article.snapshot_authors(force_update=True)
         frozen = article.frozen_authors().all()[0]
 
@@ -576,15 +585,12 @@ def test_author_form_harmful_inputs(self):
             "middle_name",
             "name_prefix",
             "suffix",
-            "institution",
-            "department",
         }):
             form = forms.AuthorForm(
                 {
                     'first_name': 'Andy',
                     'last_name': 'Byers',
                     'biography': 'Andy',
-                    'institution': 'Birkbeck, University of London',
                     'email': f'andy{i}@janeway.systems',
                     'orcid': 'https://orcid.org/0000-0003-2126-266X',
                     **{attr: harmful_string},
@@ -783,37 +789,44 @@ def create_journal():
 
         return journal_one
 
-    @staticmethod
-    def create_authors():
+    @classmethod
+    def create_authors(cls):
         author_1_data = {
+            'email': '[email protected]',
             'is_active': True,
             'password': 'this_is_a_password',
             'salutation': 'Prof.',
             'first_name': 'Martin',
+            'middle_name': '',
             'last_name': 'Eve',
             'department': 'English & Humanities',
             'institution': 'Birkbeck, University of London',
         }
         author_2_data = {
+            'email': '[email protected]',
             'is_active': True,
             'password': 'this_is_a_password',
             'salutation': 'Sr.',
             'first_name': 'Mauro',
+            'middle_name': '',
             'last_name': 'Sanchez',
             'department': 'English & Humanities',
             'institution': 'Birkbeck, University of London',
         }
-        author_1 = Account.objects.create(email="[email protected]", **author_1_data)
-        author_2 = Account.objects.create(email="[email protected]", **author_1_data)
+        author_1 = helpers.create_author(cls.journal_one, **author_1_data)
+        author_2 = helpers.create_author(cls.journal_one, **author_2_data)
 
         return author_1, author_2
 
+    @classmethod
+    def setUpTestData(cls):
+        cls.journal_one = cls.create_journal()
+
     def setUp(self):
         """
         Setup the test environment.
         :return: None
         """
-        self.journal_one = self.create_journal()
         self.editor = helpers.create_editor(self.journal_one)
         self.press = helpers.create_press()
 

src/utils/testing/helpers.py

@@ -62,6 +62,7 @@ def create_user(username, roles=None, journal=None, **attrs):
             journal=journal
         )
 
+    user.save()
     for attr, value in attrs.items():
         setattr(user, attr, value)
 
@@ -179,8 +180,6 @@ def create_author(journal, **kwargs):
         "first_name": "Author",
         "middle_name": "A",
         "last_name": "User",
-        "institution": "Author institution",
-        "department": "Author Department",
         "biography": "Author test biography"
     }
     attrs.update(kwargs)
@@ -190,6 +189,8 @@ def create_author(journal, **kwargs):
         journal=journal,
         **attrs,
     )
+    author.institution = "Author institution"
+    author.department = "Author Department"
     author.is_active = True
     author.save()
     return author
@@ -353,6 +354,7 @@ def create_preprint(repository, author, subject, title='This is a Test Preprint'
         account=author,
         order=1,
     )
+    preprint_author.save()
     preprint_author.affiliation = 'Made Up University'
     preprint_author.save()
     return preprint