fix(graph): disable primary CTA for Migrate UI when approval is required first (#30836)

This PR updates the Migrate UI such that the primary CTA always performs an action. Previously, when there is a migration to approve, the primary CTA says `Run Migrations`, but clicking it does nothing since we're already in the middle of running migrations. <img width="1093" alt="Screenshot 2025-04-23 at 9 36 05 AM" src="https://github.com/user-attachments/assets/4d55e3d0-d16b-4c4b-9b16-551690be60ab" /> Now, with this PR, the primary CTA will be disabled and tell the user to approve the current migration before continuing. ## Current Behavior Primary CTA in Migrate UI does nothing when there is a migration to approve ## Expected Behavior Primary CTA should reflect the current action that the user must take, in this case to approve the migration ## Related Issue(s)  Fixes # --------- Co-authored-by: Nicholas Cunningham <ndcunningham@gmail.com>
2025-04-24 17:31:13 -04:00 · 2025-04-24 17:31:13 -04:00 · 8397a95a7e
commit 8397a95a7e
parent 4254c4bcce
7 changed files with 161 additions and 27 deletions
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@ -122,7 +122,7 @@ jobs:
        uses: actions/cache@v4
        with:
          path: |
-            /usr/local/Homebrew
+            /opt/Homebrew
            ~/Library/Caches/Homebrew
          key: nrwl-nx-homebrew-packages

@ -137,7 +137,7 @@ jobs:
        uses: actions/cache@v4
        with:
          path: |
-            /usr/local/Homebrew
+            /opt/Homebrew
            ~/Library/Caches/Homebrew
          key: nrwl-nx-homebrew-packages

--- a/graph/migrate/.storybook/main.ts
+++ b/graph/migrate/.storybook/main.ts
@ -11,10 +11,13 @@ const config: StorybookConfig = {
    options: {},
  },

-  viteFinal: async (config) =>
-    mergeConfig(config, {
-      plugins: [],
-    }),
+  viteFinal: async (config) => {
+    const {
+      nxViteTsPaths,
+      // nx-ignore-next-line
+    } = require('@nx/vite/plugins/nx-tsconfig-paths.plugin');
+    return mergeConfig(config, { plugins: [nxViteTsPaths()] });
+  },

  docs: {},
 };
--- a/graph/migrate/src/lib/components/migration-timeline.tsx
+++ b/graph/migrate/src/lib/components/migration-timeline.tsx
@ -12,6 +12,9 @@ import {
  CheckCircleIcon,
  ClockIcon,
  MinusIcon,
+  ArrowPathIcon,
+  XMarkIcon,
+  CheckIcon,
 } from '@heroicons/react/24/outline';
 import { useEffect, useState, useRef } from 'react';
 import { MigrationCard, MigrationCardHandle } from './migration-card';
@ -204,8 +207,12 @@ export function MigrationTimeline({
                                onRunMigration(migration);
                              }}
                              type="button"
-                              className="rounded-md border border-red-500 bg-red-500 px-4 py-2 text-sm font-medium text-white shadow-sm hover:bg-red-600 dark:border-red-700 dark:bg-red-600 dark:text-white hover:dark:bg-red-700"
+                              className="flex items-center gap-2 rounded-md border border-slate-300 bg-white px-4 py-2 text-sm font-medium text-slate-700 shadow-sm transition-colors hover:bg-slate-50 dark:border-slate-600 dark:bg-slate-800 dark:text-slate-300 hover:dark:bg-slate-700"
                            >
+                              <ArrowPathIcon
+                                className="h-5 w-5"
+                                aria-hidden="true"
+                              />{' '}
                              Rerun
                            </button>
                          )}
@ -266,6 +273,10 @@ export function MigrationTimeline({
              <MigrationStateCircle
                migration={migrations[currentMigrationIndex]}
                nxConsoleMetadata={nxConsoleMetadata}
+                needsAttention={
+                  currentMigrationHasChanges &&
+                  !expandedMigrations[currentMigration.id]
+                }
                isRunning={currentMigrationRunning}
                onClick={() => toggleMigrationExpanded(currentMigration.id)}
              />
@ -300,8 +311,12 @@ export function MigrationTimeline({
                            onRunMigration(currentMigration);
                          }}
                          type="button"
-                          className="rounded-md border border-red-500 bg-red-500 px-4 py-2 text-sm font-medium text-white shadow-sm hover:bg-red-600 dark:border-red-700 dark:bg-red-600 dark:text-white hover:dark:bg-red-700"
+                          className="flex items-center gap-2 rounded-md border border-slate-300 bg-white px-4 py-2 text-sm font-medium text-slate-700 shadow-sm transition-colors hover:bg-slate-50 dark:border-slate-600 dark:bg-slate-800 dark:text-slate-300 hover:dark:bg-slate-700"
                        >
+                          <ArrowPathIcon
+                            className="h-5 w-5"
+                            aria-hidden="true"
+                          />{' '}
                          Rerun
                        </button>
                      )}
