[PM 30100][Server] Subscription Discount Database Infrastructure by cyprain-okeke · Pull Request #6936 · bitwarden/server

cyprain-okeke · 2026-02-02T16:10:54Z

🎟️ Tracking

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

📔 Objective

Improve type safety for StripeProductIds in the SubscriptionDiscount entity by changing it from a raw JSON string (string?) to a strongly-typed collection (ICollection?).

This change was suggested during code review to:

Provide compile-time type safety instead of runtime JSON parsing
Make the API more discoverable (IntelliSense shows it's a collection)
Prevent malformed JSON bugs
Simplify business logic (no manual JSON serialization/deserialization needed)

🔧 Changes Made

Core Entity

Changed StripeProductIds from string? to ICollection? in SubscriptionDiscount.cs
Data Access Layer
EF Core: Added HasConversion() in SubscriptionDiscountEntityTypeConfiguration to automatically serialize/deserialize collections to/from JSON
Dapper: Created JsonCollectionTypeHandler and registered it in BaseRepository for JSON conversion

Tests

Updated integration test parameter types and test data to use actual collections (e.g., new List { "prod_123" } instead of "["prod_123"]")
Added comprehensive unit tests in SubscriptionDiscountTests.cs to verify JSON serialization/deserialization, null handling, and empty collections

📸 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

github-actions · 2026-02-02T16:21:49Z

Checkmarx One – Scan Summary & Details – dc07cb42-ba50-4dea-b15b-b2441f7dec4c

New Issues (2)

Checkmarx found the following issues in this Pull Request

#	Severity	Issue	Source File / Package	Checkmarx Insight
1		CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 1527	details Method at line 1527 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ... Attack Vector
2		CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 1403	details Method at line 1403 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ... Attack Vector

Fixed Issues (4)

Great job! The following issues were fixed in this Pull Request

Severity	Issue	Source File / Package
	~~CSRF~~	/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs: 145
	~~CSRF~~	/src/Api/Vault/Controllers/CiphersController.cs: 847
	~~CSRF~~	/src/Api/Vault/Controllers/CiphersController.cs: 847
	~~CSRF~~	/src/Api/Vault/Controllers/CiphersController.cs: 293

…t-database-infrastructure

codecov · 2026-02-02T16:48:12Z

Codecov Report

❌ Patch coverage is 94.01709% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.27%. Comparing base (0159052) to head (fd4672a).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
...e.Dapper/Repositories/JsonCollectionTypeHandler.cs	46.15%	4 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6936      +/-   ##
==========================================
+ Coverage   60.21%   60.27%   +0.05%     
==========================================
  Files        1976     1982       +6     
  Lines       87536    87653     +117     
  Branches     7810     7816       +6     
==========================================
+ Hits        52714    52830     +116     
+ Misses      32907    32904       -3     
- Partials     1915     1919       +4

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…t-database-infrastructure

util/Migrator/DbScripts/2026-01-27_00_AddSubscriptionDiscountTable.sql

…t-database-infrastructure

The previous migrations were missing .Designer.cs files. This commit: - Removes the incomplete migration files - Regenerates all three provider migrations (MySQL, Postgres, SQLite) with proper Designer files - Updates DatabaseContextModelSnapshot.cs for each provider 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

kdenney

Looks good!

mkincaid-bw

Minor changes requested.

mkincaid-bw · 2026-02-04T16:39:40Z

src/Sql/dbo/Views/SubscriptionDiscountView.sql

@@ -0,0 +1,19 @@
+CREATE VIEW [dbo].[SubscriptionDiscountView]
+AS
+SELECT


The preferred format for simple single table views is to select * from the table (unless some for of transformation is needed on one or more of the columns).
https://contributing.bitwarden.com/contributing/code-style/sql#views

mkincaid-bw · 2026-02-04T16:40:11Z

util/Migrator/DbScripts/2026-01-27_00_AddSubscriptionDiscountTable.sql

+CREATE OR ALTER VIEW [dbo].[SubscriptionDiscountView]
+AS
+SELECT
+    [Id],


The preferred format for simple single table views is to select * from the table (unless some for of transformation is needed on one or more of the columns).
https://contributing.bitwarden.com/contributing/code-style/sql#views

mkincaid-bw · 2026-02-04T16:44:58Z

src/Sql/dbo/Tables/SubscriptionDiscount.sql

+GO
+CREATE NONCLUSTERED INDEX [IX_SubscriptionDiscount_DateRange]
+    ON [dbo].[SubscriptionDiscount]([StartDate] ASC, [EndDate] ASC)
+    INCLUDE([StripeProductIds], [AudienceType]);


❓ In the SubscriptionDiscount_ReadActive proc, you are selecting * from the table for the specified date range. SQL Server will still need to reference the underlying table for the rest of the columns, so these extra INCLUDE columns are just taking up extra space. Do you need these INCLUDE columns for another query outside of this proc?

mkincaid-bw · 2026-02-04T16:53:02Z

util/MySqlMigrations/Migrations/DatabaseContextModelSnapshot.cs

+                        .HasMaxLength(100)
+                        .HasColumnType("varchar(100)");
+
+                    b.Property<decimal?>("PercentOff")


This does not align with the SQL server column with a DECIMAL(5,2) data type.

Suggested change

b.Property<decimal?>("PercentOff")

b.Property<decimal?>("PercentOff")

.HasPrecision(5, 2)

…t-database-infrastructure

sonarqubecloud · 2026-02-06T16:01:31Z

Quality Gate passed

Issues
19 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
27.4% Duplication on New Code

See analysis details on SonarQube Cloud

mkincaid-bw

LGTM

cyprain-okeke added 2 commits January 28, 2026 14:20

Implement the detail Subscription Discount Database Infrastructure

6d06077

Change string to string list

751b866

cyprain-okeke requested review from a team as code owners February 2, 2026 16:10

cyprain-okeke requested a review from kdenney February 2, 2026 16:10

Merge branch 'main' into billing/pm-30100/server-subscription-discoun…

5d715d0

…t-database-infrastructure

cyprain-okeke and others added 2 commits February 2, 2026 18:12

fix lint error

a289d99

Merge branch 'main' into billing/pm-30100/server-subscription-discoun…

d98c601

…t-database-infrastructure

mkincaid-bw reviewed Feb 2, 2026

View reviewed changes

util/Migrator/DbScripts/2026-01-27_00_AddSubscriptionDiscountTable.sql Show resolved Hide resolved

cyprain-okeke and others added 7 commits February 3, 2026 09:15

Create all missing database object definition files

33e0bac

Merge branch 'main' into billing/pm-30100/server-subscription-discoun…

f5df9a0

…t-database-infrastructure

fix failing database

a8b5d5d

Resolve the lint warnings

6d4f191

Resolve the database failure

7335ab6

Fix the build Lint

283e079

cyprain-okeke requested a review from mkincaid-bw February 3, 2026 10:08

kdenney previously approved these changes Feb 3, 2026

View reviewed changes

mkincaid-bw requested changes Feb 4, 2026

View reviewed changes

resolve the dbops reviews

29229bb

cyprain-okeke dismissed kdenney’s stale review via 29229bb February 5, 2026 09:03

Merge branch 'main' into billing/pm-30100/server-subscription-discoun…

4513d9f

…t-database-infrastructure

cyprain-okeke requested review from kdenney and mkincaid-bw February 5, 2026 09:05

kdenney previously approved these changes Feb 5, 2026

View reviewed changes

Add the default value

2055931

cyprain-okeke dismissed kdenney’s stale review via 2055931 February 6, 2026 15:53

cyprain-okeke force-pushed the billing/pm-30100/server-subscription-discount-database-infrastructure branch from fba257f to 2055931 Compare February 6, 2026 15:53

Merge branch 'main' into billing/pm-30100/server-subscription-discoun…

fd4672a

…t-database-infrastructure

mkincaid-bw approved these changes Feb 6, 2026

View reviewed changes

cyprain-okeke requested a review from kdenney February 6, 2026 16:26

kdenney approved these changes Feb 6, 2026

View reviewed changes

cyprain-okeke merged commit 67ba9bc into main Feb 6, 2026
52 of 53 checks passed

cyprain-okeke deleted the billing/pm-30100/server-subscription-discount-database-infrastructure branch February 6, 2026 17:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PM 30100][Server] Subscription Discount Database Infrastructure#6936

