Skip to content

[PM-39236]: Add RenewalNotificationSentDate column + repository method#8031

Open
sbrown-livefront wants to merge 22 commits into
mainfrom
billing/pm-39236/add-renewalnotificationsentdate-column
Open

[PM-39236]: Add RenewalNotificationSentDate column + repository method#8031
sbrown-livefront wants to merge 22 commits into
mainfrom
billing/pm-39236/add-renewalnotificationsentdate-column

Conversation

@sbrown-livefront

@sbrown-livefront sbrown-livefront commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

🎟️ Tracking

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

📔 Objective

Adds the nullable RenewalNotificationSentDate column to OrganizationPlanMigrationCohortAssignment and a repository method to select the organizations that are candidates for a renewal-invoice notification.
Schema (both ORM tracks)

  • New nullable RenewalNotificationSentDate DATETIME2(7) column on OrganizationPlanMigrationCohortAssignment — added to the entity, the SSDT table, the Dapper _Create/_Update stored procedures, and generated for EF Core (PostgreSQL, MySQL, SQLite).
  • MSSQL migration backfills RenewalNotificationSentDate = ScheduledDate where ScheduledDate IS NOT NULL, so organizations already processed via the charge_automatically path read as notified rather than incomplete. The EF migrations perform the equivalent backfill for the other three providers.
  • The MSSQL migration refreshes OrganizationPlanMigrationCohortAssignmentView (sp_refreshview) after the ALTER TABLE, since that SELECT * view (and the read procs built on it) caches its column list and would otherwise not surface the new column.

Repository method

  • GetSendInvoiceCandidatesInWindowAsync(int minDays, int maxDays) on IOrganizationPlanMigrationCohortAssignmentRepository, implemented in both Dapper (new OrganizationPlanMigrationCohortAssignment_ReadManyByExpirationDateRange stored procedure) and EF Core.
  • Returns assignments where the cohort is active, MigratedDate IS NULL, the assignment is not-yet-complete (ScheduledDate IS NULL OR RenewalNotificationSentDate IS NULL), the organization has non-null GatewayCustomerId/GatewaySubscriptionId, and ExpirationDate falls within [now + minDays, now + maxDays].

Tests

  • Adds [DatabaseData] integration tests (run against all four providers) covering the happy path plus each exclusion clause: inactive cohort, migrated assignment, scheduled-but-not-notified (included) vs. scheduled-and-notified (excluded), missing gateway identifiers, and expiration before/after the window and null.

@sbrown-livefront sbrown-livefront self-assigned this Jul 20, 2026
@sbrown-livefront sbrown-livefront added the ai-review Request a Claude code review label Jul 20, 2026
@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the addition of the nullable RenewalNotificationSentDate column and the GetSendInvoiceCandidatesInWindowAsync repository method across all four database tracks (MSSQL/Dapper, PostgreSQL, MySQL, SQLite). Verified dual-ORM parity between the Dapper stored procedure and the EF Core LINQ query, the backfill logic in each migration, and the SSDT view/sp_refreshview handling. The [DatabaseData] integration tests cover the happy path plus every exclusion clause, and the minDays > maxDays guard is enforced in both repository implementations.

Code Review Details

No new findings. The change is consistent across all four providers:

  • MSSQL migration adds the column, refreshes the SELECT * view via sp_refreshview, and backfills RenewalNotificationSentDate = ScheduledDate; the read procs that SELECT * from the view re-resolve their column list against the refreshed view at execution.
  • Dapper and EF query semantics match (IsActive, MigratedDate IS NULL, ScheduledDate IS NULL OR RenewalNotificationSentDate IS NULL, gateway identifiers, and the [now + minDays, now + maxDays] expiration window).
  • The EF read query correctly uses AsNoTracking().
  • Integration tests exercise all providers and each eligibility branch.

All issues raised in earlier review threads (view refresh, minDays <= maxDays validation, single GETUTCDATE() capture, AsNoTracking()) are resolved.

@sbrown-livefront sbrown-livefront added the t:feature Change Type - Feature Development label Jul 20, 2026
@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.85%. Comparing base (2bdd35a) to head (2f95007).

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #8031       +/-   ##
===========================================
+ Coverage   14.96%   66.85%   +51.89%     
===========================================
  Files        1389     2293      +904     
  Lines       60360    99966    +39606     
  Branches     4793     9019     +4226     
===========================================
+ Hits         9030    66831    +57801     
+ Misses      51175    30866    -20309     
- Partials      155     2269     +2114     

☔ View full report in Codecov by Harness.
📢 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.

@sbrown-livefront
sbrown-livefront marked this pull request as ready for review July 20, 2026 21:43
@sbrown-livefront
sbrown-livefront requested review from a team as code owners July 20, 2026 21:43
@sbrown-livefront
sbrown-livefront requested a review from kdenney July 20, 2026 21:43
kdenney
kdenney previously approved these changes Jul 21, 2026

