-
Notifications
You must be signed in to change notification settings - Fork 960
feat(desktop): add customizable PR link provider setting #2333
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export { PR_LINK_PROVIDER_LABELS } from "./pr-link-provider"; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| import type { PrLinkProvider } from "@superset/local-db"; | ||
|
|
||
| export const PR_LINK_PROVIDER_LABELS: Record<PrLinkProvider, string> = { | ||
| github: "GitHub", | ||
| betterhub: "BetterHub", | ||
| devin: "Devin", | ||
| custom: "Custom", | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,8 @@ | ||
| import { cn } from "@superset/ui/utils"; | ||
| import { LuCircleDot, LuGitMerge, LuGitPullRequest } from "react-icons/lu"; | ||
| import { electronTrpc } from "renderer/lib/electron-trpc"; | ||
| import { transformPrUrl } from "renderer/utils/pr-url"; | ||
| import { DEFAULT_PR_LINK_PROVIDER } from "shared/constants"; | ||
| import { STROKE_WIDTH } from "../constants"; | ||
|
|
||
| type PRState = "open" | "merged" | "closed" | "draft"; | ||
|
|
@@ -19,6 +21,10 @@ export function WorkspaceStatusBadge({ | |
| className, | ||
| }: WorkspaceStatusBadgeProps) { | ||
| const openUrl = electronTrpc.external.openUrl.useMutation(); | ||
| const { data: prLinkSettings } = | ||
| electronTrpc.settings.getPrLinkProvider.useQuery(); | ||
| const prProvider = prLinkSettings?.provider ?? DEFAULT_PR_LINK_PROVIDER; | ||
| const prCustomDomain = prLinkSettings?.customDomain; | ||
|
Comment on lines
+24
to
+27
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Router and persistence paths for PR link settings:"
rg -n -C3 '\bgetPrLinkProvider\b|\bsetPrLinkProvider\b|pr_link_provider|pr_link_custom_domain' apps/desktop/src
echo
echo "All query consumers:"
rg -n -C3 'getPrLinkProvider\.useQuery\(' apps/desktop/src
echo
echo "Any prefetch/initial-data/hydration for this query:"
rg -n -C3 'initialData|placeholderData|prefetch|prefetchQuery|ensureQueryData|hydrate' apps/desktop/srcRepository: superset-sh/superset Length of output: 48122 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== tRPC client configuration and QueryClient setup ==="
fd -e ts -e tsx 'providers|ElectronTRPC|QueryClient' apps/desktop/src/renderer --type f | head -20 | xargs grep -l 'QueryClient\|prefetch\|defaultOptions' 2>/dev/null | head -5
echo
echo "=== ElectronTRPCProvider setup (checking for default behavior) ==="
cat apps/desktop/src/renderer/providers/ElectronTRPCProvider/ElectronTRPCProvider.tsx | head -100
echo
echo "=== Root layout or app initialization (checking for prefetch on startup) ==="
rg -n 'getPrLinkProvider' apps/desktop/src/renderer/routes --type ts --type tsx
echo
echo "=== Check if settings query is called during app initialization ==="
rg -n -B3 -A3 'useEffect|useLayoutEffect' apps/desktop/src/renderer/providers --type ts --type tsx | grep -A5 -B5 'getPrLink\|settings' || echo "No settings query in providers"Repository: superset-sh/superset Length of output: 1328 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== Root layout files ==="
find apps/desktop/src/renderer -name 'layout.*' -o -name 'root.*' -o -name '_app.*' -o -name 'page.tsx' | grep -E '(root|_app|layout)' | head -10
echo
echo "=== Check main app entry point ==="
cat apps/desktop/src/renderer/routes/_app.tsx 2>/dev/null | head -150 || cat apps/desktop/src/renderer/routes/layout.tsx 2>/dev/null | head -150 || echo "No main layout found at common paths"
echo
echo "=== Check if getPrLinkProvider query is prefetched anywhere in loaders ==="
rg 'getPrLinkProvider' apps/desktop/src/renderer/routes -A2 -B2
echo
echo "=== Check the specific component tree for WorkspaceStatusBadge ==="
cat apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceStatusBadge.tsx | head -100Repository: superset-sh/superset Length of output: 5166 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== Full WorkspaceStatusBadge component to verify click gating ==="
cat apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceStatusBadge.tsx
echo
echo "=== Check other components using getPrLinkProvider for comparison ==="
echo "--- PRButton.tsx ---"
cat apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ChangesHeader/components/PRButton/PRButton.tsx | grep -A20 'getPrLinkProvider' | head -30
echo
echo "--- WorkspaceHoverCard.tsx ---"
cat apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/components/WorkspaceHoverCard/WorkspaceHoverCard.tsx | grep -A20 'getPrLinkProvider' | head -30Repository: superset-sh/superset Length of output: 4134 Gate the click handler until the settings query is resolved. On the initial render, This pattern affects multiple components using 🤖 Prompt for AI Agents |
||
| const iconClass = "w-3 h-3"; | ||
|
|
||
| const config = { | ||
|
|
@@ -69,7 +75,7 @@ export function WorkspaceStatusBadge({ | |
| const handleClick = (e: React.MouseEvent) => { | ||
| if (prUrl) { | ||
| e.stopPropagation(); | ||
| openUrl.mutate(prUrl); | ||
| openUrl.mutate(transformPrUrl(prUrl, prProvider, prCustomDomain)); | ||
| } | ||
| }; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,8 +9,11 @@ import { | |
| LuTriangleAlert, | ||
| } from "react-icons/lu"; | ||
| import { electronTrpc } from "renderer/lib/electron-trpc"; | ||
| import { PR_LINK_PROVIDER_LABELS } from "renderer/routes/_authenticated/settings/utils/pr-link-provider"; | ||
| import { usePRStatus } from "renderer/screens/main/hooks"; | ||
| import { useHotkeyDisplay } from "renderer/stores/hotkeys"; | ||
| import { transformPrUrl } from "renderer/utils/pr-url"; | ||
| import { DEFAULT_PR_LINK_PROVIDER } from "shared/constants"; | ||
| import { STROKE_WIDTH } from "../../../constants"; | ||
| import { ChecksList } from "./components/ChecksList"; | ||
| import { ChecksSummary } from "./components/ChecksSummary"; | ||
|
|
@@ -40,6 +43,11 @@ export function WorkspaceHoverCardContent({ | |
| isLoading: isLoadingGithub, | ||
| } = usePRStatus({ workspaceId }); | ||
|
|
||
| const { data: prLinkSettings } = | ||
| electronTrpc.settings.getPrLinkProvider.useQuery(); | ||
| const prProvider = prLinkSettings?.provider ?? DEFAULT_PR_LINK_PROVIDER; | ||
| const prCustomDomain = prLinkSettings?.customDomain; | ||
|
Comment on lines
+46
to
+49
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: cat -n "apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/components/WorkspaceHoverCard/WorkspaceHoverCard.tsx"Repository: superset-sh/superset Length of output: 7968 The PR link can temporarily default to GitHub while settings are loading, allowing users to click through to the wrong provider. Lines 46-47 don't capture the loading state from 🤖 Prompt for AI Agents |
||
|
|
||
| const openPRDisplay = useHotkeyDisplay("OPEN_PR"); | ||
| const hasOpenPRShortcut = !( | ||
| openPRDisplay.length === 1 && openPRDisplay[0] === "Unassigned" | ||
|
|
@@ -168,9 +176,17 @@ export function WorkspaceHoverCardContent({ | |
| className="w-full mt-1 h-7 text-xs gap-1.5" | ||
| asChild | ||
| > | ||
| <a href={pr.url} target="_blank" rel="noopener noreferrer"> | ||
| <FaGithub className="size-3" /> | ||
| View on GitHub | ||
| <a | ||
| href={transformPrUrl(pr.url, prProvider, prCustomDomain)} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| > | ||
| {prProvider === "github" ? ( | ||
| <FaGithub className="size-3" /> | ||
| ) : ( | ||
| <LuExternalLink className="size-3" strokeWidth={STROKE_WIDTH} /> | ||
| )} | ||
| View on {PR_LINK_PROVIDER_LABELS[prProvider]} | ||
| {hasOpenPRShortcut && ( | ||
| <KbdGroup className="ml-auto"> | ||
| {openPRDisplay.map((key) => ( | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Add
noopener,noreferrerwhen opening PR links from the hotkey to prevent opener access from external pages.Prompt for AI agents