[ENG-8064] Notification Refactor Phase 2 #11322

cslzchen · 2025-09-18T15:24:50Z

Purpose

TBD

Changes

TBD

QA Notes

TBD

Documentation

TBD

Side Effects

TBD

Ticket

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

--------- Co-authored-by: John Tordoff <[email protected]> Co-authored-by: Ostap Zherebetskyi <[email protected]> Co-authored-by: Bohdan Odintsov <[email protected]> Co-authored-by: John Tordoff <>

* Fix moderator digest * Fix unit tests

…ion (#11324) * Fix issue with user confirmation/merger creating subscription * Add docstrings

* Add No Login to notification template and add to tests

…tion-refactor-p2-s * Conflicts have not been resolved in this merge since we want to track how we fixed the conflicts due to complexity * api/nodes/serializers.py * osf/models/user.py * tests/test_auth.py * tests/test_webtests.py * website/templates/node_request_institutional_access_request.html.mako

…tically

…plate-with-naming [ENG-8988] Fix/notification contrib template with naming

[ENG-8997] Turn on script tests

addons/boa/tests/test_tasks.py

addons/osfstorage/tests/test_models.py

api/providers/tasks.py

api_tests/draft_registrations/views/test_draft_registration_contributor_list.py

api_tests/institutions/views/test_institution_auth.py

opaduchak · 2025-10-17T13:13:38Z

tests/test_webtests.py

@@ -467,631 +462,6 @@ def test_wiki_url(self):
            assert self._url_to_body(self.wiki.deep_url) == self._url_to_body(self.wiki.url)


-@pytest.mark.enable_bookmark_creation


Please confirm this to be intended removal

tests/utils.py

opaduchak · 2025-10-17T13:17:46Z

website/conferences/views.py

@@ -125,36 +82,6 @@ def add_poster_by_email(conference, message):

    utils.upload_attachments(user, node, message.attachments)

-    download_url = node.web_url_for(


are these removals intended?

Yes this feature is depreciated

[ENG-8996] Fix moderator added template

opaduchak

Several leftover questions:
General opinion on templates:
We need to look through each one, identify redundant variables in event context and remove them. Also we need to verify that all required variables are indeed passed to templates.

This review contains only several topmost templates, therefore there supposedly are many more issues than comments

opaduchak · 2025-10-21T10:46:35Z

website/project/views/comment.py

@@ -15,6 +15,7 @@

 @file_updated.connect
 def update_file_guid_referent(self, target, event_type, payload, user=None):
+    event_type = payload['action']


do we even need event_type parameter in this case?
I guess we should remove it or at least make it optional

It's necessary because somewhere in the waterbulter get_auth logic the event_type gets overridden and in those other signals receivers the distinction matters, but not in this signal.

opaduchak · 2025-10-21T11:11:28Z

osf/utils/machines.py

        context['contributors_url'] = f'{self.machineable.target.absolute_url}contributors/'
        context['project_settings_url'] = f'{self.machineable.target.absolute_url}settings/'


These are not needed, they are not present in template

Yep they should be removed

This was addressed in the PR "Fix NR Review Comments."

website/templates/add_sso_email_osf4i.html.mako

opaduchak · 2025-10-21T11:14:15Z

website/templates/addons_boa_job_complete.html.mako

@@ -7,7 +7,7 @@
 </tr>
 <tr>
  <td style="border-collapse: collapse;">
-      Hello ${fullname},<br>
+      Hello ${user_fullname},<br>


during invocation, fullname is passed instead of user_fullname

Ok we will fix the mismatch

This was addressed in the PR "Fix NR Review Comments."

opaduchak · 2025-10-21T11:17:51Z

website/templates/archive_copy_error_desk.html.mako

results are not being passed during invocation

opaduchak · 2025-10-21T11:19:51Z

website/archiver/utils.py

+        event_context={
+            'domain': settings.DOMAIN,
+            'user_fullname': user.fullname,
+            'user__id': user._id,
+            'src__id': src._id,
+            'src_url': src.url,
+            'src_title': src.title,
+            'results': results,
+            'url': url,
+            'can_change_preferences': False,
+        }


We need only src_title here

ok we will remove the extraneous arguments

This was addressed in the PR "Fix NR Review Comments."

[ENG-9005] Fix/update v2 subscriptions

…tests [ENG-9603] Fix Reset Password Tests

…om/centerforopenscience/osf.io into fix-test-add-contributors-notifications

…-notifications [ENG-9600][ENG-9606] Notification refactor fix contributor adding issues

…om/centerforopenscience/osf.io into fix-institutional-request-templates

…-templates [ENG-9605] Fix institutional node request updates template

…cations [ENG-9713] new_public_project notifications to happen everytime a project is made public

…e to their new table

… table v2

…dd default message frequency

… contributors

…file-subs [ENG-9723] Fix issue with duplicate file update subscriptions

…igrations [ENG-9719] Fix issues where legacy notification subscription settings didn't mov…

…olute_url`

[ENG-9673] Fix spelling in template

[ENG-9675] Fix collection template link

[ENG-9674] Fix password reset link

…_email [ENG-9670] Moderator Notifications: Moderator added email notifications doesn't include any links to provider

…notification_subscriptions [ENG-9738] fix migrate_legacy_notification_subscriptions

cslzchen and others added 17 commits September 18, 2025 11:22

Squash Merge Notification Refactor Phase 2 (#11321)

0ae75d0

--------- Co-authored-by: John Tordoff <[email protected]> Co-authored-by: Ostap Zherebetskyi <[email protected]> Co-authored-by: Bohdan Odintsov <[email protected]> Co-authored-by: John Tordoff <>

[ENG-8996] Fix moderator digest (#11319)

5e29285

* Fix moderator digest * Fix unit tests

[ENG-8978] Fix issue with user confirmation/merger creating subscript…

ffa65ec

…ion (#11324) * Fix issue with user confirmation/merger creating subscription * Add docstrings

[ENG-8995] Fix issue with no login not being sent (#11326)

d24cb2c

* Add No Login to notification template and add to tests

Update v2 subscription API for Ag and new data

cf68080

fix issues with the API backward compatibility

caca24c

Fix merge conflicts as a result of merge commit 7975650

6c987b9

fix template language

8d55887

reorganize templates to deal with conditional statement better gramma…

cc84096

…tically

Fix templates

cce5b22

Merge pull request #11370 from bodintsov/fix/notification-contrib-tem…

491b828

…plate-with-naming [ENG-8988] Fix/notification contrib template with naming

fix unit tests

469ea23

Turn on script tests

7969612

fix merge conflict issues

b48fc7e

Merge pull request #11372 from Ostap-Zherebetskyi/fix/script_tests

25ac360

[ENG-8997] Turn on script tests

fix email template mismatch

38d6315

opaduchak suggested changes Oct 17, 2025

View reviewed changes

Merge pull request #11328 from Johnetordoff/fix-moderator-added-template

fd453ec

[ENG-8996] Fix moderator added template

opaduchak reviewed Oct 21, 2025

View reviewed changes

Johnetordoff and others added 10 commits October 22, 2025 08:34

Merge pull request #11329 from Johnetordoff/fix/update-v2-subscriptions

0b7fe79

[ENG-9005] Fix/update v2 subscriptions

fix merge conflicts for unregistered contributors

e477dcd

remove assertions

0766a7e

Merge pull request #11388 from Ostap-Zherebetskyi/fix/password_reset_…

b2440ac

…tests [ENG-9603] Fix Reset Password Tests

Merge branch 'feature/notification-refactor-p2-s' of https://github.c…

06f93dc

…om/centerforopenscience/osf.io into fix-test-add-contributors-notifications

fix institutional node request updates template

b6f5305

Merge pull request #11384 from Johnetordoff/fix-test-add-contributors…

a741876

…-notifications [ENG-9600][ENG-9606] Notification refactor fix contributor adding issues

Merge branch 'feature/notification-refactor-p2-s' of https://github.c…

faaf112

…om/centerforopenscience/osf.io into fix-institutional-request-templates

Merge pull request #11389 from Johnetordoff/fix-institutional-request…

89ae33b

…-templates [ENG-9605] Fix institutional node request updates template

fix test_non_contributor_create_project_from_public_template_success

f3aa95c

Johnetordoff and others added 30 commits November 5, 2025 10:48

Merge pull request #11421 from Johnetordoff/add-public-project-notifi…

e3da416

…cations [ENG-9713] new_public_project notifications to happen everytime a project is made public

fix issues where legacy notification subscription settings didn't mov…

bfebd2e

…e to their new table

add extra fields for migration

ad0ebfd

add more statements altering tables to separate out legacy data

2a5bff0

improve migrate notifications migration and command

3a0c583

use better migration naming convention

e18b744

use correct query for backfill migration

d3aaf14

fix comments for migration noties

ff29e91

fix tests still using model relationships

e3edf96

add file from previous commit I forgot

1fd26ff

add node subscription tests and make migration reversible by renaming…

6f15188

… table v2

use better more static raw SQL query for retrieving legacy data

fcd6d29

fix digest tests with new naming scheme for notifications refactor

95a52ca

replace bulk create for file subscriptions with get_or_create and a…

e33c0f6

…dd default message frequency

don't create necessary subscriptions, but default to instant delivery

d5c457a

add tests ensuring the correct subscriptions for registered and unreg…

0d050d2

… contributors

Merge pull request #11426 from Johnetordoff/fix-issue-with-duplicate-…

b1f0527

…file-subs [ENG-9723] Fix issue with duplicate file update subscriptions

fix migrate_legacy_notification_subscriptions

04a68ad

Merge pull request #11424 from Johnetordoff/fix-legacy-notification-m…

9398855

…igrations [ENG-9719] Fix issues where legacy notification subscription settings didn't mov…

fix notification template spelling error

7109646

fix spelling mistake in fixed mistake

d202525

change url from api link to web link, though the are both called `abs…

6b3b582

…olute_url`

fix password reset link

2ba08ad

Merge pull request #11428 from Johnetordoff/fix-spelling-in-template

0ab798a

[ENG-9673] Fix spelling in template

add provider url

2086a40

Merge pull request #11429 from Johnetordoff/fix-collection-template-link

9870b69

[ENG-9675] Fix collection template link

Merge pull request #11430 from Johnetordoff/fix-password-reset-link

85b8434

[ENG-9674] Fix password reset link

Merge pull request #11431 from Ostap-Zherebetskyi/fix/moderator_added…

a021957

…_email [ENG-9670] Moderator Notifications: Moderator added email notifications doesn't include any links to provider

fix TestNotificationSubscriptionMigration

f8961b5

Merge pull request #11432 from Ostap-Zherebetskyi/fix/migrate_legacy_…

7741716

…notification_subscriptions [ENG-9738] fix migrate_legacy_notification_subscriptions

		@@ -467,631 +462,6 @@ def test_wiki_url(self):
		assert self._url_to_body(self.wiki.deep_url) == self._url_to_body(self.wiki.url)


		@pytest.mark.enable_bookmark_creation

		@@ -125,36 +82,6 @@ def add_poster_by_email(conference, message):

		utils.upload_attachments(user, node, message.attachments)

		download_url = node.web_url_for(

		context['contributors_url'] = f'{self.machineable.target.absolute_url}contributors/'
		context['project_settings_url'] = f'{self.machineable.target.absolute_url}settings/'

[ENG-8064] Notification Refactor Phase 2 #11322

Are you sure you want to change the base?

[ENG-8064] Notification Refactor Phase 2 #11322

Uh oh!

Conversation

cslzchen commented Sep 18, 2025

Purpose

Changes

QA Notes

Documentation

Side Effects

Ticket

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

opaduchak left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

opaduchak left a comment •

edited

Loading