diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ff981d..76a67a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Organizations remember which source they import and mirror from; migration 0020 backfills the pin from each organization's repositories when they agree on one - Source picker in the add-organization dialog and on organization cards when more than one source is connected - `POST /api/sync/organization` persists `sourceId`, and `PATCH /api/organizations/:id` sets or clears it + - The bulk import only clears a pin it created itself in the same run when a second source lists the same organization; pins set through the picker or the add dialog are kept - Multi-source support: connect multiple source services per user (#375 follow-up) - Sources list on the Configuration page with add, edit and remove per source - Per-repository source attribution, with credentials resolved from each repository's own source diff --git a/src/lib/sources.test.ts b/src/lib/sources.test.ts index d4bef4f..2c601ee 100644 --- a/src/lib/sources.test.ts +++ b/src/lib/sources.test.ts @@ -4,6 +4,7 @@ import { deriveSourceName, findSourceForOrganization, findSourceForRepository, + selectSameRunPinsToClear, primarySource, resolveGitHubApiBaseUrl, selectRepositoriesToRelink, @@ -121,6 +122,32 @@ describe("findSourceForOrganization", () => { }); }); +describe("selectSameRunPinsToClear", () => { + const discovered = [{ normalizedName: "acme" }, { normalizedName: "tools" }]; + + test("clears a pin an earlier source in the same run created", () => { + const pinnedThisRun = new Map([["acme", "src-1"]]); + expect(selectSameRunPinsToClear(discovered, pinnedThisRun, "src-2")).toEqual(["acme"]); + }); + + test("leaves organizations the run has not pinned alone", () => { + // "tools" was pinned before the run (or never), so it is not in the map. + const pinnedThisRun = new Map([["other", "src-1"]]); + expect(selectSameRunPinsToClear(discovered, pinnedThisRun, "src-2")).toEqual([]); + }); + + test("keeps a pin the same source created", () => { + const pinnedThisRun = new Map([["acme", "src-2"]]); + expect(selectSameRunPinsToClear(discovered, pinnedThisRun, "src-2")).toEqual([]); + }); + + test("names each organization once", () => { + const pinnedThisRun = new Map([["acme", "src-1"]]); + const twice = [...discovered, { normalizedName: "acme" }]; + expect(selectSameRunPinsToClear(twice, pinnedThisRun, "src-2")).toEqual(["acme"]); + }); +}); + describe("selectRepositoriesToRelink", () => { const connected = new Set(["src-1"]); const base = { diff --git a/src/lib/sources.ts b/src/lib/sources.ts index ee6c4a3..3a33056 100644 --- a/src/lib/sources.ts +++ b/src/lib/sources.ts @@ -144,6 +144,29 @@ export type DanglingRepositoryRow = { updatedAt: Date | string | null; }; +/** + * Normalized names among `discovered` whose pin the current bulk import run + * created itself (`pinnedThisRun`, name -> source id) from a different source + * than `sourceId`. Such an organization spans several sources, the ambiguous + * case migration 0020 leaves unpinned, so the caller clears the pin and drops + * the name from the map. Pins that existed before the run (set through the + * picker or the add dialog) are a stored choice and never appear in the map. + */ +export function selectSameRunPinsToClear( + discovered: ReadonlyArray<{ normalizedName: string }>, + pinnedThisRun: ReadonlyMap, + sourceId: string +): string[] { + const names: string[] = []; + for (const org of discovered) { + const pinnedBy = pinnedThisRun.get(org.normalizedName); + if (pinnedBy !== undefined && pinnedBy !== sourceId && !names.includes(org.normalizedName)) { + names.push(org.normalizedName); + } + } + return names; +} + // Repositories whose sourceId no longer resolves (or was never set) but whose // provider and host match the given source: exactly the rows a deleted and // re-added source must take back. Without the re-link, the next import sees diff --git a/src/pages/api/sync/index.ts b/src/pages/api/sync/index.ts index 5ab26ca..5848592 100644 --- a/src/pages/api/sync/index.ts +++ b/src/pages/api/sync/index.ts @@ -40,7 +40,7 @@ export const POST: APIRoute = async ({ request, locals }) => { // is the primary source). A source without a token still lists public // repositories through its provider. Configs written outside the sources // API get their first source row seeded here. - const { listSources, ensureSourcesFromConfig } = await import("@/lib/sources"); + const { listSources, ensureSourcesFromConfig, selectSameRunPinsToClear } = await import("@/lib/sources"); const { createSourceProviderFromSource } = await import("@/lib/source-providers"); await ensureSourcesFromConfig(userId); const sources = (await listSources(userId)).filter((source) => source.enabled); @@ -71,6 +71,12 @@ export const POST: APIRoute = async ({ request, locals }) => { const failedOrgNames: string[] = []; const failedSourceNames: string[] = []; + // Organization pins this run set itself (inserted, or re-pinned on + // recovery), by normalized name. A later source listing the same name + // clears only these; pins that existed before the run are a stored + // choice and stay as they are. See selectSameRunPinsToClear. + const pinnedThisRun = new Map(); + for (const source of sources) { try { const sourceProvider = createSourceProviderFromSource(source, { userId }); @@ -135,6 +141,11 @@ export const POST: APIRoute = async ({ request, locals }) => { let insertedOrgs: typeof newOrgs = []; let insertedFailedOrgs: typeof failedOrgRecords = []; let recoveredOrgCount = 0; + // Names whose pin this source set or cleared; applied to pinnedThisRun + // only after the transaction commits, so a rolled-back source leaves + // the run's pin bookkeeping untouched. + let pinnedOrgNames: string[] = []; + let clearedOrgNames: string[] = []; // Transaction to insert only new items await db.transaction(async (tx) => { @@ -147,11 +158,7 @@ export const POST: APIRoute = async ({ request, locals }) => { .from(repositories) .where(eq(repositories.userId, userId)), tx - .select({ - normalizedName: organizations.normalizedName, - status: organizations.status, - sourceId: organizations.sourceId, - }) + .select({ normalizedName: organizations.normalizedName, status: organizations.status }) .from(organizations) .where(eq(organizations.userId, userId)), ]); @@ -160,9 +167,7 @@ export const POST: APIRoute = async ({ request, locals }) => { const existingRepoKeys = new Set( existingRepos.map((r) => `${r.sourceId ?? ""}|${r.normalizedFullName}`) ); - const existingOrgMap = new Map( - existingOrgs.map((o) => [o.normalizedName, { status: o.status, sourceId: o.sourceId }]) - ); + const existingOrgMap = new Map(existingOrgs.map((o) => [o.normalizedName, o.status])); insertedRepos = newRepos.filter( (r) => @@ -175,7 +180,7 @@ export const POST: APIRoute = async ({ request, locals }) => { // follow the recovering source, or the org mirror keeps scoping // to the source the failure was recorded under. const recoveredOrgs = newOrgs.filter( - (o) => existingOrgMap.get(o.normalizedName)?.status === "failed" + (o) => existingOrgMap.get(o.normalizedName) === "failed" ); for (const org of recoveredOrgs) { await tx @@ -198,29 +203,20 @@ export const POST: APIRoute = async ({ request, locals }) => { } recoveredOrgCount = recoveredOrgs.length; - // An org that a second source also lists must not stay pinned to - // the first one: the mirror and the card counts would drop the - // other source's repositories. Clearing the pin matches migration - // 0020, which leaves orgs spanning multiple sources unpinned. - // Failed orgs are excluded — their pin follows the recovering - // source in the update above. - const crossSourceOrgs = newOrgs.filter((o) => { - const existing = existingOrgMap.get(o.normalizedName); - return ( - existing !== undefined && - existing.status !== "failed" && - existing.sourceId != null && - existing.sourceId !== source.id - ); - }); - for (const org of crossSourceOrgs) { + // An organization that an earlier source in this run pinned and + // that this source lists as well spans several sources, so its pin + // is cleared, the same rule migration 0020 applies. Only pins this + // run created are touched: an org the user pinned on purpose keeps + // its pin even when another source lists the same name. + clearedOrgNames = selectSameRunPinsToClear(newOrgs, pinnedThisRun, source.id); + for (const normalizedName of clearedOrgNames) { await tx .update(organizations) .set({ sourceId: null, updatedAt: new Date() }) .where( and( eq(organizations.userId, userId), - eq(organizations.normalizedName, org.normalizedName), + eq(organizations.normalizedName, normalizedName), ) ); } @@ -228,7 +224,7 @@ export const POST: APIRoute = async ({ request, locals }) => { // Insert or update failed orgs (only update orgs already in "failed" state — don't overwrite good state) insertedFailedOrgs = failedOrgRecords.filter((o) => !existingOrgMap.has(o.normalizedName)); const stillFailedOrgs = failedOrgRecords.filter( - (o) => existingOrgMap.get(o.normalizedName)?.status === "failed" + (o) => existingOrgMap.get(o.normalizedName) === "failed" ); for (const org of stillFailedOrgs) { await tx @@ -270,8 +266,16 @@ export const POST: APIRoute = async ({ request, locals }) => { await tx.insert(organizations).values(batch); } } + pinnedOrgNames = [...recoveredOrgs, ...insertedOrgs].map((org) => org.normalizedName); }); + for (const normalizedName of pinnedOrgNames) { + pinnedThisRun.set(normalizedName, source.id); + } + for (const normalizedName of clearedOrgNames) { + pinnedThisRun.delete(normalizedName); + } + // Create mirror jobs only for newly inserted items const mirrorJobPromises = [ ...insertedRepos.map((repo) =>