fix(migrate): offer a keep-the-data fix only for users the import sends

.changeset/migrate-name-fixes-importable.md

@@ -0,0 +1,5 @@
+---
+"clerk": patch
+---
+
+`clerk migrate import` no longer suggests turning on names or passwords when only users the checks reject carry them. Those users aren't imported, so the setting would keep nothing. Fixes that clear a reject, such as making a required name optional, are still offered. `clerk migrate` is still experimental and needs `CLERK_EXPERIMENTAL=migrate`.

packages/cli-core/src/commands/migrate/README.md

@@ -941,7 +941,11 @@ instance (a `key_…` fallback ID), the fix points at the Dashboard instead: in
 `--json` it carries `url` in place of `command`.
 
 The fixes are offers, not corrections: an instance that requires an email is
-configured as its owner intended, and fixing the export may be the answer. When
+configured as its owner intended, and fixing the export may be the answer. A
+fix that only keeps a name or a password counts the users that pass the
+checks: a rejected user brings neither across, so it isn't offered for them. A
+fix that clears a reject, such as making a required name optional, counts
+every user. When
 the instance settings cannot be read (BAPI `/v1/domains` → the instance's
 Frontend API `/v1/environment`), required fields are not checked and the run
 says so. When a development instance's user count cannot be read, the run says

packages/cli-core/src/commands/migrate/lib/checks.test.ts

@@ -1170,6 +1170,59 @@ describe("warnings", () => {
     expect(checks.fixes.map((fix) => fix.label)).toEqual(["Enable First name"]);
   });
 
+  // A rejected user brings no name across, so turning names on helps nobody.
+  test("names no fix for a name only rejected users carry", async () => {
+    const checks = await checkImport(
+      input({
+        settings: bare({
+          email_address: { enabled: true, used_for_first_factor: false, first_factors: [] },
+          password: { enabled: true, used_for_first_factor: false, first_factors: [] },
+          first_name: { enabled: false },
+        }),
+        users: [user("a", { firstName: "Ada" })],
+      }),
+    );
+    expect(checks.rejects).toEqual([
+      { sourceId: "a", reason: "no password, and password is this instance's only way to sign in" },
+    ]);
+    expect(checks.warnings).toEqual([]);
+    expect(checks.fixes).toEqual([]);
+  });
+
+  // A required name rejects the users without one, and making it optional
+  // clears that reject, so the fix counts them.
+  test("still offers to make a required name optional for the users it rejects", async () => {
+    const checks = await checkImport(
+      input({
+        settings: settings({
+          email_address: { enabled: true },
+          first_name: { enabled: true, required: true },
+        }),
+        users: [user("a"), user("b", { firstName: "Ada" })],
+      }),
+    );
+    expect(checks.rejects).toEqual([
+      { sourceId: "a", reason: "no first name, which this instance requires" },
+    ]);
+    expect(checks.fixes.map((fix) => fix.label)).toEqual(["Make First name optional at sign-up"]);
+  });
+
+  // Turning passwords on keeps a password, but a rejected user brings none.
+  test("names no password fix for a password only rejected users carry", async () => {
+    const checks = await checkImport(
+      input({
+        settings: settings({
+          email_address: { enabled: true },
+          phone_number: { enabled: true, required: true },
+          password: { enabled: false },
+        }),
+        users: [user("a", { password: BCRYPT, passwordHasher: "bcrypt" })],
+      }),
+    );
+    expect(checks.rejects.map((reject) => reject.sourceId)).toEqual(["a"]);
+    expect(checks.fixes.map((fix) => fix.label)).toEqual(["Make Phone optional at sign-up"]);
+  });
+
   test("says a password is kept, not dropped, when passwords are off", async () => {
     const checks = await checkImport(
       input({

packages/cli-core/src/commands/migrate/lib/checks.ts

@@ -35,7 +35,7 @@ import {
 } from "./clerk-config.ts";
 import { resolveUserLimit, type UserLimitSource } from "./instance.ts";
 import { buildChangePayload, buildSettingChanges } from "./modify-settings.ts";
-import { acceptsIdentifier, buildReadinessReport } from "./readiness.ts";
+import { acceptsIdentifier, buildReadinessReport, type ReadinessItem } from "./readiness.ts";
 import type { ApiScheduler } from "./scheduler.ts";
 import {
   countSocialProviders,
@@ -923,8 +923,13 @@ function quoteJson(payload: unknown): string {
  *
  * These are offers, not corrections: an instance that requires an email is
  * configured as its owner intended, and fixing the export may be the answer.
+ *
+ * @param users - Everyone a fix could clear a reject for.
+ * @param importable - The users the import sends: a name or password only
+ *   rejected users carry isn't brought across, so a fix that keeps it helps
+ *   nobody.
  */
-function buildFixes(input: CheckInput, users: User[]): Fix[] {
+function buildFixes(input: CheckInput, users: User[], importable: User[]): Fix[] {
   if (!input.settings || users.length === 0) return [];
 
   const report = buildReadinessReport({
@@ -942,7 +947,17 @@ function buildFixes(input: CheckInput, users: User[]): Fix[] {
       const identifiers = splitIdentifiers(user);
       return !identifiers.primaryEmail && identifiers.unverifiedEmails.length > 0;
     });
-  const flagged = report.blocking.slice();
+  // A name or password row that only keeps data clears no reject: what a
+  // rejected user carries isn't brought across, so only the users the import
+  // sends count for it. A row that rejects (a required name) counts everyone.
+  const keepsData = (item: ReadinessItem) =>
+    item.consequence !== "rejects" && (item.section === "model" || item.section === "auth");
+  const kept = new Set(
+    buildReadinessReport({ analysis: analyzeFields(importable), settings: input.settings })
+      .blocking.filter(keepsData)
+      .map((item) => item.key),
+  );
+  const flagged = report.blocking.filter((item) => !keepsData(item) || kept.has(item.key));
   if (
     unverifiedOnly &&
     isRequired(input.settings, "email_address") &&
@@ -1259,6 +1274,9 @@ export async function checkImport(input: CheckInput): Promise<ImportChecks> {
     }
   }
 
+  // The users the import sends, with the fields a disabled setting strips put
+  // back, so the warnings and fixes can name what would be lost.
+  const unstrippedImportable = candidates.map((user) => unstripped.get(user) ?? user);
   return {
     total: input.users.length + input.failures.length,
     importable: candidates.map((user) =>
@@ -1269,10 +1287,7 @@ export async function checkImport(input: CheckInput): Promise<ImportChecks> {
     skipped,
     skippedReasons: countReasons(skipped),
     warnings: [
-      ...buildWarnings(
-        input,
-        candidates.map((user) => unstripped.get(user) ?? user),
-      ),
+      ...buildWarnings(input, unstrippedImportable),
       ...placeholderWarning(candidates.filter((user) => placeholderEmails.has(user.userId)).length),
       ...refusedNameWarning(candidates.filter((user) => refusedNames.has(user.userId)).length),
       ...refusedUsernameWarning(
@@ -1293,6 +1308,7 @@ export async function checkImport(input: CheckInput): Promise<ImportChecks> {
     fixes: buildFixes(
       input,
       input.users.filter((user) => !duplicates.has(user)),
+      unstrippedImportable,
     ),
     ...(quota ? { quota } : {}),
     settingsUnavailable: input.settings === null,