[PM-28531] Remove old proc and use new one by voommen-livefront · Pull Request #7110 · bitwarden/server

voommen-livefront · 2026-02-27T19:05:46Z

🎟️ Tracking

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

📔 Objective

Removed procedure OrganizationReport_GetSummariesByDateRange as it didn't confirm to naming convention
Created new proc OrganizationReport_ReadByOrganizationIdAndRevisionDate
Update repo to use this new proc
Updated model to include encrpytion key, org id and date.

📸 Screenshots

github-actions · 2026-02-27T19:17:07Z

Checkmarx One – Scan Summary & Details – d554ea86-f03c-4a13-b02f-a3aae8aac240

Great job! No new security vulnerabilities introduced in this pull request

codecov · 2026-02-27T19:19:44Z

Codecov Report

❌ Patch coverage is 78.26087% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.45%. Comparing base (1e7f7ac) to head (2a4f451).
⚠️ Report is 9 commits behind head on main.

Files with missing lines	Patch %	Lines
.../Dirt/Repositories/OrganizationReportRepository.cs	0.00%	9 Missing ⚠️
...ucture.Dapper/Dirt/OrganizationReportRepository.cs	0.00%	3 Missing ⚠️
...etOrganizationReportSummaryDataByDateRangeQuery.cs	94.28%	1 Missing and 1 partial ⚠️
.../Dirt/Controllers/OrganizationReportsController.cs	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7110      +/-   ##
==========================================
- Coverage   56.70%   56.45%   -0.25%     
==========================================
  Files        2013     2014       +1     
  Lines       88191    88151      -40     
  Branches     7861     7863       +2     
==========================================
- Hits        50009    49770     -239     
- Misses      36362    36565     +203     
+ Partials     1820     1816       -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.

…issues

mkincaid-bw · 2026-03-02T18:48:33Z

src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_ReadyByOrganizationIdAndRevisionDate.sql

+SELECT
+    [OrganizationId], 
+    [ContentEncryptionKey], 
+    [SummaryData], 
+    [RevisionDate]
+FROM [dbo].[OrganizationReportView]
+WHERE [OrganizationId] = @OrganizationId
+  AND [RevisionDate] >= @StartDate
+  AND [RevisionDate] <= @EndDate
+ORDER BY [RevisionDate] DESC


The entire SELECT statement should be indented 4 spaces, and SQL keywords should be on their own line
https://contributing.bitwarden.com/contributing/code-style/sql#select-statements

Suggested change

SELECT

[OrganizationId],

[ContentEncryptionKey],

[SummaryData],

[RevisionDate]

FROM [dbo].[OrganizationReportView]

WHERE [OrganizationId] = @OrganizationId

AND [RevisionDate] >= @StartDate

AND [RevisionDate] <= @EndDate

ORDER BY [RevisionDate] DESC

SELECT

[OrganizationId],

[ContentEncryptionKey],

[SummaryData],

[RevisionDate]

FROM

[dbo].[OrganizationReportView]

WHERE

[OrganizationId] = @OrganizationId

AND [RevisionDate] >= @StartDate

AND [RevisionDate] <= @EndDate

ORDER BY

[RevisionDate] DESC

I didn't commit this suggestion because I renamed the file, but I have updated the formatting in the new file,

mkincaid-bw · 2026-03-02T18:53:02Z

...grator/DbScripts/2026-02-27_00_AddOrganizationReport_ReadByOrganizationIdAndRevisionDate.sql

@@ -0,0 +1,23 @@
+DROP PROC IF EXISTS [dbo].[OrganizationReport_GetSummariesByDateRange]


This does not follow our EDD pattern.
https://contributing.bitwarden.com/contributing/database-migrations/edd/

Database changes must be deployable independently of code changes, and must support both the current and next release. Dropping this proc should happen later in another PR once the server code has been deployed and hardened a bit.

@mkincaid-bw The procedure and the endpoint weren't used this far.
It is safe to drop the proc in this PR as it is being replaced by a new one.

Also, my apologies for this PR being so large - I should have made two PR's one with the DB changes and another with the code changes. I came in this morning thinking of what else I got to do next and didn't think about making another PR.

mkincaid-bw · 2026-03-02T18:53:19Z

...grator/DbScripts/2026-02-27_00_AddOrganizationReport_ReadByOrganizationIdAndRevisionDate.sql

+BEGIN
+    SET NOCOUNT ON
+
+SELECT


Same comment as above re: formatting.

mkincaid-bw · 2026-03-02T18:55:26Z

src/Sql/dbo/Dirt/Stored Procedures/OrganizationReport_ReadyByOrganizationIdAndRevisionDate.sql

The file name is OrganizationReport_ReadyByOrganizationIdAndRevisionDate.sql (_ReadyBy...) but should be OrganizationReport_ReadByOrganizationIdAndRevisionDate.sql (_ReadBy...) to match the stored proc name.

I am so sorry about this - I should have fixed this before this was made available to you.

mkincaid-bw · 2026-03-02T22:06:04Z

src/Core/Dirt/Models/Data/OrganizationReportSummaryDataResponse.cs

-    public string? SummaryData { get; set; }
+    public required Guid OrganizationId { get; set; }
+    [JsonPropertyName("encryptedData")]
+    public required string SummaryData { get; set; }


The SummaryData and RevisionDate (below) properties are required here, but both columns in the DB allow nulls. Not sure if it's an issue or not but I just wanted to point it out.

@mkincaid-bw you are right - the procedures are required - if there is no summary data, than this object is not relevant. And I tested it in the database

sonarqubecloud · 2026-03-02T22:46:15Z

Quality Gate passed

Issues
4 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

PM-28531 remove old proc and use new one

18a2f5d

voommen-livefront requested review from a team as code owners February 27, 2026 19:05

voommen-livefront added 7 commits February 27, 2026 13:27

PM-28531 updated json property name

0505509

PM-28531 used Pascal case for named placeholders and other sonarCube …

dce39bf

…issues

PM-28531 introduce caching and update methods

8a7e064

PM-28531 added a limit of 6 records per query

87b48cf

PM-28531 added docs to the endpoint

fa587eb

PM-28531 updated test and code to fix error

39922e0

PM-28531 fix sonar qube recommendations

d467a71

prograhamming previously approved these changes Mar 2, 2026

View reviewed changes

PM-28531 updated unit tests

495cbf9

voommen-livefront dismissed prograhamming’s stale review via 495cbf9 March 2, 2026 21:08

voommen-livefront requested a review from prograhamming March 2, 2026 21:55

prograhamming previously approved these changes Mar 2, 2026

View reviewed changes

mkincaid-bw requested changes Mar 2, 2026

View reviewed changes

PM-28531 update the file name - as per PR comments and formatting issues

8e8495d

voommen-livefront dismissed prograhamming’s stale review via 8e8495d March 2, 2026 22:26

voommen-livefront requested a review from mkincaid-bw March 2, 2026 22:34

PM-28531 updating formatting

2a4f451

		@@ -0,0 +1,23 @@
		DROP PROC IF EXISTS [dbo].[OrganizationReport_GetSummariesByDateRange]

Conversation

voommen-livefront commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎟️ Tracking

📔 Objective

📸 Screenshots

Uh oh!

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

Uh oh!

Great job! No new security vulnerabilities introduced in this pull request

Uh oh!

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

Uh oh!

Codecov Report

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!

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!

sonarqubecloud bot commented Mar 2, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

voommen-livefront commented Feb 27, 2026 •

edited

Loading

github-actions bot commented Feb 27, 2026 •

edited

Loading

codecov bot commented Feb 27, 2026 •

edited

Loading