83e01477df
* refactor: extract shared shapes between jira and linear integrations Factors out the duplicated logic between internal/jira + internal/linear (backend) and components/jira + components/linear (frontend) into reusable shared shapes. Follow-up to PR #736. Backend: - New internal/integrations/secretadapter wraps secrets.SecretStore with the upsert API both jira.SecretStore and linear.SecretStore expect; drops both adapter copies from cmd/kandev/services.go. - New internal/integrations/healthpoll owns the 90s auth-health loop via a Prober interface; linear/poller.go is a thin wrapper, jira/poller.go keeps only its (jira-specific) issue-watch loop. - provideServices split into initGitHubService / initJiraService / initLinearService — now 75 lines (was ~125, exceeded the 80-line limit). Frontend: - New hooks/domains/integrations/use-integration-availability.ts and use-integration-enabled.ts; jira/linear hooks reduce to one-line delegates and move from components/<x>/ to hooks/domains/<x>/ per the project convention Greptile flagged on PR #736. - New components/integrations/{auth-status-banner,auth-error-message, validated-popover}.tsx; the four link/import-bar wrappers drop from ~110 lines to ~35 each, and JiraLinkButton picks up the clear-error-on-close behaviour LinearLinkButton already had. Docs: - New 'Adding a new third-party integration' section in CLAUDE.md/AGENTS.md documenting the shared shapes and intentional Jira/Linear divergence (transition vs state, JQL vs filter object, eventBus only on jira). * fix: address PR #759 review feedback - validated-popover: outline-with-label trigger now sets type="button" and honours triggerDisabled, matching ghost-icon (Greptile, Claude, CodeRabbit). - jira-link-button / linear-link-button: move updateTask() into the awaited fetch path so a failed title update surfaces as the popover's inline error instead of an unhandled rejection (CodeRabbit). - use-integration-availability: monotonic requestId race guard prevents a slow earlier probe from overwriting a newer auth result (CodeRabbit). - use-integration-enabled: drop unnecessary async on synchronous init (Claude). - jira/linear poller smoke tests: wrap in testing/synctest instead of time.After(2s) per backend testing convention (CodeRabbit nitpick). Skipped with reasoning on the threads: - secretadapter Set TOCTOU — single-process SQLite + single-writer per workspace; constraint violation on conflict surfaces as an error to the caller, not silent corruption. - healthpoll SetInterval validation — the function isn't called from production code today; a panic on misuse would be a footgun and silent ignore would just hide bugs differently. * fix: address PR #759 round-2 review feedback - jira-settings / linear-settings: drop the local STATUS_REFRESH_MS = 90_000 copies and import the shared INTEGRATION_STATUS_REFRESH_MS so changing the cadence only touches one file (Claude). - healthpoll: rename Poller.ProbeAll → probeAll. The only callers are the loop and the in-package tests, so widening the public API wasn't buying anything (Claude). * fix: invalidate ValidatedPopover in-flight submits on close If the user clicked Submit and dismissed the popover before the request settled, the next time they re-opened it would inherit `loading=true` (the disabled-while-loading submit button was stuck) and a late rejection from the previous session could repopulate `error` in the new one. Track the submit via a `useRef` token, bump it on close, and ignore any result/rejection whose token no longer matches. Also reset `loading` and `error` on close so the next open starts clean. Adds a regression test covering both the late-resolution and late-rejection paths. Addresses CodeRabbit review on PR #759. * ci: trigger workflow re-run GitHub Actions did not dispatch the pull_request workflows on the previous two pushes (1d869f0e,0ab6c034); only pull_request_target and CodeQL fired. Empty commit to nudge synchronize event. * fix: address PR #759 round-3 review feedback - jira.Poller.Stop: add nil-receiver guard so callers using the defer-Stop-after-NewPoller pattern don't panic when initJiraService returned nil (Greptile + Claude). Matches linear.Poller's guard. - healthpoll TestStart_RunsImmediateProbe + TestStart_IsIdempotent: rewrap in testing/synctest with synctest.Wait() instead of time.After(2s) wall-clock waits. Drops the now-unused probedAll signal channel from fakeProber (Claude). --------- Co-authored-by: Kandev Agent <agent@kandev.dev>
131 lines
4.5 KiB
TypeScript
131 lines
4.5 KiB
TypeScript
import { describe, it, expect, vi, afterEach } from "vitest";
|
|
import { render, screen, fireEvent, act, waitFor, cleanup } from "@testing-library/react";
|
|
|
|
afterEach(() => {
|
|
cleanup();
|
|
});
|
|
|
|
// Stub the Radix-backed Popover/Tooltip primitives so children render inline
|
|
// (no portal, no measurement gymnastics) and the popover's open/close is
|
|
// driven by ValidatedPopover's own onOpenChange callback that we surface via
|
|
// a parent-controlled close button.
|
|
vi.mock("@kandev/ui/popover", () => {
|
|
return {
|
|
Popover: ({
|
|
open,
|
|
onOpenChange,
|
|
children,
|
|
}: {
|
|
open: boolean;
|
|
onOpenChange: (next: boolean) => void;
|
|
children: React.ReactNode;
|
|
}) => (
|
|
<div data-testid="popover" data-open={open}>
|
|
{children}
|
|
<button data-testid="popover-close" type="button" onClick={() => onOpenChange(false)}>
|
|
close
|
|
</button>
|
|
</div>
|
|
),
|
|
PopoverTrigger: ({ children }: { children: React.ReactNode }) => <>{children}</>,
|
|
PopoverContent: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
|
|
};
|
|
});
|
|
|
|
vi.mock("@kandev/ui/tooltip", () => ({
|
|
Tooltip: ({ children }: { children: React.ReactNode }) => <>{children}</>,
|
|
TooltipTrigger: ({ children }: { children: React.ReactNode }) => <>{children}</>,
|
|
TooltipContent: ({ children }: { children: React.ReactNode }) => <>{children}</>,
|
|
}));
|
|
|
|
import { ValidatedPopover } from "./validated-popover";
|
|
|
|
function deferred<T>() {
|
|
let resolve!: (v: T) => void;
|
|
let reject!: (e: unknown) => void;
|
|
const promise = new Promise<T>((res, rej) => {
|
|
resolve = res;
|
|
reject = rej;
|
|
});
|
|
return { promise, resolve, reject };
|
|
}
|
|
|
|
function renderHarness(overrides?: { fetch?: (key: string) => Promise<string> }) {
|
|
const onSuccess = vi.fn();
|
|
const result = render(
|
|
<ValidatedPopover
|
|
triggerStyle="ghost-icon"
|
|
triggerIcon={<span>icon</span>}
|
|
triggerAriaLabel="trigger"
|
|
tooltip="tip"
|
|
headline="Headline"
|
|
placeholder="paste here"
|
|
extractKey={(raw) => (raw.trim() ? raw.trim() : null)}
|
|
validationHint="hint"
|
|
fetch={overrides?.fetch ?? (async (k: string) => k)}
|
|
onSuccess={onSuccess}
|
|
submitLabel="Submit"
|
|
submittingLabel="Submitting..."
|
|
/>,
|
|
);
|
|
return { ...result, onSuccess };
|
|
}
|
|
|
|
describe("ValidatedPopover", () => {
|
|
it("close-while-loading resets loading state and ignores the late resolution", async () => {
|
|
const d = deferred<string>();
|
|
const fetchSpy = vi.fn(async () => d.promise);
|
|
const { onSuccess } = renderHarness({ fetch: fetchSpy });
|
|
|
|
// Type a key + submit
|
|
fireEvent.change(screen.getByPlaceholderText("paste here"), { target: { value: "KEY-1" } });
|
|
fireEvent.click(screen.getByRole("button", { name: "Submit" }));
|
|
|
|
// Mid-flight: submitting label visible, button disabled.
|
|
await waitFor(() => {
|
|
const submitting = screen.getByRole("button", { name: "Submitting..." }) as HTMLButtonElement;
|
|
expect(submitting.disabled).toBe(true);
|
|
});
|
|
|
|
// User dismisses the popover before fetch resolves.
|
|
fireEvent.click(screen.getByTestId("popover-close"));
|
|
|
|
// Loading must reset on close so the next open starts clean.
|
|
await waitFor(() => {
|
|
expect(screen.queryByRole("button", { name: "Submitting..." })).toBeNull();
|
|
});
|
|
|
|
// Late resolution must not call onSuccess (the submission token was
|
|
// invalidated when we closed) and must not bring the popover back.
|
|
await act(async () => {
|
|
d.resolve("late");
|
|
await Promise.resolve();
|
|
});
|
|
expect(onSuccess).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("close-while-loading suppresses a late rejection rather than re-populating error", async () => {
|
|
const d = deferred<string>();
|
|
const fetchSpy = vi.fn(async () => d.promise);
|
|
renderHarness({ fetch: fetchSpy });
|
|
|
|
fireEvent.change(screen.getByPlaceholderText("paste here"), { target: { value: "KEY-1" } });
|
|
fireEvent.click(screen.getByRole("button", { name: "Submit" }));
|
|
await waitFor(() => {
|
|
const submitting = screen.getByRole("button", { name: "Submitting..." }) as HTMLButtonElement;
|
|
expect(submitting.disabled).toBe(true);
|
|
});
|
|
|
|
fireEvent.click(screen.getByTestId("popover-close"));
|
|
|
|
// Reject after close — the in-flight error must not leak into UI state.
|
|
await act(async () => {
|
|
d.reject(new Error("boom"));
|
|
await Promise.resolve().catch(() => {});
|
|
});
|
|
|
|
// role=alert is the inline error <p> in ValidatedPopover; should be absent.
|
|
expect(screen.queryByRole("alert")).toBeNull();
|
|
});
|
|
});
|