[PM 30100][Server] Subscription Discount Database Infrastructure#6936
cyprain-okeke merged 16 commits intomainfrom
billing/pm-30100/server-subscription-discount-database-infrastructure

cyprain-okeke commented Feb 2, 2026

Uh oh!

github-actions bot commented Feb 2, 2026 •

edited

Loading

Uh oh!

codecov bot commented Feb 2, 2026 •

edited

Loading

Uh oh!

Uh oh!

kdenney left a comment

Uh oh!

mkincaid-bw left a comment

Uh oh!

mkincaid-bw Feb 4, 2026

Uh oh!

mkincaid-bw Feb 4, 2026

Uh oh!

mkincaid-bw Feb 4, 2026

Uh oh!

mkincaid-bw Feb 4, 2026

Uh oh!

sonarqubecloud bot commented Feb 6, 2026

Uh oh!

mkincaid-bw left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	b.Property<decimal?>("PercentOff")
	b.Property<decimal?>("PercentOff")
	.HasPrecision(5, 2)

Conversation

cyprain-okeke commented Feb 2, 2026

🎟️ Tracking

📔 Objective

📸 Screenshots

⏰ Reminders before review

🦮 Reviewer guidelines

Uh oh!

github-actions bot commented Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

kdenney left a comment

Choose a reason for hiding this comment

Uh oh!

mkincaid-bw left a comment

Choose a reason for hiding this comment

Uh oh!

mkincaid-bw Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

mkincaid-bw Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

mkincaid-bw Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

mkincaid-bw Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Feb 6, 2026

Quality Gate passed

Uh oh!

mkincaid-bw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Feb 2, 2026 •

edited

Loading

codecov bot commented Feb 2, 2026 •

edited

Loading