FN-8115: consolidate GitHub import settings reads
Reuse request-scoped project settings across GitHub issue import operations. - Read project settings once for single and batch imports - Share settings with translation and GitHub tracking resolution - Cover settings reuse and translated/original batch content Files changed: .../dashboard/src/__tests__/routes-github.test.ts | 105 ++++++++++++++++++++- .../dashboard/src/routes/register-git-github.ts | 56 ++++++++--- 2 files changed, 145 insertions(+), 16 deletions(-) Fusion-Task-Id: FN-8115 Fusion-Task-Lineage: acc046c5-d599-4df4-b380-a1e167d45548 Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
@@ -44,11 +44,20 @@ const mockCentralListProjects = vi.fn().mockResolvedValue([]);
|
||||
const mockCentralInit = vi.fn().mockResolvedValue(undefined);
|
||||
const mockCentralClose = vi.fn().mockResolvedValue(undefined);
|
||||
const mockCentralReconcileProjectStatuses = vi.fn().mockResolvedValue(undefined);
|
||||
const { mockPerformUpdateCheck, mockClearUpdateCheckCache, mockExecSync, mockExecFile } = vi.hoisted(() => ({
|
||||
const {
|
||||
mockPerformUpdateCheck,
|
||||
mockClearUpdateCheckCache,
|
||||
mockExecSync,
|
||||
mockExecFile,
|
||||
mockGetCachedImportTranslation,
|
||||
mockResolveTargetLocale,
|
||||
} = vi.hoisted(() => ({
|
||||
mockPerformUpdateCheck: vi.fn(),
|
||||
mockClearUpdateCheckCache: vi.fn(),
|
||||
mockExecSync: vi.fn(),
|
||||
mockExecFile: vi.fn(),
|
||||
mockGetCachedImportTranslation: vi.fn(),
|
||||
mockResolveTargetLocale: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("../update-check.js", async () => {
|
||||
@@ -60,6 +69,19 @@ vi.mock("../update-check.js", async () => {
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock("../import-translate-service.js", async () => {
|
||||
const actual = await vi.importActual<typeof import("../import-translate-service.js")>(
|
||||
"../import-translate-service.js",
|
||||
);
|
||||
mockGetCachedImportTranslation.mockImplementation(actual.getCachedImportTranslation);
|
||||
mockResolveTargetLocale.mockImplementation(actual.resolveTargetLocale);
|
||||
return {
|
||||
...actual,
|
||||
getCachedImportTranslation: mockGetCachedImportTranslation,
|
||||
resolveTargetLocale: mockResolveTargetLocale,
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock("node:child_process", async () => {
|
||||
const actual = await vi.importActual<typeof import("node:child_process")>("node:child_process");
|
||||
mockExecSync.mockImplementation(((...args: Parameters<typeof actual.execSync>) => actual.execSync(...args)) as typeof actual.execSync);
|
||||
@@ -565,6 +587,8 @@ describe("POST /github/issues/import", () => {
|
||||
let getIssueDetailSpy: ReturnType<typeof vi.fn>;
|
||||
|
||||
beforeEach(() => {
|
||||
mockGetCachedImportTranslation.mockClear();
|
||||
mockResolveTargetLocale.mockClear();
|
||||
mockIsGhAuthenticated.mockReturnValue(true);
|
||||
getIssueSpy = vi.fn();
|
||||
vi.spyOn(GitHubClient.prototype, "getIssue").mockImplementation(getIssueSpy);
|
||||
@@ -635,6 +659,38 @@ describe("POST /github/issues/import", () => {
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:GitHubImportTranslate 2026-07-16-11:22:
|
||||
FN-8115 requires a single project-settings read per single import even though translation and tracking both depend on it. The route test protects the request-scoped sharing invariant rather than the resolver internals.
|
||||
*/
|
||||
it("reads project settings once for a single issue import", async () => {
|
||||
getIssueSpy.mockResolvedValueOnce(mockGitHubIssue);
|
||||
|
||||
const res = await REQUEST(buildApp(), "POST", "/api/github/issues/import", JSON.stringify({ owner: "owner", repo: "repo", issueNumber: 1 }), {
|
||||
"Content-Type": "application/json",
|
||||
});
|
||||
|
||||
expect(res.status).toBe(201);
|
||||
expect(store.getSettings).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("uses cached translated prose for an open single issue", async () => {
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({ githubImportAutoTranslate: true });
|
||||
mockResolveTargetLocale.mockReturnValueOnce("en");
|
||||
mockGetCachedImportTranslation.mockResolvedValueOnce({ title: "Translated title", body: "Translated body" });
|
||||
getIssueSpy.mockResolvedValueOnce({ ...mockGitHubIssue, title: "Original title", body: "Original body" });
|
||||
|
||||
const res = await REQUEST(buildApp(), "POST", "/api/github/issues/import", JSON.stringify({ owner: "owner", repo: "repo", issueNumber: 1 }), {
|
||||
"Content-Type": "application/json",
|
||||
});
|
||||
|
||||
expect(res.status).toBe(201);
|
||||
expect(store.createTask).toHaveBeenCalledWith(expect.objectContaining({
|
||||
title: "Translated title",
|
||||
description: "Translated body\n\nSource: https://github.com/owner/repo/issues/1",
|
||||
}));
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:GitHubImportAttachments 2026-07-15-11:20:
|
||||
Route-level proof that the import path itself downloads issue screenshots into task attachments — the executor's `## Attachments` section and triage's vision blocks only fire for attachments that actually exist on the task, so the wiring (not just the helper) is the invariant.
|
||||
@@ -946,6 +1002,8 @@ describe("POST /github/issues/batch-import", () => {
|
||||
|
||||
beforeEach(() => {
|
||||
__resetBatchImportRateLimiter();
|
||||
mockGetCachedImportTranslation.mockClear();
|
||||
mockResolveTargetLocale.mockClear();
|
||||
|
||||
fetchSpy = vi.fn();
|
||||
globalThis.fetch = fetchSpy as any;
|
||||
@@ -1112,6 +1170,7 @@ describe("POST /github/issues/batch-import", () => {
|
||||
expect(res.body.results.every((r: { success: boolean }) => r.success)).toBe(true);
|
||||
expect(throttledSpy).toHaveBeenCalledTimes(3);
|
||||
expect(store.createTask).toHaveBeenCalledTimes(3);
|
||||
expect(store.getSettings).toHaveBeenCalledTimes(1);
|
||||
expect(store.createTask).toHaveBeenNthCalledWith(1, expect.objectContaining({
|
||||
sourceIssue: {
|
||||
provider: "github",
|
||||
@@ -1141,6 +1200,50 @@ describe("POST /github/issues/batch-import", () => {
|
||||
}));
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:GitHubImportTranslate 2026-07-16-11:22:
|
||||
Batch imports reuse one request-scoped settings object for every item. Cached translations still apply to open issues, while closed issues always retain original prose even when the cache has a hit.
|
||||
*/
|
||||
it("preserves open translations and closed original prose in a batch", async () => {
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({ githubImportAutoTranslate: true });
|
||||
mockResolveTargetLocale.mockReturnValueOnce("en").mockReturnValueOnce("en");
|
||||
mockGetCachedImportTranslation.mockResolvedValueOnce({ title: "Translated batch title", body: "Translated batch body" });
|
||||
vi.spyOn(GitHubClient.prototype, "fetchThrottled")
|
||||
.mockResolvedValueOnce({
|
||||
success: true,
|
||||
data: { ...mockGitHubIssue(1, "Original open title"), body: "Original open body", state: "open" },
|
||||
} as Awaited<ReturnType<GitHubClient["fetchThrottled"]>>)
|
||||
.mockResolvedValueOnce({
|
||||
success: true,
|
||||
data: { ...mockGitHubIssue(2, "Original closed title"), body: "Original closed body", state: "closed" },
|
||||
} as Awaited<ReturnType<GitHubClient["fetchThrottled"]>>);
|
||||
|
||||
const res = await REQUEST(
|
||||
buildApp(),
|
||||
"POST",
|
||||
"/api/github/issues/batch-import",
|
||||
JSON.stringify({ owner: "owner", repo: "repo", issueNumbers: [1, 2], delayMs: 1 }),
|
||||
{ "Content-Type": "application/json" },
|
||||
);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(store.getSettings).toHaveBeenCalledTimes(1);
|
||||
expect(store.createTask).toHaveBeenNthCalledWith(1, expect.objectContaining({
|
||||
title: "Translated batch title",
|
||||
description: "Translated batch body\n\nSource: https://github.com/owner/repo/issues/1",
|
||||
}));
|
||||
expect(store.createTask).toHaveBeenNthCalledWith(2, expect.objectContaining({
|
||||
title: "Original closed title",
|
||||
description: "Original closed body\n\nSource: https://github.com/owner/repo/issues/2",
|
||||
}));
|
||||
expect(mockGetCachedImportTranslation).toHaveBeenCalledTimes(2);
|
||||
expect(mockGetCachedImportTranslation).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
expect.anything(),
|
||||
expect.objectContaining({ state: "closed" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("marks batch imported issues as tracked when global tracking defaults are on", async () => {
|
||||
const globalSettingsStore = { getSettings: vi.fn().mockResolvedValue({ githubTrackingDefaultEnabledForNewTasks: true }) };
|
||||
(store.getGlobalSettingsStore as ReturnType<typeof vi.fn>).mockReturnValueOnce(globalSettingsStore);
|
||||
|
||||
@@ -2101,10 +2101,14 @@ async function resolveImportedIssueTranslation(
|
||||
owner: string,
|
||||
repo: string,
|
||||
issue: { number: number; title: string; body: string | null; state: "open" | "closed" },
|
||||
projectSettings: Awaited<ReturnType<TaskStore["getSettings"]>>,
|
||||
): Promise<{ title: string; body: string } | null> {
|
||||
try {
|
||||
const settings = await store.getSettings();
|
||||
if (settings.githubImportAutoTranslate !== true) return null;
|
||||
/*
|
||||
FNXC:GitHubImportTranslate 2026-07-16-11:22:
|
||||
FN-8115 passes request-scoped project settings from both import routes so translation and tracking share one project-settings read. FN-8112 first made the prior two-read test setup stable; this consolidation preserves its behavior without a second store lookup.
|
||||
*/
|
||||
if (projectSettings.githubImportAutoTranslate !== true) return null;
|
||||
|
||||
const { getCachedImportTranslation, resolveTargetLocale } = await import(
|
||||
"../import-translate-service.js"
|
||||
@@ -2115,10 +2119,10 @@ async function resolveImportedIssueTranslation(
|
||||
Resolution therefore falls through to the global `language` setting server-side, which also fixes direct API callers and stale clients; the request locale stays as the last tier because `language` is itself unset when a surface browser-detects its locale.
|
||||
*/
|
||||
const targetLocale = resolveTargetLocale(
|
||||
settings.importTranslateTargetLocale,
|
||||
projectSettings.importTranslateTargetLocale,
|
||||
// The panel forwards its active locale; a direct API caller may not.
|
||||
(req.body as { targetLocale?: unknown } | undefined)?.targetLocale,
|
||||
settings.language,
|
||||
projectSettings.language,
|
||||
);
|
||||
if (!targetLocale) return null;
|
||||
|
||||
@@ -2132,8 +2136,14 @@ async function resolveImportedIssueTranslation(
|
||||
}
|
||||
}
|
||||
|
||||
async function resolveImportedIssueGithubTracking(store: TaskStore): Promise<{ enabled: true } | undefined> {
|
||||
const projectSettings = await store.getSettings();
|
||||
async function resolveImportedIssueGithubTracking(
|
||||
store: TaskStore,
|
||||
projectSettings: Awaited<ReturnType<TaskStore["getSettings"]>>,
|
||||
): Promise<{ enabled: true } | undefined> {
|
||||
/*
|
||||
FNXC:GithubImportTracking 2026-07-16-11:22:
|
||||
FN-8115 shares the import request's project settings with translation and tracking, removing duplicate project-store reads after FN-8112 stabilized the prior test setup. The global settings read remains distinct because tracking precedence still requires it.
|
||||
*/
|
||||
if (projectSettings.githubLinkImportedIssuesToTracking === true) {
|
||||
/*
|
||||
FNXC:GithubImportTracking 2026-07-01-00:00:
|
||||
@@ -4086,12 +4096,20 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void {
|
||||
Cache-read only: a miss imports the original rather than blocking the import on a fresh model call, because import must stay fast and must never fail because translation failed.
|
||||
The `Source: <url>` suffix is appended AFTER translation so the URL is never rewritten by the model.
|
||||
*/
|
||||
const translatedIssue = await resolveImportedIssueTranslation(req, scopedStore, owner, repo, issue);
|
||||
const projectSettings = await scopedStore.getSettings();
|
||||
const translatedIssue = await resolveImportedIssueTranslation(
|
||||
req,
|
||||
scopedStore,
|
||||
owner,
|
||||
repo,
|
||||
issue,
|
||||
projectSettings,
|
||||
);
|
||||
const title = (translatedIssue?.title || issue.title).slice(0, 200);
|
||||
const body = (translatedIssue?.body ?? issue.body)?.trim() || "(no description)";
|
||||
const description = `${body}\n\nSource: ${sourceUrl}`;
|
||||
|
||||
const importedIssueGithubTracking = await resolveImportedIssueGithubTracking(scopedStore);
|
||||
const importedIssueGithubTracking = await resolveImportedIssueGithubTracking(scopedStore, projectSettings);
|
||||
const source = buildGitHubIssueSource(owner, repo, issue);
|
||||
/*
|
||||
FNXC:Workflows 2026-07-05-00:00:
|
||||
@@ -4313,7 +4331,8 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void {
|
||||
|
||||
// Get existing tasks to check for duplicates
|
||||
const existingTasks = await scopedStore.listTasks({ slim: false, includeArchived: false });
|
||||
const importedIssueGithubTracking = await resolveImportedIssueGithubTracking(scopedStore);
|
||||
const projectSettings = await scopedStore.getSettings();
|
||||
const importedIssueGithubTracking = await resolveImportedIssueGithubTracking(scopedStore, projectSettings);
|
||||
|
||||
// Process issues sequentially with throttling
|
||||
const results: Array<{
|
||||
@@ -4386,12 +4405,19 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void {
|
||||
FNXC:GitHubImportTranslate 2026-07-15-09:30:
|
||||
Batch import carries translations exactly like single import (shared helper) — the requirement is about imported issues, not about which import button was used.
|
||||
*/
|
||||
const batchTranslation = await resolveImportedIssueTranslation(req, scopedStore, owner, repo, {
|
||||
number: issue.number,
|
||||
title: issue.title,
|
||||
body: issue.body,
|
||||
state: issue.state === "closed" ? "closed" : "open",
|
||||
});
|
||||
const batchTranslation = await resolveImportedIssueTranslation(
|
||||
req,
|
||||
scopedStore,
|
||||
owner,
|
||||
repo,
|
||||
{
|
||||
number: issue.number,
|
||||
title: issue.title,
|
||||
body: issue.body,
|
||||
state: issue.state === "closed" ? "closed" : "open",
|
||||
},
|
||||
projectSettings,
|
||||
);
|
||||
const title = (batchTranslation?.title || issue.title).slice(0, 200);
|
||||
const body = (batchTranslation?.body ?? issue.body)?.trim() || "(no description)";
|
||||
const description = `${body}\n\nSource: ${sourceUrl}`;
|
||||
|
||||
Reference in New Issue
Block a user