Lookup head commit to push in modal

Summary:
"Commit & Push..." didn't work well, because it would not know the new commit hash to push. It would just use the head commit at modal creation time, which was wrong.

I was back and forth about how to implement this. The trouble is that the commit we can look up the moment you click "commit & push..." can at best be the optimistic commit from the commit operation. But this will become a real commit shortly after opening the modal. In order for this not to disappear, we need to be able to use a reference that is maintained while the modal is open.

I settled on passing a CommitInfo for the top of the stack, and looking up the stack in the Modal. This way, the stack can update if the underlying graph updates. Typically, this shouldn't change while the modal is open. But if it does, we should update the UI to show what clicking "push" would actually do—which is to act on the head commit.

Reviewed By: quark-zju

Differential Revision: D62669564

fbshipit-source-id: ef4ed11e9ba6bedd1ec8c160e02fcc4af6941191

addons/isl/src/CommitInfoView/CommitInfoView.tsx

@@ -5,7 +5,6 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-import type {UICodeReviewProvider} from '../codeReview/UICodeReviewProvider';
 import type {Operation} from '../operations/Operation';
 import type {CommitInfo, DiffId} from '../types';
 import type {CommitInfoMode, EditedMessage} from './CommitInfoState';
@@ -55,7 +54,7 @@ import {SetConfigOperation} from '../operations/SetConfigOperation';
 import {useRunOperation} from '../operationsState';
 import {useUncommittedSelection} from '../partialSelection';
 import platform from '../platform';
-import {CommitPreview, uncommittedChangesWithPreviews} from '../previews';
+import {CommitPreview, dagWithPreviews, uncommittedChangesWithPreviews} from '../previews';
 import {repoRelativeCwd, useIsIrrelevantToCwd} from '../repositoryData';
 import {selectedCommits} from '../selection';
 import {commitByHash, latestHeadCommit, repositoryInfo} from '../serverAPIState';
@@ -93,6 +92,7 @@ import {Badge} from 'isl-components/Badge';
 import {Banner, BannerKind, BannerTooltip} from 'isl-components/Banner';
 import {Button} from 'isl-components/Button';
 import {Divider} from 'isl-components/Divider';
+import {ErrorNotice} from 'isl-components/ErrorNotice';
 import {Column} from 'isl-components/Flex';
 import {Icon} from 'isl-components/Icon';
 import {RadioGroup} from 'isl-components/Radio';