@ -312,8 +327,9 @@ export function MigrationTimeline({
                            onSkipMigration(currentMigration);
                          }}
                          type="button"
-                          className="rounded-md border border-slate-500 bg-slate-500 px-4 py-2 text-sm font-medium text-white shadow-sm hover:bg-slate-600 dark:border-slate-600 dark:bg-slate-600 dark:text-white hover:dark:bg-slate-700"
+                          className="flex items-center rounded-md border border-red-500 bg-red-500 px-4 py-2 text-sm font-medium text-white shadow-sm hover:bg-red-600 dark:border-red-700 dark:bg-red-600 hover:dark:bg-red-700"
                        >
+                          <XMarkIcon className="h-5 w-5" aria-hidden="true" />{' '}
                          Skip
                        </button>
                      )}
@ -325,8 +341,9 @@ export function MigrationTimeline({
                            onReviewMigration(currentMigration.id);
                          }}
                          type="button"
-                          className="flex items-center rounded-md border border-green-500 bg-green-500 px-4 py-2 text-sm font-medium text-white shadow-sm hover:bg-green-600 dark:border-green-700 dark:bg-green-600 dark:text-white hover:dark:bg-green-700"
+                          className="flex items-center gap-2 rounded-md border border-blue-500 bg-blue-500 px-4 py-2 text-sm font-medium text-white shadow-sm hover:bg-blue-600 dark:border-blue-600 dark:bg-blue-600 hover:dark:bg-blue-700"
                        >
+                          <CheckIcon className="h-5 w-5" aria-hidden="true" />{' '}
                          Approve Changes
                        </button>
                      )}
@ -404,8 +421,12 @@ export function MigrationTimeline({
                                onSkipMigration(migration);
                              }}
                              type="button"
-                              className="rounded-md border border-slate-500 bg-slate-500 px-4 py-2 text-sm font-medium text-white shadow-sm hover:bg-slate-600 dark:border-slate-600 dark:bg-slate-600 dark:text-white hover:dark:bg-slate-700"
+                              className="flex items-center gap-2 rounded-md border border-red-500 bg-red-500 px-4 py-2 text-sm font-medium text-white shadow-sm hover:bg-red-600 dark:border-red-700 dark:bg-red-600 hover:dark:bg-red-700"
                            >
+                              <XMarkIcon
+                                className="h-5 w-5"
+                                aria-hidden="true"
+                              />{' '}
                              Skip
                            </button>
                          </div>
@ -505,6 +526,7 @@ interface MigrationStateCircleProps {
  migration: MigrationDetailsWithId;
  nxConsoleMetadata: MigrationsJsonMetadata;
  isRunning?: boolean;
+  needsAttention?: boolean;
  onClick: () => void;
 }

