Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
import { type Href, Redirect, Stack, useLocalSearchParams } from 'expo-router';
import { useMemo } from 'react';

import { appUnlockScreenLayout } from '@/components/app-unlock-screen';
import { InvalidRouteState } from '@/components/invalid-route-state';
import { PrReviewConnectGate } from '@/components/pr-review/pr-review-connect-gate';
import { useFormSheetDetents } from '@/lib/form-sheet';
import { useCurrentUserId } from '@/lib/hooks/use-current-user-id';
import { useRouteForegroundRefresh } from '@/lib/hooks/use-route-foreground-refresh';
import { useOrganization } from '@/lib/organization-context';
import {
pendingReviewDraftKey,
PendingReviewProvider,
} from '@/lib/pr-review/pending-review-provider';
import {
parseProviderPrRoute,
providerPrRefKey,
providerPrRoutePath,
ProviderPrScopeProvider,
providerPrTriple,
} from '@/lib/pr-review/provider-pr-ref';
import { parseParam } from '@/lib/route-params';

type Params = {
platform: string;
identity: string[];
instance?: string;
};

/**
* Param guard + scope hoist for the provider PR-review surface.
*
* The route is `[platform]/[...identity]`, where the LAST identity segment is
* the number (a GitLab MR iid, a Bitbucket PR id) and everything before it is
* the project path — a GitLab FULL nested path (`group/sub/repo`) or a
* Bitbucket `workspace/repo`. `parseProviderPrRoute` validates every segment,
* so a hand-built deep link with a missing, repeated or non-numeric segment
* never reaches a query.
*
* GitHub keeps its original `[owner]/[repo]/[number]` route untouched — this
* layout only redirects a hand-built `/pr-review/github/...` link there so
* that surface, its connect gate and its write sheets stay exactly as they
* were.
*
* The scope (ref + organization) is published in context rather than threaded
* through props: the diff list, the file navigator and the discussion tree
* take the GitHub-shaped `owner`/`repo`/`number` triple, and reading the real
* ref from context moves their queries to the right provider without a
* per-provider copy of that tree.
*/
export default function ProviderPrReviewLayout() {
const params = useLocalSearchParams<Params>();
const platform = parseParam(params.platform) ?? '';
// A catch-all param is a fresh array on every render; the joined form is a
// stable dependency, and a `/` inside a segment is percent-encoded by
// `providerPrRoutePath`, so splitting it back is lossless.
const identity = Array.isArray(params.identity)
? params.identity.join('/')
: (parseParam(params.identity) ?? '');
const instance = parseParam(params.instance) ?? '';
const { organizationId } = useOrganization();
const { fullSheetDetent } = useFormSheetDetents();
const { userId } = useCurrentUserId();
useRouteForegroundRefresh([[['providerReview']]]);

const ref = useMemo(
() =>
parseProviderPrRoute({
platform,
identity: identity.split('/'),
instance: instance || undefined,
}),
[platform, identity, instance]
);
const scope = useMemo(() => (ref ? { ref, organizationId } : null), [ref, organizationId]);

if (!ref || !scope) {
return <InvalidRouteState backTo={'/(app)/pr-review' as Href} />;
}

if (ref.platform === 'github') {
return <Redirect href={providerPrRoutePath(ref)} />;
}

// One draft queue per PR/MR: the GitHub-shaped key the store already uses,
// suffixed with the s1 collision-free ref identity so a GitLab MR and a
// GitHub PR that share `owner/repo#number` — and one project reached on two
// GitLab instances — never share a queue.
const triple = providerPrTriple(ref);
const draftEntityKey = `${pendingReviewDraftKey(triple.owner, triple.repo, triple.number)}@${providerPrRefKey(ref)}`;

const sheetOptions = {
presentation: 'formSheet' as const,
sheetAllowedDetents: [0.5, fullSheetDetent] as [number, number],
sheetInitialDetentIndex: 'last' as const,
sheetGrabberVisible: true,
headerShown: false,
};

return (
<ProviderPrScopeProvider value={scope}>
<PendingReviewProvider
key={`${draftEntityKey}:${userId ?? ''}`}
userId={userId}
draftEntityKey={draftEntityKey}
>
{/* The provider-aware connect gate (s7): a disconnected reader can
never reach the authenticated queries and mutations below, and a
Bitbucket personal scope gets the terminal org-only explanation
instead of a retry that could not succeed. */}
<PrReviewConnectGate platform={ref.platform} organizationId={organizationId}>
<Stack screenLayout={appUnlockScreenLayout} screenOptions={{ headerShown: false }}>
{/* The three write sheets (s6) are siblings of the GitHub route's
sheets: they mount inside this layout, so they see the provider
scope and this PR's single `PendingReviewProvider` queue. */}
<Stack.Screen name="comment-composer" options={sheetOptions} />
<Stack.Screen name="review-submit" options={sheetOptions} />
<Stack.Screen name="merge" options={sheetOptions} />
<Stack.Screen name="file-navigator" options={sheetOptions} />
</Stack>
</PrReviewConnectGate>
</PendingReviewProvider>
</ProviderPrScopeProvider>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { PrReviewCommentComposerScreen } from '@/components/pr-review/pr-review-comment-composer-screen';

export default function ProviderPrReviewCommentComposerRoute() {
return <PrReviewCommentComposerScreen />;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { PrReviewFileNavigatorScreen } from '@/components/pr-review/pr-review-file-navigator-screen';

export default function ProviderPrReviewFileNavigatorRoute() {
return <PrReviewFileNavigatorScreen />;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { type Href, Stack, useLocalSearchParams } from 'expo-router';

import { InvalidRouteState } from '@/components/invalid-route-state';
import { PrReviewScreen } from '@/components/pr-review/pr-review-screen';
import { parseProviderPrRoute, providerPrTriple } from '@/lib/pr-review/provider-pr-ref';
import { parseParam } from '@/lib/route-params';

type Params = {
platform: string;
identity: string[];
instance?: string;
};

/**
* The provider PR/MR detail screen. The layout above already validated the
* route and published the scope, so the screen renders through the same tree
* GitHub uses; the triple it takes is the GitHub-shaped identity its stores
* are keyed on, while its queries follow the ref from the scope.
*/
export default function ProviderPrReviewIndexRoute() {
const params = useLocalSearchParams<Params>();
const ref = parseProviderPrRoute({
platform: parseParam(params.platform) ?? '',
identity: params.identity,
instance: params.instance,
});

if (!ref) {
return <InvalidRouteState backTo={'/(app)/pr-review' as Href} />;
}

const { owner, repo, number } = providerPrTriple(ref);

return (
<>
<Stack.Screen options={{ headerShown: false }} />
<PrReviewScreen owner={owner} repo={repo} number={number} />
</>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { PrReviewMergeScreen } from '@/components/pr-review/pr-review-merge-screen';

export default function ProviderPrReviewMergeRoute() {
return <PrReviewMergeScreen />;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { PrReviewReviewSubmitScreen } from '@/components/pr-review/pr-review-review-submit-screen';

export default function ProviderPrReviewSubmitRoute() {
return <PrReviewReviewSubmitScreen />;
}
25 changes: 20 additions & 5 deletions apps/mobile/src/components/agents/session-pr-badge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ describe('SessionPrBadge mounted', () => {
expect(mocks.openExternalUrl).not.toHaveBeenCalled();
});

it('opens the browser for a GitLab PR on press', async () => {
it('opens the in-app merge request route for a GitLab MR on press', async () => {
const renderer = await renderBadge({
pr: pr({
platform: 'gitlab',
Expand All @@ -255,11 +255,26 @@ describe('SessionPrBadge mounted', () => {
const pressable = findHost(renderer.root, 'Pressable')[0];
pressable?.props.onPress();

expect(mocks.openExternalUrl).toHaveBeenCalledWith(
'https://gitlab.com/octocat/hello-world/-/merge_requests/42',
{ label: 'pull request' }
expect(mocks.push).toHaveBeenCalledWith(
'/(app)/pr-review/gitlab/octocat/hello-world/42?instance=https%3A%2F%2Fgitlab.com'
);
expect(mocks.push).not.toHaveBeenCalled();
expect(mocks.openExternalUrl).not.toHaveBeenCalled();
});

it('opens the in-app pull request route for a Bitbucket PR on press', async () => {
const renderer = await renderBadge({
pr: pr({
platform: 'bitbucket',
url: 'https://bitbucket.org/acme/api/pull-requests/42',
}),
loading: false,
});

const pressable = findHost(renderer.root, 'Pressable')[0];
pressable?.props.onPress();

expect(mocks.push).toHaveBeenCalledWith('/(app)/pr-review/bitbucket/acme/api/42');
expect(mocks.openExternalUrl).not.toHaveBeenCalled();
});

it('opens the browser for a GitHub PR when the PR review flag is off', async () => {
Expand Down
1 change: 0 additions & 1 deletion apps/mobile/src/components/agents/session-pr-badge.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ export function SessionPrBadge(props: SessionPrBadgeProps) {
}
const target = resolveSessionPrTapTarget({
url: pr.url,
number: pr.number,
});
if (target.kind === 'in-app') {
router.push(target.href);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ export function useComposerInlineError(error: unknown, isEdit: boolean) {
})();
return;
}
const display = mutationErrorDisplay('composer', classification, error);
const display = mutationErrorDisplay('composer', classification, { rawError: error });
setInlineError(display.message);
setInlineErrorKind(display.kind);
setInlineErrorIsLocal(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { formatNumber } from '@/lib/format';
import { useIsTablet } from '@/lib/hooks/use-is-tablet';
import { useThemeColors } from '@/lib/hooks/use-theme-colors';
import { type DiffViewMode } from '@/lib/pr-review/diff/pr-diff-list-items';
import { providerPrChildRoutePath, useProviderPrScope } from '@/lib/pr-review/provider-pr-ref';
import { cn } from '@/lib/utils';

type PrDiffFileListHeaderProps = {
Expand All @@ -34,8 +35,6 @@ type PrDiffFileListHeaderProps = {
readonly onViewModeChange: (mode: DiffViewMode) => void;
};

const FILE_NAVIGATOR_PATH = '/(app)/pr-review/[owner]/[repo]/[number]/file-navigator' as const;

export function PrDiffFileListHeader({
owner,
repo,
Expand All @@ -51,10 +50,14 @@ export function PrDiffFileListHeader({
const colors = useThemeColors();
const { t } = useTranslation();

const navigatorHref = useMemo<Href>(
() => ({ pathname: FILE_NAVIGATOR_PATH, params: { owner, repo, number } }),
[owner, repo, number]
);
// The sheet is a sibling of the screen it was opened from, so its href is
// built from the live scope: a GitHub ref keeps the original
// `[owner]/[repo]/[number]/file-navigator` path, a GitLab or Bitbucket ref
// opens the sheet inside the provider layout — the only place its scope is
// published, and therefore the only place the sheet can query the right
// provider.
const { ref } = useProviderPrScope({ owner, repo, number });
const navigatorHref = useMemo<Href>(() => providerPrChildRoutePath(ref, 'file-navigator'), [ref]);

const handleOpenNavigator = useCallback(() => {
router.push(navigatorHref);
Expand Down
Loading
Loading