fix: mark completed till validation for migration reset#916
Conversation
9e2d164 to
8423194
Compare
There was a problem hiding this comment.
Pull request overview
Fixes admin migration reset validation so markCompletedTillMigration is validated against DBMigration (instead of accidentally re-validating untilMigration) and adds regression tests to prevent the issue from recurring.
Changes:
- Corrected validation logic for
markCompletedTillMigrationin fleet and per-tenant reset routes. - Tightened typing/casting for the forwarded
markCompletedTillMigrationvalue in the fleet reset route. - Added regression tests covering invalid
markCompletedTillMigrationfor both reset endpoints.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/http/routes/admin/tenants.ts | Fixes markCompletedTillMigration validation in tenant migration reset route. |
| src/http/routes/admin/migrations.ts | Fixes markCompletedTillMigration validation and casting in fleet migration reset route. |
| src/test/admin-migrations.test.ts | Adds regression tests ensuring invalid markCompletedTillMigration is rejected. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
8423194 to
f1bd17b
Compare
There was a problem hiding this comment.
Pull request overview
Fixes validation for markCompletedTillMigration in admin migration reset routes and adds regression tests to ensure invalid values are rejected.
Changes:
- Validate
markCompletedTillMigrationagainstDBMigrationin both fleet and per-tenant reset endpoints. - Tighten handling of
markCompletedTillMigrationto only pass it through when it’s a string. - Add Jest regression tests covering invalid
markCompletedTillMigrationinputs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/http/routes/admin/tenants.ts | Fixes the migration key used for markCompletedTillMigration validation and refines how it’s forwarded to resetMigration. |
| src/http/routes/admin/migrations.ts | Fixes the migration key used for markCompletedTillMigration validation and refines how it’s forwarded to resetMigrationsOnTenants. |
| src/test/admin-migrations.test.ts | Adds regression tests ensuring invalid markCompletedTillMigration is rejected for both fleet and tenant reset endpoints. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
f1bd17b to
e40bdc7
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes request-body validation for admin migration reset endpoints so markCompletedTillMigration and untilMigration are validated against known migration keys, and adds regression tests to cover the invalid-markCompletedTillMigration cases.
Changes:
- Update fleet reset route validation to properly validate
markCompletedTillMigration(and avoid the0/truthiness pitfall foruntilMigration). - Update per-tenant reset route validation similarly and ensure only string
markCompletedTillMigrationvalues are forwarded. - Add Jest regression tests asserting invalid
markCompletedTillMigrationis rejected for both fleet and tenant reset endpoints.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/http/routes/admin/migrations.ts |
Fixes validation logic for /migrations/reset/fleet request inputs. |
src/http/routes/admin/tenants.ts |
Fixes validation logic for /:tenantId/migrations/reset request inputs. |
src/test/admin-migrations.test.ts |
Adds regression tests for invalid markCompletedTillMigration handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
e40bdc7 to
0e1c0d3
Compare
Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>
0e1c0d3 to
403039b
Compare
There was a problem hiding this comment.
Pull request overview
Fixes admin migration-reset input validation so untilMigration and markCompletedTillMigration are validated correctly (including the create-migrations-table case that maps to numeric id 0), and adds regression tests to prevent reintroducing the bug.
Changes:
- Add
isDBMigrationName()guard and use it in admin migration reset routes to validate migration keys by existence (not truthiness). - Mark
DBMigrationasas const(generated + checked-in) to keep it read-only and improve type safety. - Add Jest regression tests covering invalid
markCompletedTillMigrationand the0-mapped migration key.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/admin-migrations.test.ts | Adds regression tests for admin fleet/tenant reset migration validation and the 0-id migration case. |
| src/scripts/migrations-types.ts | Updates the generator output to emit DBMigration as as const. |
| src/internal/database/migrations/types.ts | Marks DBMigration as as const to make it read-only and preserve literal typing. |
| src/internal/database/migrations/index.ts | Re-exports the new guard utilities. |
| src/internal/database/migrations/guards.ts | Introduces isDBMigrationName() for safe migration-key validation. |
| src/http/routes/admin/tenants.ts | Fixes tenant reset validation to correctly validate markCompletedTillMigration. |
| src/http/routes/admin/migrations.ts | Fixes fleet reset validation to correctly validate markCompletedTillMigration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
What kind of change does this PR introduce?
Bug fix
What is the current behavior?
For admin migration reset, one input is validated twice and one isn't validated at all.
What is the new behavior?
Inputs are validated correctly and covered by regression tests.
Additional context
Mark migration types
as constto mark read only.Related to allow-failure directive feature.