Skip to content

[PM-36965] feat: Add CSV export of cohort to cohort management table#7847

Draft
cyprain-okeke wants to merge 5 commits into
mainfrom
billing/pm-36965/determining-eligibility-add.csv-export-of-cohort-to-cohort-management-table
Draft

[PM-36965] feat: Add CSV export of cohort to cohort management table#7847
cyprain-okeke wants to merge 5 commits into
mainfrom
billing/pm-36965/determining-eligibility-add.csv-export-of-cohort-to-cohort-management-table

Conversation

@cyprain-okeke

@cyprain-okeke cyprain-okeke commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

🎟️ Tracking

PM-36965 — [Determining Eligibility] Add .csv export of cohort to Cohort management table

📔 Objective

Extends the Admin Portal cohort management UI (PM-36951) with an Export CSV action that downloads every organization currently assigned to a migration cohort. This is an internal operator tool for auditing cohort contents and cross-referencing migration lists against external data.

Endpoint: GET /migration-cohorts/{id}/exporttext/csv with Content-Disposition: attachment, gated by the existing Tools_ManagePlanMigrationCohorts permission and the PM35215_BusinessPlanPriceMigration feature flag (same gates as the rest of the cohort UI).

Columns: OrganizationId, OrganizationName, AssignedAt, ScheduledDate, MigratedDate.

How it works:

  • The response is streamed directly to the body (not buffered), so large cohorts (thousands of orgs) export without loading every row into memory.
  • A new repository method GetExportRowsByCohortIdAsync returns a bounded keyset page in both ORM tracks (Dapper stored proc + EF Core), ordered by (CreationDate, Id) so the new (CohortId, CreationDate, Id) index serves both the seek and the sort with no residual Sort operator — important because bulk-loaded cohorts share one CreationDate across the whole batch.
  • A query object (IExportCohortAssignmentsQuery) yields rows as IAsyncEnumerable; the controller owns CSV formatting and HTTP streaming.

Safety / operability:

  • Filename is slugged from the cohort name (guards Content-Disposition header injection); OrganizationName is defanged against CSV formula injection.
  • A mid-stream failure logs with context and aborts the connection, so the operator gets a visibly broken download rather than a silently truncated CSV.
  • Reads use the read-only connection string; an info-level audit log records actor + cohort id only (never org names or CSV contents).

Testing:

  • Core query unit tests (paging/cursor advancement) and Admin controller tests (gates, headers, formula-injection, filename slug, audit log) — 44 unit tests.
  • Repository integration tests run across all four databases (SQL Server, MySQL, PostgreSQL, SQLite): join surfaces the org name, keyset paging has no gaps/dupes including the all-shared-CreationDate bulk case.

Notes for reviewers:

  • Open product question (non-blocking): final column set and whether export should be reachable from the table row vs. detail page — currently both; defaults implemented per the ticket, to confirm with the operator team.
  • Row order in the CSV is not guaranteed identical across DB providers (each uses its native GUID order); the export contract is "every row, once," which is correct for a download.

📸 Screenshots

Internal Admin Portal action (Export CSV link on the cohort list and detail pages); no new design surface — omitted.

Adds an "Export CSV" action to the Admin Portal cohort management UI that
streams every organization assigned to a migration cohort as a CSV. Gated by
the existing Tools_ManagePlanMigrationCohorts permission and the
PM35215_BusinessPlanPriceMigration feature flag.

The response is streamed directly to the body rather than buffered, so large
cohorts (thousands of orgs) export without loading every row into memory. The
repository exposes a bounded keyset page (GetExportRowsByCohortIdAsync) in both
the Dapper and EF Core tracks, ordered by (CreationDate, Id) so the new
(CohortId, CreationDate, Id) index serves both the seek and the sort.
@cyprain-okeke cyprain-okeke added the ai-review Request a Claude code review label Jun 22, 2026
….csv-export-of-cohort-to-cohort-management-table
@github-actions

github-actions Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the new Admin Portal CSV export of cohort assignments: the streaming controller endpoint, the IExportCohortAssignmentsQuery keyset-paging query, the dual-ORM repository methods (Dapper stored proc + EF Core), the new (CohortId, CreationDate, Id) index across SSDT/EF, and the three EF migrations. Security-sensitive surfaces (CSV formula injection, Content-Disposition filename injection, audit logging, read-only connection use) were examined closely, along with dual-ORM parity and migration idempotency.

No new findings meet the confidence threshold for posting. The change is well-tested (core query paging, controller gates/headers/sanitization, and four-database integration tests) and the security and operability concerns are handled correctly.

Code Review Details

No findings.

Notes on areas examined and cleared:

  • CSV formula injection: SanitizeCsvField correctly defangs the only attacker-influenced field (OrganizationName); GUID and ISO-8601 date fields cannot begin with a formula trigger.
  • Content-Disposition header injection: BuildExportFileName slugs the cohort name to [a-zA-Z0-9-], eliminating header injection.
  • Keyset paging: cursor advancement in ExportCohortAssignmentsQuery is correct (both-null-or-both-set contract enforced by the sole caller) and matches the seek/order in both ORM tracks.
  • Dual-ORM parity: Dapper stored proc, EF LINQ, SSDT table index, EF configuration, and all three EF provider migrations are consistent; the dated migration is idempotent (guarded index, CREATE OR ALTER proc).
  • CsvHelper is a pre-existing dependency (src/Core/Core.csproj); no manifest change in this PR.

The automated-bot threads on stream disposal, broad catch, and the nullable afterId! were reviewed; the author's responses are sound and those points are not re-raised.

Comment thread src/Admin/Billing/Controllers/OrganizationPlanMigrationCohortsController.cs Dismissed
Comment thread src/Admin/Billing/Controllers/OrganizationPlanMigrationCohortsController.cs Dismissed
@codecov

codecov Bot commented Jun 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.87097% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.72%. Comparing base (dbf6308) to head (fb61051).

Files with missing lines Patch % Lines
...lers/OrganizationPlanMigrationCohortsController.cs 70.23% 19 Missing and 6 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7847      +/-   ##
==========================================
+ Coverage   65.69%   65.72%   +0.03%     
==========================================
  Files        2209     2211       +2     
  Lines       97750    97904     +154     
  Branches     8816     8834      +18     
==========================================
+ Hits        64216    64347     +131     
- Misses      31316    31333      +17     
- Partials     2218     2224       +6     

☔ 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.

Comment thread src/Admin/Billing/Controllers/OrganizationPlanMigrationCohortsController.cs Dismissed
….csv-export-of-cohort-to-cohort-management-table
@sonarqubecloud

Copy link
Copy Markdown

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant