Skip to content

Add CONFIRM dialog guard for Run Migration button in Dev Tools#420

Open
CasperL1218 wants to merge 1 commit intophase1-all-mergesfrom
dev-tools-safety
Open

Add CONFIRM dialog guard for Run Migration button in Dev Tools#420
CasperL1218 wants to merge 1 commit intophase1-all-mergesfrom
dev-tools-safety

Conversation

@CasperL1218
Copy link
Copy Markdown
Contributor

Summary

  • Replaced window.confirm() on the "Run Migration" button with a MUI Dialog that requires the admin to type
    CONFIRM before proceeding
  • Prevents accidental batch-writes to all apartment records in the database
  • No new dependencies or imports needed

Test Plan

  1. Go to Admin → Dev Tools → Schema Migration
  2. Click "Run Migration" — confirm dialog should open
  3. Verify the "Run Migration" button inside the dialog is disabled until CONFIRM is typed exactly
  4. Verify Cancel closes the dialog without running anything
  5. Type CONFIRM → button enables → migration runs as before

Notes

This PR should be merged after PR #418 (phase1-all-merges), which introduced the Dev Tools tab. Base branch is set
to phase1-all-merges for that reason — retarget to main once #418 merges.

Breaking Changes

  Replaces window.confirm() with a MUI Dialog that requires the admin to
  type 'CONFIRM' before the schema migration can proceed, preventing
  accidental batch-writes to all apartment records.
@dti-github-bot
Copy link
Copy Markdown
Member

[diff-counting] Significant lines: 49.

Copy link
Copy Markdown

@bryanshao07 bryanshao07 left a comment

Choose a reason for hiding this comment

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

Looks good! Adding the "CONFIRM" button feature is a good safeguarding feature. One small thing might be making the confirmation check case-insensitive with .toUpperCase() for usability, or making it super clear that it must be in all caps. Good overall.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants