Skip to content

Commit

Permalink
Only show fetch from cloud button if commit cloud enabled
Browse files Browse the repository at this point in the history
Summary:
Check if the commitcloud extension is enabled in order to show the fetch from cloud button after loading all commits.

I rearranged the fetch code into its own component so it can use Suspense to lazy load the commitcloud fetch.

I had to make a new atom to store this enabled state, as we only had an expensive check for cloud sync status available.

This new check means we include `extensions.commitcloud` in our sl config call, which unforutnately means we get all `extension.*` values... I'm thinking of updating our call to avoid this. We tried this in the past but it requires a newer version of Sapling, which we can't guarantee in OSS.

Reviewed By: nsblake

Differential Revision: D68041107

fbshipit-source-id: 4fdfd7d9e98d26e529b75977ca44c6af8f5e7e17
  • Loading branch information
evangrayk authored and facebook-github-bot committed Jan 13, 2025
1 parent abf49d1 commit 38f05f6
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 30 deletions.
16 changes: 16 additions & 0 deletions addons/isl/src/CommitCloud.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {CommitCloudSyncOperation} from './operations/CommitCloudSyncOperation';
import {useRunOperation} from './operationsState';
import {CommitPreview, dagWithPreviews, useMostRecentPendingOperation} from './previews';
import {RelativeDate} from './relativeDate';
import {repoRootAndCwd} from './repositoryData';
import {CommitCloudBackupStatus} from './types';
import {registerDisposable} from './utils';
import {Button} from 'isl-components/Button';
Expand All @@ -34,6 +35,21 @@ import {notEmpty} from 'shared/utils';

import './CommitCloud.css';

export const commitCloudEnabledAtom = atom(async (get): Promise<boolean> => {
// depend on the cwd so we recheck when the cwd changes
get(repoRootAndCwd);

serverAPI.postMessage({
type: 'getConfig',
name: 'extensions.commitcloud',
});
const message = await serverAPI.nextMessageMatching('gotConfig', message => {
return message.name === 'extensions.commitcloud';
});
const enabled = message.value != null && message.value !== '!';
return enabled;
});

const cloudSyncStateAtom = atom<Result<CommitCloudSyncState> | null>(null);

registerDisposable(
Expand Down
71 changes: 44 additions & 27 deletions addons/isl/src/FetchAdditionalCommitsButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import serverAPI from './ClientToServerAPI';
import {commitCloudEnabledAtom} from './CommitCloud';
import {t, T} from './i18n';
import {writeAtom} from './jotaiUtils';
import {CommitCloudSyncOperation} from './operations/CommitCloudSyncOperation';
Expand All @@ -16,13 +17,16 @@ import {Button} from 'isl-components/Button';
import {Icon} from 'isl-components/Icon';
import {DOCUMENTATION_DELAY, Tooltip} from 'isl-components/Tooltip';
import {atom, useAtomValue} from 'jotai';
import {Suspense} from 'react';

export function FetchingAdditionalCommitsRow() {
return (
<div className="fetch-additional-commits-row">
<FetchingAdditionalCommitsButton />
<FetchingAdditionalCommitsIndicator />
</div>
<Suspense>
<div className="fetch-additional-commits-row">
<FetchingAdditionalCommitsButton />
<FetchingAdditionalCommitsIndicator />
</div>
</Suspense>
);
}

Expand All @@ -35,44 +39,57 @@ function FetchingAdditionalCommitsIndicator() {

function FetchingAdditionalCommitsButton() {
const shownRange = useAtomValue(commitsShownRange);
const isRunningSync = useIsOperationRunningOrQueued(CommitCloudSyncOperation) != null;
const isFetching = useAtomValue(isFetchingAdditionalCommits);
const isLoading = isFetching || isRunningSync;
const isLoading = useAtomValue(isFetchingAdditionalCommits);
const hasAlreadySynced = useAtomValue(hasSyncedFromCloudAtom);
const runOperation = useRunOperation();
if (shownRange === undefined && hasAlreadySynced) {
return null;
}
const fetchFromCloudNext = shownRange == null;
const commitsShownMessage = fetchFromCloudNext
? t('Showing full commit history. Click to fetch all commits from Commit Cloud')
: t('Showing commits from the last $numDays days', {
replace: {$numDays: shownRange.toString()},
});
if (fetchFromCloudNext) {
return <LoadMoreFromCloudButton />;
}
const commitsShownMessage = t('Showing commits from the last $numDays days', {
replace: {$numDays: shownRange.toString()},
});
return (
<Tooltip placement="top" delayMs={DOCUMENTATION_DELAY} title={commitsShownMessage}>
<Button
disabled={isLoading}
onClick={() => {
if (fetchFromCloudNext) {
runOperation(new CommitCloudSyncOperation(/* full */ true)).then(() =>
writeAtom(hasSyncedFromCloudAtom, true),
);
return;
}

serverAPI.postMessage({
type: 'loadMoreCommits',
});
}}
icon>
{fetchFromCloudNext ? (
<>
<Icon icon={isLoading ? 'spinner' : 'cloud-download'} /> Fetch all cloud commits
</>
) : (
<T>Load more commits</T>
)}
<T>Load more commits</T>
</Button>
</Tooltip>
);
}

function LoadMoreFromCloudButton() {
const runOperation = useRunOperation();
const isRunning = useIsOperationRunningOrQueued(CommitCloudSyncOperation) != null;
const isFetching = useAtomValue(isFetchingAdditionalCommits);
const isLoading = isRunning || isFetching;
const isCloudEnabled = useAtomValue(commitCloudEnabledAtom);
if (!isCloudEnabled) {
return null;
}
return (
<Tooltip
placement="top"
delayMs={DOCUMENTATION_DELAY}
title={t('Showing full commit history. Click to fetch all commits from Commit Cloud')}>
<Button
disabled={isLoading}
onClick={() => {
runOperation(new CommitCloudSyncOperation(/* full */ true)).then(() =>
writeAtom(hasSyncedFromCloudAtom, true),
);
}}
icon>
<Icon icon={isLoading ? 'spinner' : 'cloud-download'} /> Fetch all cloud commits
</Button>
</Tooltip>
);
Expand Down
28 changes: 27 additions & 1 deletion addons/isl/src/__tests__/FetchAdditionalCommitsButton.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,12 @@ describe('CommitTreeList', () => {
act(() => simulateMessageFromServer({type: 'beganLoadingMoreCommits'}));
act(() => simulateCommits({value: allCommits}));

expect(screen.getByText('Fetch all cloud commits'));
expectMessageSentToServer({type: 'getConfig', name: 'extensions.commitcloud'});
act(() =>
simulateMessageFromServer({type: 'gotConfig', name: 'extensions.commitcloud', value: ''}),
);

await waitFor(() => expect(screen.getByText('Fetch all cloud commits')));
mockNextOperationId('1');
fireEvent.click(screen.getByText('Fetch all cloud commits'));
expectMessageSentToServer({
Expand Down Expand Up @@ -111,4 +116,25 @@ describe('CommitTreeList', () => {
expect(screen.queryByText('Fetch all cloud commits')).not.toBeInTheDocument();
});
});

it('does not show cloud sync button if commit cloud not enabled', async () => {
fireEvent.click(screen.getByText('Load more commits'));
expectMessageSentToServer({type: 'loadMoreCommits'});
act(() => simulateMessageFromServer({type: 'commitsShownRange', rangeInDays: 60}));
act(() => simulateMessageFromServer({type: 'beganLoadingMoreCommits'}));
act(() => simulateCommits({value: allCommits}));

fireEvent.click(screen.getByText('Load more commits'));
expectMessageSentToServer({type: 'loadMoreCommits'});
act(() => simulateMessageFromServer({type: 'commitsShownRange', rangeInDays: undefined}));
act(() => simulateMessageFromServer({type: 'beganLoadingMoreCommits'}));
act(() => simulateCommits({value: allCommits}));

expectMessageSentToServer({type: 'getConfig', name: 'extensions.commitcloud'});
await act(async () =>
simulateMessageFromServer({type: 'gotConfig', name: 'extensions.commitcloud', value: '!'}),
);

expect(screen.queryByText('Fetch all cloud commits')).not.toBeInTheDocument();
});
});
2 changes: 1 addition & 1 deletion addons/isl/src/jotaiUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ export function atomWithOnChange<T>(
* Creates a lazily initialized atom.
* On first read, trigger `load` to get the actual value.
* `fallback` provides the value when the async `load` is running.
* `original` is an optioinal nullable atom to provide the value.
* `original` is an optional nullable atom to provide the value.
*/
export function lazyAtom<T>(
load: (get: Getter) => Promise<T> | T,
Expand Down
2 changes: 1 addition & 1 deletion addons/isl/src/repositoryData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export const irrelevantCwdDeemphasisEnabled = localStorageBackedAtom<boolean>(
* A string of repo root and the "cwd". Note a same "cwd" does not infer the same repo,
* when there are nested (ex. submodule) repos.
*/
const repoRootAndCwd = atom<string>(get => `${get(serverCwd)}\n${get(repoRoot)}`);
export const repoRootAndCwd = atom<string>(get => `${get(serverCwd)}\n${get(repoRoot)}`);

/** A specific version of `atomResetOnDepChange`. */
export function atomResetOnCwdChange<T>(defaultValue: T) {
Expand Down
1 change: 1 addition & 0 deletions addons/isl/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -734,6 +734,7 @@ export const allConfigNames = [
'ui.username',
'ui.merge',
'fbcodereview.code-browser-url',
'extensions.commitcloud',
] as const;

/** sl configs read by ISL */
Expand Down

0 comments on commit 38f05f6

Please sign in to comment.