sql:migrate --force should ignore invalid connector (#7199)

firebase · May 21, 2024 · 04aacc2 · 04aacc2
1 parent dda60a1
commit 04aacc2
Showing 1 changed file with 34 additions and 20 deletions.
diff --git a/src/dataconnect/schemaMigration.ts b/src/dataconnect/schemaMigration.ts
@@ -79,15 +79,11 @@ export async function migrateSchema(args: {
 
     const shouldDeleteInvalidConnectors = await promptForInvalidConnectorError(
       options,
+      serviceName,
       invalidConnectors,
       validateOnly,
     );
-    if (!shouldDeleteInvalidConnectors && invalidConnectors.length) {
-      const cmd = suggestedCommand(serviceName, invalidConnectors);
-      throw new FirebaseError(
-        `Command aborted. Try deploying those connectors first with ${clc.bold(cmd)}`,
-      );
-    }
+
     const migrationMode = incompatible
       ? await promptForSchemaMigration(
           options,
@@ -112,11 +108,13 @@ export async function migrateSchema(args: {
       });
     }
 
-    if (invalidConnectors.length) {
+    if (shouldDeleteInvalidConnectors) {
       await deleteInvalidConnectors(invalidConnectors);
     }
-    // Then, try to upsert schema again. If there still is an error, just throw it now
-    await upsertSchema(schema, validateOnly);
+    if (!validateOnly) {
+      // Then, try to upsert schema again. If there still is an error, just throw it now
+      await upsertSchema(schema, validateOnly);
+    }
     return diffs;
   }
   return [];
@@ -212,7 +210,7 @@ async function promptForSchemaMigration(
 ): Promise<"none" | "safe" | "all"> {
   displaySchemaChanges(err);
   if (!options.nonInteractive) {
-    // Always prompt in interactive mode. Desturctive migrations are too potentially dangerous to not prompt for with --force
+    // Always prompt in interactive mode. Destructive migrations are too potentially dangerous to not prompt for with --force
     const choices = err.destructive
       ? [
           { name: "Execute all changes (including destructive changes)", value: "all" },
@@ -250,6 +248,7 @@ async function promptForSchemaMigration(
 
 async function promptForInvalidConnectorError(
   options: Options,
+  serviceName: string,
   invalidConnectors: string[],
   validateOnly: boolean,
 ): Promise<boolean> {
@@ -258,18 +257,33 @@ async function promptForInvalidConnectorError(
   }
   displayInvalidConnectors(invalidConnectors);
   if (validateOnly) {
-    return false;
-  } else if (
-    options.force ||
-    (!options.nonInteractive &&
-      (await confirm({
-        ...options,
-        message: "Would you like to delete and recreate these connectors?",
-      })))
+    if (options.force) {
+      // `firebase dataconnect:sql:migrate --force` ignores invalid connectors.
+      return false;
+    }
+    // `firebase dataconnect:sql:migrate` aborts if there are invalid connectors.
+    throw new FirebaseError(
+      `Command aborted. If you'd like to migrate it anyway, you may override with --force.`,
+    );
+  }
+  if (options.force) {
+    // `firebase deploy --force` will delete invalid connectors without prompting.
+    return true;
+  }
+  // `firebase deploy` prompts in case of invalid connectors.
+  if (
+    !options.nonInteractive &&
+    (await confirm({
+      ...options,
+      message: `Would you like to delete and recreate these connectors? This will cause ${clc.red(`downtime.`)}.`,
+    }))
   ) {
     return true;
   }
-  return false;
+  const cmd = suggestedCommand(serviceName, invalidConnectors);
+  throw new FirebaseError(
+    `Command aborted. Try deploying those connectors first with ${clc.bold(cmd)}`,
+  );
 }
 
 async function deleteInvalidConnectors(invalidConnectors: string[]): Promise<void[]> {
@@ -284,7 +298,7 @@ function displayInvalidConnectors(invalidConnectors: string[]) {
   );
   logLabeledWarning(
     "dataconnect",
-    `This is a ${clc.red("breaking")} change and will cause a brief downtime.`,
+    `This is a ${clc.red("breaking")} change and may break existing apps.`,
   );
 }