Warn on rebase to an older commit

Summary:
Warning for devs rebasing their diffs with a large time deviation (e.g. >12 hours jump)

Indicates the slowness in making this jump and provides alternatives, which are to rebase the target diff against the nearest warm/stable and then make the jump

## Rollout plan
Gk attached in the diff

Reviewed By: evangrayk

Differential Revision: D75969145

fbshipit-source-id: cf34d89f22a7910e7ac30570f1ef549c336bc103

Author: architmodi

addons/isl-server/src/analytics/eventNames.ts

@@ -170,6 +170,7 @@ export type TrackEventName =
   | 'PendingSlocCommand'
   | 'SplitSuggestionsDismissedForSevenDays'
   | 'WarnAboutRebaseOffWarm'
+  | 'WarnAboutDistantRebase'
   | 'SaplingISLUriHandlerHandle'
   | 'CommitInfoFieldEditFieldClick';
 

addons/isl/src/Commit.tsx

@@ -84,6 +84,11 @@ export const rebaseOffWarmWarningEnabled = localStorageBackedAtom<boolean>(
   true,
 );
 
+export const distantRebaseWarningEnabled = localStorageBackedAtom<boolean>(
+  'isl.distant-rebase-warning-enabled',
+  true,
+);
+
 /**
  * Some preview types should not allow actions on top of them
  * For example, you can't click goto on the preview of dragging a rebase,
@@ -303,18 +308,7 @@ export const Commit = memo(
           <Button
             primary
             onClick={() => {
-              handlePreviewedOperation(/* cancel */ false);
-
-              const dag = readAtom(dagWithPreviews);
-              const onto = dag.get(commit.parents[0]);
-              if (onto) {
-                tracker.track('ConfirmDragAndDropRebase', {
-                  extras: {
-                    remoteBookmarks: onto.remoteBookmarks,
-                    locations: onto.stableCommitMetadata?.map(s => s.value),
-                  },
-                });
-              }
+              return handleRebaseConfirmation(commit, handlePreviewedOperation);
             }}>
             <T>Run Rebase</T>
           </Button>
@@ -761,3 +755,76 @@ async function gotoAction(runOperation: ReturnType<typeof useRunOperation>, comm
   writeAtom(selectedCommits, new Set());
 }
 const ObsoleteTip = React.memo(ObsoleteTipInner);
+
+function handleRebaseConfirmation(
+  commit: CommitInfo,
+  handlePreviewedOperation: (cancel: boolean) => void,
+): Promise<void> {
+  return maybeWarnAboutDistantRebase(commit).then(shouldProceed => {
+    if (shouldProceed) {
+      handlePreviewedOperation(/* cancel */ false);
+
+      const dag = readAtom(dagWithPreviews);
+      const onto = dag.get(commit.parents[0]);
+      if (onto) {
+        tracker.track('ConfirmDragAndDropRebase', {
+          extras: {
+            remoteBookmarks: onto.remoteBookmarks,
+            locations: onto.stableCommitMetadata?.map(s => s.value),
+          },
+        });
+      }
+    }
+  });
+}
+
+async function maybeWarnAboutDistantRebase(commit: CommitInfo): Promise<boolean> {
+  const isDistantRebaseWarningEnabled = readAtom(distantRebaseWarningEnabled);
+  if (!isDistantRebaseWarningEnabled) {
+    return true;
+  }
+  const dag = readAtom(dagWithPreviews);
+  const onto = dag.get(commit.parents[0]);
+  if (!onto) {
+    return true; // If there's no target commit, proceed without warning
+  }
+  const currentBase = findPublicBaseAncestor(dag);
+  const destBase = findPublicBaseAncestor(dag, onto.hash);
+  if (!currentBase || !destBase) {
+    // can't determine if we can show warning
+    return Promise.resolve(true);
+  }
+  const warning = Promise.resolve(
+    Internal.maybeWarnAboutDistantRebase?.(currentBase, destBase) ?? false,
+  );
+
+  if (await warning) {
+    tracker.track('WarnAboutDistantRebase');
+    const buttons = [t('Opt Out of Future Warnings'), t('Cancel'), t('Rebase Anyway')];
+    const answer = await showModal({
+      type: 'confirm',
+      buttons,
+      title: <T>Distant Rebase Warning</T>,
+      message: t(
+        Internal.warnAboutDistantRebaseReason ??
+          'The target commit is $age away from your current commit. ' +
+            'Rebasing across a large time gap may cause slower builds and performance. ' +
+            "It's recommended to rebase the destination commit(s) to the nearest stable or warm commit first and then attempt this rebase. " +
+            'Do you want to `rebase` anyway?',
+        {
+          replace: {
+            $age: relativeDate(onto.date, {reference: commit.date, useRelativeForm: true}),
+          },
+        },
+      ),
+    });
+
+    if (answer === buttons[0]) {
+      writeAtom(distantRebaseWarningEnabled, false);
+      return true;
+    }
+    return answer === buttons[2];
+  }
+
+  return true;
+}

