[PM-10564] Push notification updates to other clients

[mzieniukbw]

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-10564

📔 Objective

When a notification is updated, marked as read or deleted, a push notification is sent with updated push type event. The push notification includes the ReadDate and DeletedDate fields.

Notes

• Renamed SyncNotification to SyncNotificationCreate push type event.
• Added SyncNotificationUpdate push type event
• Enabled nullable checks in places in which i have added nullable field
• Creation of notification status sends update push type event. This is because notification status does not serve any purpose without notification itself, they always are come together as pair. This means that the create or update on the status itself is always treated as update to the notification.

📸 Screenshots

⏰ Reminders before review

• Contributor guidelines followed
• All formatters and local linters executed and passed
• Written new unit and / or integration tests where applicable
• Protected functional changes with optionality (feature flags)
• Used internationalization (i18n) for all UI strings
• CI builds passed
• Communicated to DevOps any deployment requirements
• Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

• 👍 (:+1:) or similar for great changes
• 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
• ❓ (:question:) for questions
• 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
• 🎨 (:art:) for suggestions / improvements
• ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
• 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
• ⛏ (:pick:) for minor or nitpick changes

[codecov]

Codecov Report

Attention: Patch coverage is 60.65574% with 48 lines in your changes missing coverage. Please review.

Project coverage is 43.33%. Comparing base (d028029) to head (1285a7e).

Files with missing lines	Patch %	Lines
...es/Implementations/RelayPushNotificationService.cs	0.00%	25 Missing ⚠️
...tations/NotificationsApiPushNotificationService.cs	0.00%	15 Missing ⚠️
...ementations/MultiServicePushNotificationService.cs	61.53%	3 Missing and 2 partials ⚠️
...NoopImplementations/NoopPushNotificationService.cs	0.00%	2 Missing ⚠️
src/Core/Models/PushNotification.cs	80.00%	1 Missing ⚠️

Additional details and impacted files

@@               Coverage Diff               @@
##           km/pm-10600    #5057      +/-   ##
===============================================
+ Coverage        43.30%   43.33%   +0.02%     
===============================================
  Files             1412     1412              
  Lines            64887    64985      +98     
  Branches          5935     5944       +9     
===============================================
+ Hits             28099    28159      +60     
- Misses           35550    35588      +38     
  Partials          1238     1238              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

---

🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests
• JS Bundle Analysis - Avoid shipping oversized bundles

[github-actions]

Checkmarx One – Scan Summary & Details – 9ab96396-f390-4361-a6b9-c7d277b25ad6

New Issues

Severity	Issue	Source File / Package	Checkmarx Insight
	CSRF	/src/Api/Controllers/PushController.cs: 38	Attack Vector

[Thomas-Avery]

Code changes look good.

I have one question around what should happen with individual notification status on a notification update. Should the notification statuses reset since the message is altered, or should they stay the same?

[Thomas-Avery]

🤔
Trying to understand how the client would receive this and determine what to do?

For example:

1. A notification is made for the user
2. The user reads the notification (updates notification status)
3. The notification message is updated
4. This push is triggered, sending back the notification with a null ReadDate and DeletedDate
5. The client shows the notification as unread now from the push?
6. Next time the client hits the API it will pick up the notification status as read?

More generally, do we need to determine between a notification update and a notification status update?