PM-10560: Adding Cascades back to Notification Center

[mzieniukbw]

🎟️ Tracking

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

📔 Objective

• Fix inability to drop organization or user, due to missing cascades.
  In context of [PM-10560] Create notification database storage #4688 (comment)
• Added missing foreign key in NotificationStatus for Notification with cascades

📸 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 0% with 6 lines in your changes missing coverage. Please review.

Project coverage is 41.78%. Comparing base (a19fc6a) to head (feca45a).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...dminConsole/Repositories/OrganizationRepository.cs	0.00%	4 Missing ⚠️
...ure.EntityFramework/Repositories/UserRepository.cs	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4769      +/-   ##
==========================================
- Coverage   41.78%   41.78%   -0.01%     
==========================================
  Files        1308     1308              
  Lines       62055    62061       +6     
  Branches     5716     5716              
==========================================
  Hits        25930    25930              
- Misses      34931    34937       +6     
  Partials     1194     1194              

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

[github-actions]

Checkmarx One – Scan Summary & Details – ef4776c2-b7fb-469c-9d8f-4332ad5f7448

New Issues

Severity	Issue	Source File / Package	Checkmarx Insight
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 478	Attack Vector
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 176	Attack Vector
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 503	Attack Vector
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 554	Attack Vector
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 540	Attack Vector
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 478	Attack Vector
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 272	Attack Vector
	CSRF	/src/Api/Billing/Public/Controllers/OrganizationController.cs: 47	Attack Vector
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 445	Attack Vector
	Missing_CSP_Header	/src/Core/MailTemplates/Handlebars/TrialInitiation.html.hbs: 47	Attack Vector

Fixed Issues

Severity	Issue	Source File / Package
	CSRF	/src/Identity/Controllers/AccountsController.cs: 75
	CSRF	/src/Api/Vault/Controllers/FoldersController.cs: 64
	CSRF	/src/Api/Vault/Controllers/FoldersController.cs: 45
	CSRF	/src/Api/Vault/Controllers/FoldersController.cs: 45
	CSRF	/src/Identity/Controllers/AccountsController.cs: 75
	CSRF	/src/Api/Vault/Controllers/FoldersController.cs: 64
	CSRF	/src/Api/Vault/Controllers/FoldersController.cs: 64

[mzieniukbw]

I have changed the Primary key columns order to be consistent with what we have in SQL Server, by changing EF src/Infrastructure.EntityFramework/NotificationCenter/Configurations/NotificationStatusEntityTypeConfiguration.cs
But now MySql migration does not work correctly, due to:

Applying migration '20240909133252_NotificationCenter'.
Applying migration '20240916193337_UpdateNotificationCenterOnDeleteCascade'.
Failed executing DbCommand (3ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
CALL POMELO_BEFORE_DROP_PRIMARY_KEY(NULL, 'NotificationStatus');
ALTER TABLE `NotificationStatus` DROP PRIMARY KEY;
MySqlConnector.MySqlException (0x80004005): Cannot drop index 'PRIMARY': needed in a foreign key constraint

Should i revert it ? But then we have inconsistencies between Sql Server and EF ?
Or maybe we need some manual migration for EF MySQL ?
Or maybe i change the order for Sql server ?
Any idea ?

[withinfocus]

There shouldn't be cascades at all now, since your procs for MSSQL and EF logic elsewhere handles it.

[Thomas-Avery]

LGTM

[rkac-bw]

lgtm