addons/isl/src/SettingsTooltip.tsx

@@ -19,7 +19,7 @@ import {Tooltip} from 'isl-components/Tooltip';
 import {useAtom, useAtomValue} from 'jotai';
 import {Suspense} from 'react';
 import {nullthrows, tryJsonParse} from 'shared/utils';
-import {rebaseOffWarmWarningEnabled} from './Commit';
+import {distantRebaseWarningEnabled, rebaseOffWarmWarningEnabled} from './Commit';
 import {condenseObsoleteStacks} from './CommitTreeList';
 import {Column, Row} from './ComponentUtils';
 import {confirmShouldSubmitEnabledAtom} from './ConfirmSubmitStack';
@@ -131,6 +131,7 @@ function SettingsDropdown({
           <RenderCompactSetting />
           <CondenseObsoleteSetting />
           <RebaseOffWarmWarningSetting />
+          <DistantRebaseWarningSetting />
           <DeemphasizeIrrelevantCommitsSetting />
         </Column>
       </Setting>
@@ -320,6 +321,25 @@ function RebaseOffWarmWarningSetting() {
   );
 }
 
+function DistantRebaseWarningSetting() {
+  const [value, setValue] = useAtom(distantRebaseWarningEnabled);
+  return (
+    <Tooltip
+      title={t(
+        'Show a warning when rebasing onto a commit that is significantly older than the current commit.',
+      )}>
+      <Checkbox
+        data-testid="distant-rebase-warning-enabled"
+        checked={value}
+        onChange={checked => {
+          setValue(checked);
+        }}>
+        <T>Show Warning on Distant Rebase</T>
+      </Checkbox>
+    </Tooltip>
+  );
+}
+
 export const openFileCmdAtom = configBackedAtom<string | null>(
   'isl.open-file-cmd',
   null,

addons/isl/src/__tests__/operations.test.tsx

@@ -5,7 +5,7 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-import {act, fireEvent, render, screen, within} from '@testing-library/react';
+import {act, fireEvent, render, screen, waitFor, within} from '@testing-library/react';
 import * as utils from 'shared/utils';
 import App from '../App';
 import {Internal} from '../Internal';
@@ -501,6 +501,9 @@ describe('operations', () => {
 
       dragAndDropCommits('c', 'a');
       fireEvent.click(screen.getByText('Run Rebase'));
+      await waitFor(() => {
+        expect(screen.getByText('rebasing...')).toBeInTheDocument();
+      });
       await clickGoto('d'); // checkout d, which is now optimistic from the rebase, since it'll actually become d2.
 
       act(() =>

addons/isl/src/__tests__/operations/rebaseOperation.test.tsx

@@ -107,26 +107,27 @@ describe('rebase operation', () => {
     expect(screen.queryAllByTestId('commit-1')).toHaveLength(1);
   });
 
-  it('runs rebase operation', () => {
+  it('runs rebase operation', async () => {
     dragAndDropCommits('d', '2');
 
     const runRebaseButton = screen.getByText('Run Rebase');
     expect(runRebaseButton).toBeInTheDocument();
 
     fireEvent.click(runRebaseButton);
-
-    expectMessageSentToServer({
-      type: 'runOperation',
-      operation: {
-        args: ['rebase', '-s', succeedableRevset('d'), '-d', succeedableRevset('remote/master')],
-        id: expect.anything(),
-        runner: CommandRunner.Sapling,
-        trackEventName: 'RebaseOperation',
-      },
+    await waitFor(() => {
+      expectMessageSentToServer({
+        type: 'runOperation',
+        operation: {
+          args: ['rebase', '-s', succeedableRevset('d'), '-d', succeedableRevset('remote/master')],
+          id: expect.anything(),
+          runner: CommandRunner.Sapling,
+          trackEventName: 'RebaseOperation',
+        },
+      });
     });
   });
 
-  it('shows optimistic preview of rebase', () => {
+  it('shows optimistic preview of rebase', async () => {
     dragAndDropCommits('d', '2');
 
     fireEvent.click(screen.getByText('Run Rebase'));
@@ -136,14 +137,16 @@ describe('rebase operation', () => {
     // also includes descendants
     expect(screen.queryAllByTestId('commit-e')).toHaveLength(1);
 
-    expect(screen.getByText('rebasing...')).toBeInTheDocument();
+    await waitFor(() => {
+      expect(screen.getByText('rebasing...')).toBeInTheDocument();
+    });
 
     expect(
       screen.queryByTestId('commit-d')?.querySelector('.commit-preview-rebase-optimistic-root'),
     ).toBeInTheDocument();
   });
 
-  it('allows re-dragging a previously dragged commit back onto the same parent', () => {
+  it('allows re-dragging a previously dragged commit back onto the same parent', async () => {
     // Drag and drop 'e' normally onto 'c'
     dragCommits('e', 'd');
     expect(screen.queryByText('Run Rebase')).not.toBeInTheDocument(); // dragging on original parent is noop
@@ -158,15 +161,16 @@ describe('rebase operation', () => {
     dropCommits(getCommitWithPreview('e', CommitPreview.REBASE_ROOT), 'c');
 
     fireEvent.click(screen.getByText('Run Rebase'));
-
-    expectMessageSentToServer({
-      type: 'runOperation',
-      operation: {
-        args: ['rebase', '-s', succeedableRevset('e'), '-d', succeedableRevset('c')],
-        id: expect.anything(),
-        runner: CommandRunner.Sapling,
-        trackEventName: 'RebaseOperation',
-      },
+    await waitFor(() => {
+      expectMessageSentToServer({
+        type: 'runOperation',
+        operation: {
+          args: ['rebase', '-s', succeedableRevset('e'), '-d', succeedableRevset('c')],
+          id: expect.anything(),
+          runner: CommandRunner.Sapling,
+          trackEventName: 'RebaseOperation',
+        },
+      });
     });
   });
 
@@ -276,7 +280,7 @@ describe('rebase operation', () => {
       expect(scanForkedBranchHashes('b')).toEqual(['d', 'e']);
     });
 
-    it('can preview drag drop while previous rebase running', () => {
+    it('can preview drag drop while previous rebase running', async () => {
       //              c
       //       c      | e
       // e     b      |/
@@ -286,6 +290,9 @@ describe('rebase operation', () => {
       // a     a      a
       dragAndDropCommits('d', 'a');
       fireEvent.click(screen.getByText('Run Rebase'));
+      await waitFor(() => {
+        expect(screen.getByText('rebasing...')).toBeInTheDocument();
+      });
 
       dragAndDropCommits(
         getCommitWithPreview('e', CommitPreview.REBASE_OPTIMISTIC_DESCENDANT),
@@ -298,7 +305,7 @@ describe('rebase operation', () => {
       expect(scanForkedBranchHashes('b')).toEqual(['e']);
     });
 
-    it('can see optimistic drag drop while previous rebase running', () => {
+    it('can see optimistic drag drop while previous rebase running', async () => {
       //              c
       //       c      | e
       // e     b      |/
@@ -308,7 +315,9 @@ describe('rebase operation', () => {
       // a     a      a
       dragAndDropCommits('d', 'a');
       fireEvent.click(screen.getByText('Run Rebase'));
-
+      await waitFor(() => {
+        expect(screen.getByText('rebasing...')).toBeInTheDocument();
+      });
       dragAndDropCommits(
         getCommitWithPreview('e', CommitPreview.REBASE_OPTIMISTIC_DESCENDANT),
         'b',

addons/isl/src/types.ts

@@ -800,6 +800,7 @@ export type LocalStorageName =
   | 'isl.warn-about-diagnostics'
   | 'isl.hide-non-blocking-diagnostics'
   | 'isl.rebase-off-warm-warning-enabled'
+  | 'isl.distant-rebase-warning-enabled'
   | 'isl.experimental-features-local-override'
   // These keys are prefixes, with further dynamic keys appended afterwards
   | 'isl.edited-commit-messages:'

addons/isl/src/utils.ts

@@ -80,3 +80,10 @@ export function registerDisposable(
 ): void {
   registerCleanup(obj, () => disposable.dispose(), hot);
 }
+
+/** Calculate hour difference between two dates (date1 - date2) */
+export function calculateHourDifference(date1: Date, date2: Date): number {
+  const msDifference = date1.getTime() - date2.getTime();
+  const hoursDifference = msDifference / (1000 * 60 * 60);
+  return hoursDifference;
+}