Skip to content

[WIP] Impact Notifications #11096

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 40 commits into
base: feature/pbs-25-07
Choose a base branch
from

Conversation

Johnetordoff
Copy link
Contributor

@Johnetordoff Johnetordoff commented Apr 14, 2025

Purpose

Proof of Concept for improvement of the notification system. This system is designed to formalize and consolidate OSF Notifications under one system in order to better facilate colloboration between Product, Engineers and QA and end persistent problems with notication email related tech debt. This project is the result of an extensive audit of all OSF emails and conbined email digests and seeks to move the NotificationSubscription model out of it's transitiional state having been migrated from a document based model, into it's final data model which is more regular for a relation our current relational database (django's postgress db).

In order to do this I have taken @mfraezz design documents and implemeted them with minor changes. This means the responsibility for sending notifications in osf.io is shared by three new models, which will have the old data migrated into them via a migration script and management command. The models are:

  • NotificationType

    • Similar to RegistrationSchemas or Waffle flags these are in db instrances which are poulated from static config documents in this case yaml.
    • Since these are synced on migration with notification.yaml a developer will be able to see at a glance if where a notification template is and what it's purpose is.
  • NotificationSubscription

    • This model is somewhat analogous to the earlier EmailDigest model, this holds references to potentially many Notifcations models that can be compiled into a single digest this is sent at a periodic basis.
  • Notification

    • Holds message information and context
    • Unlike earlier implementations this will record each Notification creation and sending for metrics purposes.

Changes

  • creates notifications.yaml to contain all notification types info it is populated via postmigrate hook
  • creates new Notification NotificationSubscription and NotificationType
  • adds migrations to rename legacy tables and a managment command to populate the new ones.
  • Deletes old send_mails method and replaces it with emit of NotificationType
  • adds tests and updates old mocking
  • updates views and permission classed

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

@Johnetordoff Johnetordoff changed the base branch from develop to feature/pbs-25-07 April 14, 2025 14:18
@Johnetordoff Johnetordoff force-pushed the impact-notifications branch 15 times, most recently from 5de4a03 to 5bd0830 Compare April 17, 2025 18:44
@Johnetordoff Johnetordoff force-pushed the impact-notifications branch 2 times, most recently from 08ebcc7 to 54f1f93 Compare April 18, 2025 15:46
@Johnetordoff Johnetordoff force-pushed the impact-notifications branch 20 times, most recently from c3f29ab to 9667873 Compare April 29, 2025 15:08
@Johnetordoff Johnetordoff force-pushed the impact-notifications branch from 9667873 to 3f356a4 Compare April 29, 2025 16:02
Vlad0n20 and others added 5 commits April 29, 2025 14:49
…h date uploaded) via the admin app (CenterForOpenScience#11118)

## Purpose
improve exception handling and minor fixes

## Changes
- improve exception handling
- supplemental node permission error
- set subjects from relationship to keep valid hierarchy
- ignore unfinished/unpublished versions

## Ticket
https://openscience.atlassian.net/browse/ENG-7716
…cience/osf.io into impact-notifications

* 'feature/pbs-25-07' of https://github.com/CenterForOpenScience/osf.io:
  [ENG-7289] Fix Search Index Discrepancy in Collection Facets (CenterForOpenScience#11085)
…cience/osf.io into impact-notifications

* 'feature/pbs-25-08' of https://github.com/CenterForOpenScience/osf.io: (28 commits)
  [ENG-7716] Allow for reinstatement of previous preprint versions (with date uploaded) via the admin app (CenterForOpenScience#11118)
  [ENG-7263] Fix/eng 7263 part 3 (CenterForOpenScience#11119)
  [ENG-7263] Part 2 (CenterForOpenScience#11110)
  fix feature for non-contributor admin (CenterForOpenScience#11111)
  [ENG-7716] Allow for reinstatement of previous preprint versions (with date uploaded) via the admin app (CenterForOpenScience#11097)
  delete sharev2 push [ENG-7387] (CenterForOpenScience#11032)
  [ENG-7503] Fix/eng 7503 (CenterForOpenScience#11092)
  [ENG-7263] Fix/eng 7263 (CenterForOpenScience#11090)
  [ENG-7798] Parse versioned guid (CenterForOpenScience#11104)
  [ENG-7270] Enable Product Team to Force Archive Registrations in the Admin App (CenterForOpenScience#11105)
  gdpr deletion shouldn't take into account deleted nodes (CenterForOpenScience#11098)
  Bind task for proper retrying
  improved naming
  use newly built doi for previous versions
  mint missing doi when build metadata
  use minted doi for building metadata
  updated error text
  simplified query
  flat guids
  added version filtering
  ...

# Conflicts:
#	api/crossref/views.py
#	tests/test_events.py
#	tests/test_misc_views.py
@Johnetordoff Johnetordoff force-pushed the impact-notifications branch from fac5151 to 3d6a791 Compare May 1, 2025 14:30
@Johnetordoff Johnetordoff force-pushed the impact-notifications branch from 3d6a791 to a4ca1c8 Compare May 1, 2025 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants