Feature/institutional access #10884

Johnetordoff · 2025-01-03T14:19:50Z

Purpose

Just to ensure the branch is up to snuff.

Changes

QA Notes

Please make verification statements inspired by your code and what your code touches.

Verify
Verify

What are the areas of risk?

Any concerns/considerations/questions that development raised?

Documentation

Side Effects

Ticket

…ser` field

…of 403s etc

…wn address as a reply_to

…-message [ENG-6682] Add UserMessage feature for Institutional Access

…terForOpenScience/osf.io into institutional-access-user-message-arb * 'feature/institutional_access' of https://github.com/CenterForOpenScience/osf.io: add user message read/write permissions to full add new user message oauth scope and throttling classes Fix backfill, report Update changelog and bump versions Follow-up fix for target/next (start/end) month Fix failures caused by base class MonthlyReporter update [ENG-6506] Fix: counted-usage clobbers (#10799) [ENG-6435] Fix: duplicate reports when run for past years (#10800) Add PrivateSpamMetricsReport (#10791) [ENG-4438] Add OOPSpam and Akismet metrics to spam report (#10783) [ENG-6364] Migrate Preprint Affilations (#10787)

…-message-arb [ENG-6682] Add reply-to and cc-ing features to Institutional Access

…terForOpenScience/osf.io into institutional-access-node-request-improvements * 'feature/institutional_access' of https://github.com/CenterForOpenScience/osf.io: change to bcc the sender instead of CC-ing them revert typo add code to allow cc-ing fellow institutional admins and put their own address as a reply_to # Conflicts: # osf/migrations/0025_noderequest_requested_permissions_and_more.py

…e request access is turned off and added test case

…-request-improvements [ENG-6666] NodeRequest improvements for Institutional Access Project

into feature/institutional_access * 'develop' of https://github.com/CenterForOpenScience/osf.io: Assume default for global_ notifications Update CHANGELOG, bump version [Feature] Dashboard B&I (#10843)

[Bug} Fix sendgrid email for Institutional Access

…to sentry

[BUG][WIP] Fix sendgrid email 400s and 202s reporting to sentry

add fix to email mocking, test for catagories

Keep institutional_access feature branch up to date with develop

…ntributor_page [ENG-6670] Make the requested permissions show up as defaults

Add to admin "can_request_access" option to the institution API Changes for "can_request_access" feature Done 2 tasks (ENG-6779) and (ENG-6780) within one PR, due to tickets being related to each other

…#10825) ## Purpose The new Institutional Access feature creates a new "Curator" role that has certain requirements. The first is that it have a special label in the contributor list, shown here: The second requirement is that the user is unable to set the curator to a bibliographic or "visible contributor. ## Changes - Add check constraint to avoid visible institutional admins. - Locates area in page where Curator status can be lableled - adds tests to test for specific behavior at model and API level - adds extra UI notice for contributor page.

* rename migrations for readbility * fix serialization of `institutional_request_access_enabled` to not be string --------- Co-authored-by: John Tordoff <>

Johnetordoff · 2025-01-14T17:21:30Z

osf/models/mixins.py

@@ -1839,7 +1845,11 @@ def set_visible(self, user, visible, log=True, auth=None, save=False):
        if visible and not self.contributor_class.objects.filter(**kwargs).exists():
            set_visible_kwargs = kwargs
            set_visible_kwargs['visible'] = False
-            self.contributor_class.objects.filter(**set_visible_kwargs).update(visible=True)
+            contribs = self.contributor_class.objects.filter(**set_visible_kwargs)
+            if self.guardian_object_type == 'node' and contribs.filter(is_curator=True).exists():


This is less then ideal. Potentially this method is a clean-up candidate.

Johnetordoff · 2025-01-14T17:22:17Z

osf/models/user.py

+            name__endswith='_institutional_admins'
+        ).exists()
+
+    def is_institutional_curator(self, node):


Johnetordoff · 2025-01-14T17:23:24Z

osf/models/user_message.py

+        }[message_type]
+
+
+class UserMessage(BaseModel, ObjectIDMixin):


Update the docstrings and maybe help_text

Johnetordoff · 2025-01-14T17:24:06Z

osf/models/user_message.py

+        )
+
+
+@receiver(post_save, sender=UserMessage)


Just override .create method? post-commit?

Johnetordoff · 2025-01-14T17:25:19Z

website/mails/mails.py

@@ -76,17 +76,29 @@ def render_message(tpl_name, **context):


 def send_mail(


Check docstings and debugging/logging

Johnetordoff · 2025-01-15T13:18:43Z

osf/models/contributor.py

@@ -41,6 +42,12 @@ class Meta:
        # NOTE: Adds an _order column
        order_with_respect_to = 'node'

+    def save(self, *args, **kwargs):
+        if self.is_curator and self.visible:


I feel like we might have reduced the complexity of this to the point where a DB constraint actually is faster the overriding save.

[ENG-6882]fixed double mail sending

Johnetordoff · 2025-01-15T20:30:48Z

osf/utils/machines.py

-                osf_contact_email=OSF_CONTACT_EMAIL,
-                **context
-            )
+        if not self.machineable.request_type == NodeRequestTypes.INSTITUTIONAL_REQUEST.value:


Potentially rework into NodeRequest object better

Johnetordoff · 2025-01-16T18:43:21Z

api/requests/permissions.py

@@ -32,7 +36,7 @@ def has_object_permission(self, request, view, obj):
            raise ValueError(f'Not a request-related model: {obj}')

        if not node.access_requests_enabled:
-            return False


clean-up later conditionals?

Johnetordoff · 2025-01-16T18:43:33Z

api/requests/permissions.py

@@ -52,7 +56,35 @@ def has_object_permission(self, request, view, obj):
                # Requesters may not be contributors
                # Requesters may edit their comment or submit their request
                return is_requester and auth.user not in node.contributors
-            return False


Might be fine...

api/requests/serializers.py

api/users/serializers.py

api/institutions/serializers.py

api/users/views.py

api/requests/serializers.py

mfraezz · 2025-01-16T20:39:53Z

framework/email/tasks.py

+        msg['Reply-To'] = reply_to
+
+    # Combine recipients for SMTP
+    recipients = [to_addr] + (bcc_addr or [])


Minor, as we're not using basic SMTP: while this is correct behavior, the docs allude to sendmail supporting a msg['Bcc'] field, similar to the above msg['To'], etc. It may be preferable to include that in a conditional.

Minor, as we're not using basic SMTP:

I would really like to remove this code then, how do feel about just deleting the SMTP code? I thought it was a fail safe, but I think it would actually break in Production.

There's a conditional at the top that circuit-breaks sending via SMTP without credentials. I'm not especially partial to either keeping or removing, but removing seems out of scope for this project.

@felliott

## Purpose Make some recommended changes from our CR group. Thanks to @felliott and others for keen eyes. ## Changes - Squash migrations - fix typos

John Tordoff and others added 30 commits December 5, 2024 14:41

Add new UserMessage feature for Institutional Access

edb3118

Respond to CR by changing permissions logic and removing vestigial `u…

