[ENG-6682] Add UserMessage feature for Institutional Access #10824

Johnetordoff · 2024-12-05T12:53:44Z

Purpose

This is to allow institutional admins limited messaging capabilities for users within their institution. This PR should allow an email message be sent to an institution member by an admin, using the UserMessage system.

Changes

adds UserMessagePermissions
adds UserMessage model with MessageType choices
adds unit tests
adds UserMessageView
adds UserMessageSerializer
adds is_institutional_admin method to user model
adds UserMessageFactory
adds USER_MESSAGE_INSTITUTIONAL_ACCESS_REQUEST email template

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

https://openscience.atlassian.net/browse/ENG-6682

Bits discussed in at ARB:

#10841

Johnetordoff · 2024-12-05T13:54:38Z

api/users/serializers.py

+            },
+        )
+
+    def to_internal_value(self, data):


this was caused by using the RelationshipField it doesn't handle POSTing to relationships well.

brianjgeiger

A few changes requested. Also, I'm not seeing some of the things mentioned in ARB. For example, did you ask product have having a "cc the sender" option, or did they say no to that? I also didn't see any rate limiting on the API call. Please go over the list of suggestions from ARB and make sure that they're all either disapproved by Product or implemented here.

api/users/permissions.py

brianjgeiger · 2024-12-06T20:24:12Z

api/users/permissions.py

+        if message_type == MessageTypes.INSTITUTIONAL_REQUEST:
+            return self._validate_institutional_request(request, user)


Is this the right place for this check? Again, it would give a 403, when a 409 Conflict or just a straight 400 might be more appropriate.

Since currently this only applies to insti admin, I'll make that check here and move the institution validation and other details to the serializer. Good comment thanks.

api/users/permissions.py

api/users/serializers.py

api_tests/users/views/test_user_message_institutional_access.py

osf/models/user_message.py

brianjgeiger · 2024-12-06T20:47:10Z

website/templates/emails/user_message_institutional_access_request.html.mako

+        <p>
+            To review this request, please visit your project dashboard or contact your institution administrator for further details.
+        </p>


This isn't accurate for this kind of email. Please check with product to verify what the text of the email surrounding the comment will be when it's not a request for access message. Also, the name of the email template is not correct for this feature, since it's not a request for access message.

Sorry about that I simply copied the wrong message Product gave me, here's their docs if you want to confirm scroll down to the end. https://docs.google.com/document/d/1ZVbfohipKiJrMeRQIp3xCIdO_l9kyMVPui1-CP0teBs/edit?tab=t.0