@ -512,6 +534,7 @@ function MigrationStateCircle({
  migration,
  nxConsoleMetadata,
  isRunning,
+  needsAttention,
  onClick,
 }: MigrationStateCircleProps) {
  let bgColor = '';
@ -551,7 +574,10 @@ function MigrationStateCircle({

  return (
    <div
-      className={`absolute left-0 top-0 flex h-8 w-8 -translate-x-1/2 cursor-pointer items-center justify-center rounded-full ${bgColor} ${textColor}`}
+      className={twMerge(
+        `absolute left-0 top-0 flex h-8 w-8 -translate-x-1/2 cursor-pointer items-center justify-center rounded-full ${bgColor} ${textColor}`,
+        needsAttention ? 'animate-pulse' : ''
+      )}
      onClick={onClick}
    >
      {isRunning ? (
--- a/graph/migrate/src/lib/migrate.stories.tsx
+++ b/graph/migrate/src/lib/migrate.stories.tsx
@ -234,3 +234,76 @@ export const AllCompleted: Story = {
    },
  },
 };
+
+export const PendingApproval: Story = {
+  args: {
+    currentMigrationId: 'migration-2',
+    migrations: [
+      {
+        id: 'migration-1',
+        name: 'migration-1',
+        description: 'Migrate 1',
+        version: '1.0.0',
+        package: 'nx',
+        implementation: './src/migrations/migration-1.ts',
+      },
+      {
+        id: 'migration-2',
+        name: 'migration-2',
+        description: 'Migrate 2',
+        version: '1.0.1',
+        package: '@nx/react',
+        implementation: './src/migrations/migration-2.ts',
+      },
+      {
+        id: 'migration-3',
+        name: 'migration-3',
+        description: 'Migrate 3',
+        version: '1.0.1',
+        package: '@nx/react',
+        implementation: './src/migrations/migration-3.ts',
+      },
+    ],
+    nxConsoleMetadata: {
+      completedMigrations: {
+        'migration-1': {
+          name: 'migration-1',
+          type: 'successful',
+          changedFiles: [],
+          ref: '123',
+        },
+        'migration-2': {
+          name: 'migration-2',
+          type: 'successful',
+          changedFiles: [{ path: 'foo.txt', type: 'CREATE' }],
+          ref: '124',
+        },
+      },
+      targetVersion: '20.3.2',
+    },
+    onRunMigration: (migration) => {
+      console.log('run migration', migration);
+    },
+    onRunMany: (migrations, configuration) => {
+      console.log('run many migrations', migrations, configuration);
+    },
+    onSkipMigration: (migration) => {
+      console.log('skip migration', migration);
+    },
+    onFileClick: (migration, file) => {
+      console.log('file click', migration, file);
+    },
+    onViewImplementation: (migration) => {
+      console.log('view implementation', migration);
+    },
+    onViewDocumentation: (migration) => {
+      console.log('view documentation', migration);
+    },
+    onCancel: () => {
+      console.log('cancel');
+    },
+    onFinish: (squash: boolean) => {
+      console.log('finished', squash);
+    },
+  },
+};
--- a/graph/migrate/src/lib/migrate.tsx
+++ b/graph/migrate/src/lib/migrate.tsx
@ -21,6 +21,7 @@ import {
 export interface MigrateUIProps {
  migrations: MigrationDetailsWithId[];
  nxConsoleMetadata: MigrationsJsonMetadata;
+  currentMigrationId?: string;
  onRunMigration: (
    migration: MigrationDetailsWithId,
    configuration: {
@ -48,6 +49,7 @@ export interface MigrateUIProps {
 export enum PrimaryAction {
  RunMigrations = 'Run Migrations',
  PauseMigrations = 'Pause Migrations',
+  ApproveChanges = 'Approve Changes to Continue',
  FinishWithoutSquashingCommits = 'Finish without squashing commits',
  FinishSquashingCommits = 'Finish (squash commits)',
 }
@ -73,13 +75,18 @@ export function MigrateUI(props: MigrateUIProps) {
  });
  const isDone = useSelector(actor, (state) => state.matches('done'));
  const isInit = useSelector(actor, (state) => state.matches('init'));
-  const running = useSelector(actor, (state) => state.matches('running'));
+  const isRunning = useSelector(actor, (state) => state.matches('running'));
+
+  const isNeedReview = useSelector(actor, (state) =>
+    state.matches('needsReview')
+  );

  useEffect(() => {
    actor.send({
      type: 'loadInitialData',
      migrations: props.migrations,
      metadata: props.nxConsoleMetadata,
+      currentMigrationId: props.currentMigrationId,
    });
    // eslint-disable-next-line react-hooks/exhaustive-deps -- only load initial data when migrations change
  }, [JSON.stringify(props.migrations)]);
@ -99,18 +106,23 @@ export function MigrateUI(props: MigrateUIProps) {

  useEffect(() => {
    if (
-      (primaryAction === PrimaryAction.RunMigrations ||
+      (primaryAction === PrimaryAction.ApproveChanges ||
+        primaryAction === PrimaryAction.RunMigrations ||
        primaryAction === PrimaryAction.PauseMigrations) &&
      !isInit
    ) {
      setPrimaryAction(
-        running ? PrimaryAction.PauseMigrations : PrimaryAction.RunMigrations
+        isRunning
+          ? PrimaryAction.PauseMigrations
+          : isNeedReview
+          ? PrimaryAction.ApproveChanges
+          : PrimaryAction.RunMigrations
      );
    }
-  }, [running, primaryAction, isInit]);
+  }, [isRunning, primaryAction, isInit, isNeedReview]);

  const handlePauseResume = () => {
-    if (running) {
+    if (isRunning) {
      actor.send({ type: 'pause' });
    } else {
      actor.send({ type: 'startRunning' });
@ -195,12 +207,13 @@ export function MigrateUI(props: MigrateUIProps) {
          >
            Cancel
          </button>
-          <div className="flex">
+          <div className="group flex">
            <button
              onClick={handlePrimaryActionSelection}
              type="button"
-              title="Finish"
-              className="whitespace-nowrap rounded-l-md border border-blue-700 bg-blue-500 px-4 py-2 text-sm font-medium text-white shadow-sm hover:bg-blue-600 dark:border-blue-700 dark:bg-blue-600 dark:text-white hover:dark:bg-blue-700"
+              title={primaryAction}
+              disabled={isNeedReview}
+              className="whitespace-nowrap rounded-l-md border border-blue-700 bg-blue-500 px-4 py-2 text-sm font-medium text-white shadow-sm hover:bg-blue-600 disabled:cursor-not-allowed disabled:border-blue-400 disabled:bg-blue-400 disabled:opacity-50 dark:border-blue-700 dark:bg-blue-600 dark:text-white hover:dark:bg-blue-700"
            >
              {primaryAction}
            </button>
@ -208,7 +221,8 @@ export function MigrateUI(props: MigrateUIProps) {
              <button
                type="button"
                onClick={() => setIsOpen((prev) => !prev)}
-                className="border-l-1 flex items-center rounded-r-md border border-blue-700 bg-blue-500 px-2 py-2 text-sm font-medium text-white shadow-sm hover:bg-blue-700 dark:border-blue-700 dark:bg-blue-700 dark:text-white hover:dark:bg-blue-800"
+                disabled={isNeedReview}
+                className="border-l-1 flex items-center rounded-r-md border border-blue-700 bg-blue-500 px-2 py-2 text-sm font-medium text-white shadow-sm hover:bg-blue-700 disabled:cursor-not-allowed disabled:border-blue-400 disabled:bg-blue-400 disabled:opacity-50 dark:border-blue-700 dark:bg-blue-700 dark:text-white hover:dark:bg-blue-800"
              >
                <ChevronDownIcon className="h-4 w-4" />
              </button>
@ -224,7 +238,7 @@ export function MigrateUI(props: MigrateUIProps) {
                  {!isDone && (
                    <>
                      {' '}
-                      {!running && (
+                      {!isRunning && (
                        <li
                          className="flex cursor-pointer items-center gap-2 p-2 hover:bg-slate-50 dark:border-slate-600 dark:bg-slate-800 dark:text-slate-300 hover:dark:bg-slate-700"
                          onClick={() => {
@ -244,7 +258,7 @@ export function MigrateUI(props: MigrateUIProps) {
                          <span>{'Run Migrations'}</span>
                        </li>
                      )}
-                      {running && (
+                      {isRunning && (
                        <li
                          className="flex cursor-pointer items-center gap-2 p-2 hover:bg-slate-50 dark:border-slate-600 dark:bg-slate-800 dark:text-slate-300 hover:dark:bg-slate-700"
                          onClick={() => {
--- a/graph/migrate/src/lib/state/automatic/machine.ts
+++ b/graph/migrate/src/lib/state/automatic/machine.ts
@ -37,6 +37,15 @@ export const machine = createMachine<
              target: 'running',
            },
          ],
+          reviewMigration: [
+            {
+              cond: 'currentMigrationCanLeaveReview',
+              target: 'running',
+              actions: assign((ctx, event) => {
+                ctx.reviewedMigrations.push(event.migrationId);
+              }),
+            },
+          ],
        },
      },
      running: {
@ -74,6 +83,13 @@ export const machine = createMachine<
              target: 'paused',
            },
          ],
+          reviewMigration: {
+            cond: 'currentMigrationCanLeaveReview',
+            target: 'running',
+            actions: assign((ctx, event) => {
+              ctx.reviewedMigrations.push(event.migrationId);
+            }),
+          },
        },
        always: [
          {
@ -91,10 +107,11 @@ export const machine = createMachine<
            assign((ctx, event) => {
              ctx.migrations = event.migrations;
              ctx.nxConsoleMetadata = event.metadata;
-              ctx.currentMigration = findFirstIncompleteMigration(
-                event.migrations,
-                event.metadata
-              );
+              ctx.currentMigration =
+                event.migrations.find(
+                  (m) => m.id === event.currentMigrationId
+                ) ??
+                findFirstIncompleteMigration(event.migrations, event.metadata);
            }),
          ],
        },
--- a/graph/migrate/src/lib/state/automatic/types.ts
+++ b/graph/migrate/src/lib/state/automatic/types.ts
@ -16,6 +16,7 @@ export type AutomaticMigrationEvents =
      type: 'loadInitialData';
      migrations: MigrationDetailsWithId[];
      metadata: MigrationsJsonMetadata;
+      currentMigrationId?: string;
    }
  | {
      type: 'updateMetadata';