@kdenney kdenney left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for answering my question; looks good to me. Nice job!

Copilot AI review requested due to automatic review settings July 21, 2026 20:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a nullable RenewalNotificationSentDate field to OrganizationPlanMigrationCohortAssignment across schema/migrations, and introduces a repository query to fetch cohort assignments eligible for renewal-invoice notifications within a configurable expiration window.

Changes:

  • Added RenewalNotificationSentDate column plus backfill-from-ScheduledDate in migrations (MSSQL script + EF migrations for SQLite/Postgres/MySQL).
  • Added GetSendInvoiceCandidatesInWindowAsync(minDays, maxDays) to repository interface and implemented it for both Dapper (new stored proc) and EF Core.
  • Added multi-provider integration tests covering inclusion/exclusion criteria for the new candidate-selection query.

Reviewed changes

Copilot reviewed 16 out of 19 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
util/SqliteMigrations/Migrations/DatabaseContextModelSnapshot.cs Adds RenewalNotificationSentDate to SQLite EF snapshot.
util/SqliteMigrations/Migrations/20260717173646_AddRenewalNotificationSentDateToOrganizationPlanMigrationCohortAssignment.Designer.cs SQLite migration designer reflecting the new column.
util/SqliteMigrations/Migrations/20260717173646_AddRenewalNotificationSentDateToOrganizationPlanMigrationCohortAssignment.cs SQLite EF migration adding column + backfill SQL.
util/PostgresMigrations/Migrations/DatabaseContextModelSnapshot.cs Adds RenewalNotificationSentDate to Postgres EF snapshot.
util/PostgresMigrations/Migrations/20260717173652_AddRenewalNotificationSentDateToOrganizationPlanMigrationCohortAssignment.Designer.cs Postgres migration designer reflecting the new column.
util/PostgresMigrations/Migrations/20260717173652_AddRenewalNotificationSentDateToOrganizationPlanMigrationCohortAssignment.cs Postgres EF migration adding column + backfill SQL.
util/MySqlMigrations/Migrations/DatabaseContextModelSnapshot.cs Adds RenewalNotificationSentDate to MySQL EF snapshot.
util/MySqlMigrations/Migrations/20260717173659_AddRenewalNotificationSentDateToOrganizationPlanMigrationCohortAssignment.cs MySQL EF migration adding column + backfill SQL.
util/Migrator/DbScripts/2026-07-17_00_AddRenewalNotificationSentDateToOrganizationPlanMigrationCohortAssignment.sql MSSQL migration script: adds column, refreshes SELECT * view, backfills, updates create/update procs, adds read-many proc.
src/Sql/dbo/Billing/Tables/OrganizationPlanMigrationCohortAssignment.sql SSDT table definition updated to include the new nullable column.
src/Sql/dbo/Billing/Stored Procedures/OrganizationPlanMigrationCohortAssignment_Create.sql SSDT create proc updated to accept/persist RenewalNotificationSentDate.
src/Sql/dbo/Billing/Stored Procedures/OrganizationPlanMigrationCohortAssignment_Update.sql SSDT update proc updated to accept/persist RenewalNotificationSentDate.
src/Sql/dbo/Billing/Stored Procedures/OrganizationPlanMigrationCohortAssignment_ReadManyByExpirationDateRange.sql New stored proc to select invoice-notification candidates by expiration window.
src/Core/Billing/Organizations/PlanMigration/Entities/OrganizationPlanMigrationCohortAssignment.cs Core entity updated with RenewalNotificationSentDate.
src/Core/Billing/Organizations/PlanMigration/Repositories/IOrganizationPlanMigrationCohortAssignmentRepository.cs Interface extended with GetSendInvoiceCandidatesInWindowAsync.
src/Infrastructure.Dapper/Billing/Repositories/OrganizationPlanMigrationCohortAssignmentRepository.cs Dapper implementation added for the new candidate query.
src/Infrastructure.EntityFramework/Billing/Repositories/OrganizationPlanMigrationCohortAssignmentRepository.cs EF implementation added for the new candidate query (needs fix per comments).
test/Infrastructure.IntegrationTest/Billing/Repositories/OrganizationPlanMigrationCohortAssignmentRepositoryTests.cs New integration tests for the candidate-selection query and edge cases.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI review requested due to automatic review settings July 21, 2026 20:53
@sbrown-livefront
sbrown-livefront removed the request for review from Copilot July 21, 2026 20:53
Copilot AI review requested due to automatic review settings July 21, 2026 21:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 19 changed files in this pull request and generated 2 comments.

Copilot AI review requested due to automatic review settings July 21, 2026 21:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 19 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings July 21, 2026 22:02
@sbrown-livefront
sbrown-livefront requested review from kdenney and removed request for Copilot July 21, 2026 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review t:feature Change Type - Feature Development

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants