Conversation
…or the rebuild review, and stay quiet after Cancel
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follows #3147 and updates the
StructureTypelessColumnUITestsit added, which clicked both dialogs.Root cause
Two components decided on their own whether to confirm a Structure save, and neither knew the other had already asked.
StructureEditingSession+Applyshowed a "Destructive Changes" alert for any change withrequiresDataMigration: drop column, type change, nullable to NOT NULL, primary key change, adding a CHECK, rewriting a CHECK. ThenexecuteSchemaChangessent the statements to the execution gate, which confirms again whenever the text is destructive (anyALTERcontainingDROP,DROP INDEX) or the Safe Mode level confirms writes (Alert and above). A dropped column was asked twice at every level, Silent included. Every data-loss change was asked twice at Alert and above, and at Safe Mode that was alert, sheet, then Touch ID.SchemaStatement.isDestructivewas set by the generator and ignored byexecuteSchemaChanges, so the gate could not see thatALTER COLUMN .. TYPEorMODIFY COLUMN .. NOT NULLcan lose or refuse rows.StructureRebuildPlanRunnerwith a plain.interactiveUserrequest, so the gate stacked a second critical sheet over the review at every level. The comment inapplyStagedChangessaid the review was meant to be the one confirmation.DatabaseError.queryFailed("Operation cancelled by user"), shown as an "Error Applying Changes" sheet and reported as a failed operation.Fix
SchemaStatementGeneratormarks every statement of arequiresDataMigrationchange as destructive.executeSchemaChangesbuilds its request throughDatabaseManager.schemaChangeAuthorizationRequest, andschemaOperationKind(for:combinedSQL:databaseType:)returns.destructiveQueryfor a flagged statement. The signature matches feat/mongodb-structure-field-edits, so that branch can drop its copy, andSchemaOperationKindTests.swiftis byte-identical to that branch's.isConfirmationPreCleared: truethroughStructureRebuildPlanRunner.authorizationRequest. The gate skips its confirmation but still asks for Touch ID, still refuses at Read-Only and still writes the audit record. A metadata-only reorder, which has no review, passes false. A runnable review is titled with the operation, carries the "Runs on '…'" line, shows the script uncut and warns "This may permanently modify or delete data and cannot be undone." ahead of the plan's caveats, like the gate's own sheet for a destructive statement. A preview has no such warning, because it runs nothing.OperationDecision.deniedcarries anOperationDenialCause(.policyby default,.cancelledByUserfrom the confirmation Cancel), andExecutionGateErrorgained.cancelledByUser. The compiler flagged every existing binding site.StructureApplyFailuremaps an error to what the save does: a Cancel returns.refusedwith the edits staged and shows nothing, a refusal shows its reason without a failed-operation report, and a server error behaves as before. The column reorder paths use the same mapping.StructureEditingSession,executeSchemaChangesand the rebuild runner, so unit tests run the realDefaultExecutionGatewith prompts that count and never reach a windowless alert.What each Safe Mode level asks now
Before this change a dropped column asked twice at every level, a rebuild asked twice at every level, and Cancel was answered with an error sheet.
Verified
StructureSaveConfirmationTests.alterSavesAskOnceruns 8 saves at all 6 levels through the real gate: the confirmation count, whether the sheet is marked destructive, the Touch ID count, and the Read-Only refusal with no prompt. It also covers the rebuild request with and without pre-clear, the review accessors and the failure mapping.StructureEditingSessionTests.cancelAtTheGateKeepsTheEdits: a Cancel returns.refused, the edits stay staged, nothing runs and nothing is shown.ExecutionGateTestschecks the denial cause for a Cancel, Read-Only and a declined Touch ID.runnableRebuildReviewWarnsOfDataLosscovers the warning with and without caveats, andpreviewShowsTheRebuildReadOnlynow expects the review, but not the preview, to carry it.mainatc21dc512e: 6 unit suites, 114/114. UI: StructureSaveConfirmationUITests, StructureTypelessColumnUITests, StructureForeignKeyEditUITests, StructureColumnMoveUITests and StructurePreviewSQLUITests, 18/18.verify.sh docs: pass.Deliberately not fixed here
AlertOperationConfirmingpasses the joined SQL as one statement for every caller. This predates the change and will be fixed separately.OperationAuthenticatingreturns a Bool that cannot tell a Cancel from a Mac with no passcode.Touch ID levels cannot run in XCUITest, so the stub-prompt matrix covers them.