@@ -1025,21 +1025,32 @@ function SubmitButton({
           primary
           disabled={disabledReason != null}
           onClick={async () => {
-            const operations = await getApplicableOperations();
-            if (operations == null || operations.length === 0) {
-              return;
-            }
-
-            for (const operation of operations) {
-              runOperation(operation);
-            }
-
-            const pushOps = await showBranchingPrModal(commit);
-            if (pushOps == null) {
-              return;
-            }
-            for (const pushOp of pushOps) {
-              runOperation(pushOp);
+            try {
+              const operations = await getApplicableOperations();
+              if (operations == null || operations.length === 0) {
+                return;
+              }
+
+              for (const operation of operations) {
+                runOperation(operation);
+              }
+              const dag = readAtom(dagWithPreviews);
+              const topOfStack = commit.isDot && isCommitMode ? dag.resolve('.') : commit;
+              if (topOfStack == null) {
+                throw new Error('could not find commit to push');
+              }
+              const pushOps = await showBranchingPrModal(topOfStack);
+              if (pushOps == null) {
+                return;
+              }
+              for (const pushOp of pushOps) {
+                runOperation(pushOp);
+              }
+            } catch (err) {
+              const error = err as Error;
+              showToast(<ErrorNotice error={error} title={<T>Failed to push commits</T>} />, {
+                durationMs: 10000,
+              });
             }
           }}>
           {commit.isDot && anythingToCommit ? (

addons/isl/src/codeReview/github/BranchingPrModal.tsx

@@ -10,8 +10,6 @@ import type {CommitInfo} from '../../types';
 import type {GithubUICodeReviewProvider} from './github';
 
 import {T} from '../../i18n';
-import {readAtom} from '../../jotaiUtils';
-import {dagWithPreviews} from '../../previews';
 import {showModal} from '../../useModal';
 import {codeReviewProvider} from '../CodeReviewInfo';
 import {ErrorNotice} from 'isl-components/ErrorNotice';
@@ -22,10 +20,8 @@ import {lazy, Suspense} from 'react';
 const BranchingPrModalContent = lazy(() => import('./BranchingPrModalContent'));
 
 export function showBranchingPrModal(
-  topOfStackToPush: CommitInfo,
+  topOfStack: CommitInfo,
 ): Promise<Array<Operation> | undefined> {
-  const dag = readAtom(dagWithPreviews);
-  const stack = dag.getBatch(dag.ancestors(topOfStackToPush.hash, {within: dag.draft()}).toArray());
   return showModal<Array<Operation> | undefined>({
     maxWidth: 'calc(min(90vw, 800px)',
     maxHeight: 'calc(min(90vw, 600px)',
@@ -34,16 +30,16 @@ export function showBranchingPrModal(
     type: 'custom',
     dataTestId: 'create-pr-modal',
     component: ({returnResultAndDismiss}) => (
-      <CreatePrModal stack={stack} returnResultAndDismiss={returnResultAndDismiss} />
+      <CreatePrModal topOfStack={topOfStack} returnResultAndDismiss={returnResultAndDismiss} />
     ),
   });
 }
 
 export function CreatePrModal({
-  stack,
+  topOfStack,
   returnResultAndDismiss,
 }: {
-  stack: Array<CommitInfo>;
+  topOfStack: CommitInfo;
   returnResultAndDismiss: (operations: Array<Operation> | undefined) => unknown;
 }) {
   const provider = useAtomValue(codeReviewProvider);
@@ -63,7 +59,7 @@ export function CreatePrModal({
       </div>
       <BranchingPrModalContent
         provider={provider as GithubUICodeReviewProvider}
-        stack={stack}
+        topOfStack={topOfStack}
         returnResultAndDismiss={returnResultAndDismiss}
       />
     </Suspense>

addons/isl/src/codeReview/github/BranchingPrModalContent.tsx

@@ -14,14 +14,15 @@ import {Commit} from '../../Commit';
 import {Column, FlexSpacer, Row} from '../../ComponentUtils';
 import {T} from '../../i18n';
 import {PushOperation} from '../../operations/PushOperation';
-import {CommitPreview} from '../../previews';
+import {CommitPreview, dagWithPreviews} from '../../previews';
 import {latestSuccessorUnlessExplicitlyObsolete} from '../../successionUtils';
 import * as stylex from '@stylexjs/stylex';
 import {Badge} from 'isl-components/Badge';
 import {Button} from 'isl-components/Button';
 import {Checkbox} from 'isl-components/Checkbox';
 import {Dropdown} from 'isl-components/Dropdown';
 import {HorizontallyGrowingTextField} from 'isl-components/HorizontallyGrowingTextField';
+import {useAtomValue} from 'jotai';
 import {useState} from 'react';
 
 const styles = stylex.create({
@@ -51,16 +52,25 @@ function recommendNewBranchName(stack: Array<CommitInfo>) {
 }
 
 export default function BranchingPrModalContent({
-  stack,
+  topOfStack,
   provider,
   returnResultAndDismiss,
 }: {
-  stack: Array<CommitInfo>;
+  topOfStack: CommitInfo;
   provider: GithubUICodeReviewProvider;
   returnResultAndDismiss: (result: Array<Operation> | undefined) => unknown;
 }) {
   const [createPr, setCreatePr] = useState(false);
 
+  const dag = useAtomValue(dagWithPreviews);
+  // If passed the optimistic isDot commit, we may need to resolve it to a real commit
+  // once the optimistic commit is no longer in the dag.
+  const top =
+    topOfStack.isDot && topOfStack.optimisticRevset != null
+      ? dag.resolve('.') ?? topOfStack
+      : topOfStack;
+  const stack = dag.getBatch(dag.ancestors(top.hash, {within: dag.draft()}).toArray());
+
   const pushChoices = getPushChoices(provider);
   const [pushChoice, setPushChoice] = useState(pushChoices[0]);