…ser` field

Johnetordoff · 2024-12-06T22:05:52Z

@brianjgeiger For the thing mentioned in the ARB, I assumed that these would come after the the other aspects have been approved. I'd like to make the reply-to header and ccing sender a separate PR, because it's not MVP. I also need to do more research I'd like to confirm the Django stuff we know before we add a new feature that hasn't been tested before. But I'm open to putting it in this PR. I'd just like to isolate that functionality in a new PR after this, because I'm not too familiar with reply-to headers it and I want to include a testing suite which is a little scope creeping here.

Johnetordoff · 2024-12-06T22:06:23Z

Maybe I can cc here, but reply-to in another PR?

…of 403s etc

brianjgeiger · 2024-12-09T16:15:17Z

Okay, that's fine.

brianjgeiger · 2024-12-09T16:16:49Z

@Johnetordoff The cc: functionality was only supposed to go to the sender. It was a way of giving the sender confirmation that the message was sent and what the contents of the message was. It was not meant to go to the other institutional admins at all.

brianjgeiger · 2024-12-09T16:20:07Z

Also, did you ask Product about the cc functionality, or did you just add it? I didn't see any questions in slack about any of the ARB ideas.

Johnetordoff · 2024-12-09T16:24:49Z

@brianjgeiger I spoke with Product and had a meeting with Blaine right after the ARB and she was enthusiastic about adding the reply-to and cc-ing features, agreed with the developers that they seemed like a good idea anyway. The "just message" for the Project tab they supported to and said that they would supoort any UI changes for that.

I'll try to keep some of this discussion in the slack. Sorry about that.

brianjgeiger

Almost there. A couple more changes to views, and maybe another test.

Oh, also, could you make a feature/institutional_access branch and point this to that, rather than pointing it directly to develop?

brianjgeiger · 2024-12-10T14:50:37Z

api/users/views.py

+    required_read_scopes = [CoreScopes.USERS_READ]
+    required_write_scopes = [CoreScopes.USERS_WRITE]
+    parser_classes = (JSONAPIMultipleRelationshipsParser, JSONAPIMultipleRelationshipsParserForRegularJSON)
+
+    serializer_class = UserMessageSerializer
+


This view should have Throttle classes set per ARB. Probably just needs:

throttle_classes = [BurstRateThrottle, SendEmailThrottle]

brianjgeiger · 2024-12-10T14:54:16Z

api/users/views.py

+        UserMessagePermissions,
+    )
+
+    required_read_scopes = [CoreScopes.USERS_READ]


This probably needs USER_EMAIL_READ as well.

framework/auth/oauth_scopes.py

Johnetordoff mentioned this pull request Dec 5, 2024

[ENG-6665] Add user messaging modal to user's tab on institutional dashboard CenterForOpenScience/ember-osf-web#2421

Merged

Johnetordoff changed the title ~~[ENG-6682] Add new UserMessage feature for Institutional Access~~ [ENG-6682] Add UserMessage feature for Institutional Access Dec 5, 2024

Johnetordoff commented Dec 5, 2024

View reviewed changes

Johnetordoff force-pushed the institutional-access-user-message branch from ce125ed to f7a73c2 Compare December 5, 2024 19:40

Add new UserMessage feature for Institutional Access

edb3118

Johnetordoff force-pushed the institutional-access-user-message branch from f7a73c2 to edb3118 Compare December 5, 2024 19:41

Johnetordoff marked this pull request as ready for review December 5, 2024 22:02

brianjgeiger requested changes Dec 6, 2024

View reviewed changes

John Tordoff added 2 commits December 6, 2024 16:07

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

Johnetordoff force-pushed the institutional-access-user-message branch from 9fc47ae to cd45df2 Compare December 6, 2024 22:07

Johnetordoff force-pushed the institutional-access-user-message branch 4 times, most recently from b97e597 to cd45df2 Compare December 10, 2024 14:05

Johnetordoff mentioned this pull request Dec 10, 2024

[ENG-6682] Add CCing and Reply-to features to Institutional Access User Messaging Johnetordoff/osf.io#240

Closed

brianjgeiger requested changes Dec 10, 2024

View reviewed changes

Johnetordoff changed the base branch from develop to feature/institutional_access December 10, 2024 15:15

add new user message oauth scope and throttling classes

f3ab16b

Johnetordoff force-pushed the institutional-access-user-message branch from c91534e to f3ab16b Compare December 10, 2024 15:25

brianjgeiger reviewed Dec 10, 2024

View reviewed changes

framework/auth/oauth_scopes.py Show resolved Hide resolved

add user message read/write permissions to full

6b8c81c

brianjgeiger approved these changes Dec 11, 2024

View reviewed changes

Johnetordoff mentioned this pull request Dec 11, 2024

[ENG-6682] Add reply-to and cc-ing features to Institutional Access #10841

Merged

Johnetordoff merged commit 9831d86 into CenterForOpenScience:feature/institutional_access Dec 11, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ENG-6682] Add UserMessage feature for Institutional Access #10824

[ENG-6682] Add UserMessage feature for Institutional Access #10824

Johnetordoff commented Dec 5, 2024 •

edited

Loading

Johnetordoff Dec 5, 2024

brianjgeiger left a comment

brianjgeiger Dec 6, 2024

Johnetordoff Dec 6, 2024 •

edited

Loading

brianjgeiger Dec 6, 2024

Johnetordoff Dec 6, 2024

Johnetordoff commented Dec 6, 2024

Johnetordoff commented Dec 6, 2024

brianjgeiger commented Dec 9, 2024

brianjgeiger commented Dec 9, 2024

brianjgeiger commented Dec 9, 2024

Johnetordoff commented Dec 9, 2024 •

edited

Loading

brianjgeiger left a comment

brianjgeiger Dec 10, 2024

brianjgeiger Dec 10, 2024

		if message_type == MessageTypes.INSTITUTIONAL_REQUEST:
		return self._validate_institutional_request(request, user)

[ENG-6682] Add UserMessage feature for Institutional Access #10824

[ENG-6682] Add UserMessage feature for Institutional Access #10824

Conversation

Johnetordoff commented Dec 5, 2024 • edited Loading

Purpose

Changes

QA Notes

Documentation

Side Effects

Ticket

Bits discussed in at ARB:

Johnetordoff Dec 5, 2024

Choose a reason for hiding this comment

brianjgeiger left a comment

Choose a reason for hiding this comment

brianjgeiger Dec 6, 2024

Choose a reason for hiding this comment

Johnetordoff Dec 6, 2024 • edited Loading

Choose a reason for hiding this comment

brianjgeiger Dec 6, 2024

Choose a reason for hiding this comment

Johnetordoff Dec 6, 2024

Choose a reason for hiding this comment

Johnetordoff commented Dec 6, 2024

Johnetordoff commented Dec 6, 2024

brianjgeiger commented Dec 9, 2024

brianjgeiger commented Dec 9, 2024

brianjgeiger commented Dec 9, 2024

Johnetordoff commented Dec 9, 2024 • edited Loading

brianjgeiger left a comment

Choose a reason for hiding this comment

brianjgeiger Dec 10, 2024

Choose a reason for hiding this comment

brianjgeiger Dec 10, 2024

Choose a reason for hiding this comment

Johnetordoff commented Dec 5, 2024 •

edited

Loading

Johnetordoff Dec 6, 2024 •

edited

Loading

Johnetordoff commented Dec 9, 2024 •

edited

Loading