cbcd763

…ser` field

expand tests fix typos, align email templates and improve help text

ec0d147

clean-up serializer/permission code to validate better, 409s instead …

cd45df2

…of 403s etc

add code to allow cc-ing fellow institutional admins and put their o…

25d7857

…wn address as a reply_to

add new user message oauth scope and throttling classes

f3ab16b

add user message read/write permissions to full

6b8c81c

Merge pull request #10824 from Johnetordoff/institutional-access-user…

9831d86

…-message [ENG-6682] Add UserMessage feature for Institutional Access

revert typo

dc2331e

change to bcc the sender instead of CC-ing them

8de394a

Merge pull request #10841 from Johnetordoff/institutional-access-user…

7068791

…-message-arb [ENG-6682] Add reply-to and cc-ing features to Institutional Access

add NodeRequest improvements for Institutional Access

b7405af

fix up tests

7359973

clean-up docstrings and typos

4d16ab5

re-introduce in email template

a024264

tweak permissions for 400 error code and add test cases

e34b928

fix typo

8b5c183

split off node request file again, added descriptive message when nod…

efc5dd2

…e request access is turned off and added test case

Merge pull request #10826 from Johnetordoff/institutional-access-node…

66df96c

…-request-improvements [ENG-6666] NodeRequest improvements for Institutional Access Project

Merge branch 'develop' of https://github.com/CenterForOpenScience/osf.io

8c65e13

into feature/institutional_access * 'develop' of https://github.com/CenterForOpenScience/osf.io: Assume default for global_ notifications Update CHANGELOG, bump version [Feature] Dashboard B&I (#10843)

allow BCC and reply emails to work with sendgrid

be8f711

Merge pull request #10859 from Johnetordoff/fix-sendgrid-email

c99d066

[Bug} Fix sendgrid email for Institutional Access

try fixing sendgrid agian to not send 400 and fix to make 202 not go …

79135e4

…to sentry

Merge pull request #10860 from Johnetordoff/fix-sendgrid-email

4fef0d0

[BUG][WIP] Fix sendgrid email 400s and 202s reporting to sentry

add fix to email mocking, test for catagories

60eb88f

Merge pull request #10890 from Johnetordoff/fix-mail-mocks

4611112

add fix to email mocking, test for catagories

Merge pull request #10897 from bodintsov/develop

d34ceb8

Keep institutional_access feature branch up to date with develop

Make the requested permissions show up as defaults

06e7198

brianjgeiger and others added 4 commits January 8, 2025 13:18

Merge pull request #10891 from bodintsov/feature/changed_to_legacy_co…

d5138e0

…ntributor_page [ENG-6670] Make the requested permissions show up as defaults

Changes for "can_request_access" feature (#10877)

d9540f3

Add to admin "can_request_access" option to the institution API Changes for "can_request_access" feature Done 2 tasks (ENG-6779) and (ENG-6780) within one PR, due to tickets being related to each other

[IAC][Bugs] Fix migrations and serializer (#10915)

83a6049

* rename migrations for readbility * fix serialization of `institutional_request_access_enabled` to not be string --------- Co-authored-by: John Tordoff <>

Johnetordoff commented Jan 14, 2025

View reviewed changes

osf/models/user_message.py

)

@receiver(post_save, sender=UserMessage)

Copy link

Contributor Author

Johnetordoff Jan 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just override .create method? post-commit?

Johnetordoff commented Jan 14, 2025

View reviewed changes

bodintsov added 3 commits January 14, 2025 20:31

fixed double mail sending

8f79cfd

reverted changes to test_node_request_list

d007d63

added testcases to check email send once

b268003

Johnetordoff commented Jan 15, 2025

View reviewed changes

bodintsov and others added 3 commits January 15, 2025 16:57

removed useless code

bb243a1

refactored test

979b157

Merge pull request #10919 from bodintsov/feature/double_email_fix

e5cc7fe

[ENG-6882]fixed double mail sending

Johnetordoff commented Jan 15, 2025

View reviewed changes

Johnetordoff commented Jan 16, 2025

View reviewed changes

felliott reviewed Jan 16, 2025

View reviewed changes

api/requests/serializers.py Outdated Show resolved Hide resolved

felliott reviewed Jan 16, 2025

View reviewed changes

api/users/serializers.py Outdated Show resolved Hide resolved

Johnetordoff commented Jan 16, 2025

View reviewed changes

api/institutions/serializers.py Show resolved Hide resolved

mfraezz reviewed Jan 17, 2025

View reviewed changes

[IAC][Group CR][ENG-6751] (#10923)

d75b026

## Purpose Make some recommended changes from our CR group. Thanks to @felliott and others for keen eyes. ## Changes - Squash migrations - fix typos

brianjgeiger marked this pull request as ready for review January 17, 2025 20:34

brianjgeiger approved these changes Jan 17, 2025

View reviewed changes

brianjgeiger changed the title ~~[Reference][WIP] Feature/institutional access~~ Feature/institutional access Jan 21, 2025

brianjgeiger merged commit 94be963 into develop Jan 21, 2025
17 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/institutional access #10884

Feature/institutional access #10884

Johnetordoff commented Jan 3, 2025

Johnetordoff Jan 14, 2025

Johnetordoff Jan 14, 2025 •

edited

Loading

Johnetordoff Jan 14, 2025

Johnetordoff Jan 14, 2025

Johnetordoff Jan 14, 2025

Johnetordoff Jan 15, 2025

Johnetordoff Jan 15, 2025

Johnetordoff Jan 16, 2025

Johnetordoff Jan 16, 2025

mfraezz Jan 16, 2025

Johnetordoff Jan 17, 2025

mfraezz Jan 17, 2025

		@@ -76,17 +76,29 @@ def render_message(tpl_name, **context):


		def send_mail(

Feature/institutional access #10884

Feature/institutional access #10884

Conversation

Johnetordoff commented Jan 3, 2025

Purpose

Changes

QA Notes

Documentation

Side Effects

Ticket

Choose a reason for hiding this comment

Johnetordoff Jan 14, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Johnetordoff Jan 14, 2025 •

edited

Loading