diff --git a/server/services/projectAssignment.js b/server/services/projectAssignment.js index fdcb247..ed1557a 100644 --- a/server/services/projectAssignment.js +++ b/server/services/projectAssignment.js @@ -1,11 +1,21 @@ import { config } from '../config.js' import { moveRepoToOrg, WARN } from './repoMove.js' +import { splitRepo } from './giteaClient.js' import { listRecentPushes } from './pushHistory.js' import { createGitPushWorksheet, isSystemPusher } from './gitWorksheet.js' import { withLocks } from './keyedLock.js' import { ensureProjectTicket } from './ticketAdmin.js' import { getDocument, updateDocument } from './appwriteAdmin.js' +/** + * Warnungen dieses Moduls, woertlich (Controller-Ruling R14, Fix-Runde 1) - Anfuehrungszeichen + * wie die bestehenden Warnungen in repoMove.js (‚...‘ mit U+201A/U+2018). + */ +const WARN_BACKFILL_FAILED = (reason) => + `Pushes konnten nicht nachgetragen werden (${reason}). Bitte später ‚Pushes nachtragen‘ klicken.` +const WARN_RECONCILE_FAILED = (reason) => + `Zuweisung gespeichert, aber Umzug und Nachtragen sind fehlgeschlagen (${reason}). Bitte später ‚Pushes nachtragen‘ klicken.` + /** * Commit-Nachrichten (erste Zeile), die frueher das System mit Kensos Schluessel geschrieben hat * (Preview-Config, automatisch erstellte Preview-Seite) - siehe Plan, Abschnitt "Global @@ -42,65 +52,132 @@ export async function backfillPushes(project, { now = new Date() } = {}) { try { pushes = await listRecentPushes(project.repoFullName, { now }) } catch (err) { - return { backfilled: 0, warnings: [WARN.giteaDown(err.message)] } + return { backfilled: 0, warnings: [WARN.giteaDown(err.message)], ticketId: project.ticketId || '' } } const relevant = pushes.filter((push) => !isSystemPusher(push.pusher) && !isOldSystemCommitPush(push)) - if (relevant.length === 0) return { backfilled: 0, warnings: [] } - - const ticketId = project.ticketId || (await ensureProjectTicket(project, { repoFullName: project.repoFullName })) - if (!ticketId) return { backfilled: 0, warnings: [] } + if (relevant.length === 0) return { backfilled: 0, warnings: [], ticketId: project.ticketId || '' } + // `ensureProjectTicket` und die Arbeitsblatt-Schleife sind hier bewusst in EINEM try/catch + // (Controller-Ruling R14): schlaegt eine der beiden Appwrite-Schreiboperationen fehl (z. B. + // Ausfall waehrend POST workorders/worksheets), bleibt die Zuweisung trotzdem bestehen - siehe + // `assignProject`, das sich darauf verlaesst, dass `backfillPushes` nie wirft. + let ticketId = project.ticketId || '' let backfilled = 0 - // Aelteste zuerst (siehe `listRecentPushes`), damit die Arbeitsblaetter in derselben - // Reihenfolge entstehen, in der die Pushes tatsaechlich passiert sind. - for (const push of relevant) { - const result = await createGitPushWorksheet({ ticketId, repoFullName: project.repoFullName, ...push }) - if (result?.created) backfilled += 1 + try { + ticketId = ticketId || (await ensureProjectTicket(project, { repoFullName: project.repoFullName })) + if (!ticketId) return { backfilled: 0, warnings: [], ticketId: '' } + + // Aelteste zuerst (siehe `listRecentPushes`), damit die Arbeitsblaetter in derselben + // Reihenfolge entstehen, in der die Pushes tatsaechlich passiert sind. + for (const push of relevant) { + const result = await createGitPushWorksheet({ ticketId, repoFullName: project.repoFullName, ...push }) + if (result?.created) backfilled += 1 + } + return { backfilled, warnings: [], ticketId } + } catch (err) { + return { backfilled, warnings: [WARN_BACKFILL_FAILED(err.message)], ticketId } } - return { backfilled, warnings: [] } } /** + * Kern von `reconcileProject`, OHNE selbst die Sperre zu nehmen - fuer `assignProject`, das + * seine eigene Lese-Pruef-Schreib-Folge und diesen Kern unter DERSELBEN Sperre laufen laesst + * (Controller-Ruling R15), statt die Sperre zweimal zu nehmen. `reconcileProject` (unten) bleibt + * der oeffentliche, selbst sperrende Einstieg (z. B. fuer die Migration, Task 9). + * * Liest das Projekt, verschiebt sein Repo bei Bedarf nach `GITEA_ORG` und traegt danach dessen - * Pushes nach - alles unter der Projekt-Sperre, damit ein Doppelklick auf "Zuweisen" (oder ein - * gleichzeitiger Aufruf ueber die Migration, Task 9) nicht zweimal umzieht oder doppelte - * Arbeitsblaetter erzeugt (Review Focus 3, Plan). Das Projektdokument wird ERST innerhalb der - * Sperre gelesen, damit ein zweiter, gleichzeitiger Aufruf nie mit einem inzwischen veralteten - * Objekt arbeitet. + * Pushes nach. Wirft nur noch, wenn schon das ERSTE Lesen des Projekts fehlschlaegt (dann gibt es + * nichts Sinnvolles zu verschieben/nachzutragen) - `assignProject` faengt das ab (R14). */ -export async function reconcileProject(projectId, { now } = {}) { - return withLocks(['project:' + projectId], async () => { - let project = await getDocument(config.collections.websiteProjects, projectId) - const { moved, warnings: moveWarnings } = await moveRepoToOrg(project) +async function reconcileProjectLocked(projectId, { now } = {}) { + let project = await getDocument(config.collections.websiteProjects, projectId) + const { moved, warnings: moveWarnings } = await moveRepoToOrg(project) + try { // Immer neu lesen, nicht nur wenn `moved` gesetzt ist: der Selbstheilungs-Zweig in // `moveRepoToOrg` (von Hand verschobenes/umbenanntes Repo) kann das Dokument umbenennen und // dabei trotzdem `moved: null` liefern (Controller-Ruling R13). project = await getDocument(config.collections.websiteProjects, projectId) + } catch { + // R14: Schlaegt das Nachlesen fehl, mit dem VOR dem Umzug gelesenen Dokument weitermachen - + // ist tatsaechlich etwas verschoben worden, von Hand dessen `repoFullName`/`giteaRepoUrl`/ + // `giteaRepoName` nachziehen (siehe `updateProjectDocument` in repoMove.js). Keine zusaetzliche + // Warnung - `moveWarnings` deckt einen etwaigen Gitea-Ausfall bereits ab. + if (moved) { + project = { + ...project, + repoFullName: moved.to, + giteaRepoUrl: `${config.gitea.baseUrl}/${moved.to}`, + giteaRepoName: splitRepo(moved.to)[1], + } + } + } - const { backfilled, warnings: backfillWarnings } = await backfillPushes(project, { now }) + const { backfilled, warnings: backfillWarnings, ticketId } = await backfillPushes(project, { now }) + // R15: die Antwort soll die (evtl. gerade erst automatisch angelegte) ticketId zeigen, auch + // wenn `project` oben vor dem Anlegen gelesen wurde. + if (ticketId && project.ticketId !== ticketId) project = { ...project, ticketId } - return { project, moved, backfilled, warnings: [...moveWarnings, ...backfillWarnings] } - }) + return { project, moved, backfilled, warnings: [...moveWarnings, ...backfillWarnings] } } /** - * Weist ein Projekt einem Kunden/Ticket zu und stoesst danach Umzug + Nachtragen an. Die - * Zuweisung selbst wird ZUERST und ausserhalb der Projekt-Sperre gespeichert (reines Appwrite, - * ohne Gitea) - sie bleibt deshalb bestehen, selbst wenn Gitea gerade nicht erreichbar ist - * (siehe `reconcileProject`/`moveRepoToOrg`, die deshalb nie wegen Gitea werfen duerfen). + * Oeffentlicher, selbst sperrender Einstieg fuer den Kern oben - damit ein Doppelklick auf + * "Zuweisen" (oder ein gleichzeitiger Aufruf ueber die Migration, Task 9) nicht zweimal umzieht + * oder doppelte Arbeitsblaetter erzeugt (Review Focus 3, Plan). + */ +export async function reconcileProject(projectId, { now } = {}) { + return withLocks(['project:' + projectId], () => reconcileProjectLocked(projectId, { now })) +} + +/** + * Ticket-Id fuer die Zuweisung (Controller-Ruling R15): eine mitgegebene `ticketId` gewinnt immer. + * Ist sie leer UND bleibt die Kunden-Id gleich, bleibt die gespeicherte `ticketId` erhalten - ein + * Entfernen laeuft ueber "Zuweisung aufheben", nicht ueber diesen Endpunkt (sonst loescht ein + * zweiter Klick ohne `ticketId` ein zwischenzeitlich automatisch angelegtes Ticket, Review I3). + * Aendert sich die Kunden-Id und wird keine `ticketId` mitgegeben, wird sie geleert. + */ +function resolveTicketId(payloadTicketId, customerId, existing) { + if (payloadTicketId) return payloadTicketId + if (customerId === existing.customerId) return existing.ticketId || '' + return '' +} + +/** + * Weist ein Projekt einem Kunden/Ticket zu und stoesst danach Umzug + Nachtragen an. Laeuft + * komplett unter der Projekt-Sperre (Controller-Ruling R15): Lesen (fuer die 404-Pruefung und die + * `ticketId`-Aufloesung), Schreiben der Zuweisung und der Reconcile-Kern teilen sich dieselbe + * Sperre, statt sie zweimal zu nehmen - ein zweiter, gleichzeitiger Aufruf sieht daher nie einen + * veralteten Stand. Ein unbekanntes Projekt wirft weiterhin (`.status = 404`) BEVOR irgendetwas + * geschrieben wird. Schlaegt danach der Reconcile-Kern (Umzug/Nachtragen) mit etwas anderem als + * einem 404 fehl, bleibt die Zuweisung trotzdem gespeichert - Ruling R14 verlangt eine Warnung + * statt eines Abbruchs, denn Spec 5.2 verlangt genau das. */ export async function assignProject(projectId, { customerId = '', ticketId = '' }, { now } = {}) { - // 404 fuer ein unbekanntes Projekt, bevor ueberhaupt geschrieben wird. - await getDocument(config.collections.websiteProjects, projectId) + return withLocks(['project:' + projectId], async () => { + // 404 fuer ein unbekanntes Projekt, bevor ueberhaupt geschrieben wird. + const existing = await getDocument(config.collections.websiteProjects, projectId) - await updateDocument(config.collections.websiteProjects, projectId, { - customerId, - ticketId, - updatedAt: new Date().toISOString(), + const nextTicketId = resolveTicketId(ticketId, customerId, existing) + const saved = await updateDocument(config.collections.websiteProjects, projectId, { + customerId, + ticketId: nextTicketId, + updatedAt: new Date().toISOString(), + }) + + try { + const result = await reconcileProjectLocked(projectId, { now }) + return { ok: true, ...result } + } catch (err) { + if (err.status === 404) throw err + return { + ok: true, + project: saved, + moved: null, + backfilled: 0, + warnings: [WARN_RECONCILE_FAILED(err.message)], + } + } }) - - const { project, moved, backfilled, warnings } = await reconcileProject(projectId, { now }) - return { ok: true, project, moved, backfilled, warnings } } diff --git a/test/projectAssignment.test.js b/test/projectAssignment.test.js index 148baa3..00ea4c2 100644 --- a/test/projectAssignment.test.js +++ b/test/projectAssignment.test.js @@ -43,6 +43,22 @@ function pushEntry({ sha, at, user, msg }) { } } +/** + * Wartet, bis `aw.calls` mindestens `count` Eintraege hat - deterministisch, kein fester Timeout. + * `calls.push` passiert server-seitig, SOBALD eine Anfrage beim Fake-Appwrite ankommt - also + * strikt BEVOR die Antwort beim Aufrufer eintrifft und dieser (falls ueberhaupt) die naechste + * Anfrage lostritt. Damit laesst sich `failNext` gezielt auf einen bestimmten von mehreren gleich + * aussehenden Aufrufen ansetzen (z. B. das zweite GET desselben Dokuments), ohne zu raten. + */ +function waitForAppwriteCalls(count) { + return new Promise((resolve) => { + ;(function check() { + if (aw.calls.length >= count) return resolve() + setImmediate(check) + })() + }) +} + before(async () => { gitea = await startFakeGitea({ repos: [ @@ -50,9 +66,15 @@ before(async () => { { full_name: 'JUSN/leer' }, { full_name: 'JUSN/nur-kunde' }, { full_name: 'JUSN/doppelt' }, + { full_name: 'WEBklar/ticket-fail' }, + { full_name: 'WEBklar/worksheet-fail' }, + { full_name: 'JUSN/reread-fail' }, + { full_name: 'WEBklar/inlock-fail' }, ], feeds: { // Newest first, wie der echte Gitea-Feed. + 'WEBklar/ticket-fail': [pushEntry({ sha: 'tf1', user: 'JUSN', at: '2026-09-27T10:00:00Z', msg: 'Ticket-Fail-Push' })], + 'WEBklar/worksheet-fail': [pushEntry({ sha: 'wf1', user: 'JUSN', at: '2026-09-27T10:00:00Z', msg: 'Worksheet-Fail-Push' })], 'JUSN/D-ner-Nation': [ pushEntry({ sha: 's1', user: 'webklar-system', at: '2026-09-27T14:50:00Z', msg: 'System-Push' }), pushEntry({ sha: 'k1', user: 'knso', at: '2026-09-27T14:48:00Z', msg: 'Update preview config from ticket system' }), @@ -73,6 +95,10 @@ before(async () => { { $id: 'p2', repoFullName: 'JUSN/leer', projectName: 'leer', customerId: '', ticketId: '' }, { $id: 'p3', repoFullName: 'JUSN/nur-kunde', projectName: 'nur-kunde', customerId: '', ticketId: '' }, { $id: 'p4', repoFullName: 'JUSN/doppelt', projectName: 'doppelt', customerId: '', ticketId: '' }, + { $id: 'p5', repoFullName: 'WEBklar/ticket-fail', projectName: 'ticket-fail', customerId: '', ticketId: '' }, + { $id: 'p6', repoFullName: 'WEBklar/worksheet-fail', projectName: 'worksheet-fail', customerId: '', ticketId: '' }, + { $id: 'p7', repoFullName: 'JUSN/reread-fail', projectName: 'reread-fail', customerId: '', ticketId: '' }, + { $id: 'p8', repoFullName: 'WEBklar/inlock-fail', projectName: 'inlock-fail', customerId: '', ticketId: '' }, ], workorders: [{ $id: 't1', woid: '100115' }], customers: [{ $id: 'c1', name: 'Döner Nation' }], @@ -123,9 +149,34 @@ test('nur Kunde ohne Pushes: kein Ticket', async () => { assert.equal(aw.all('workorders').length, before) }) test('nur Kunde mit Pushes: Webpage-Ticket entsteht', async () => { - const r = await assignProject('p3', { customerId: 'c1' }, { now }) - const t = aw.all('workorders').find((w) => w.$id === aw.get('websiteProjects', 'p3').ticketId) + // ticketAdmin loggt die automatische Ticket-Anlage (console.log) - hier abfangen, damit die + // Testausgabe sauber bleibt (Ruling, Fix-Runde 1). + const originalLog = console.log + console.log = () => {} + let r + try { + r = await assignProject('p3', { customerId: 'c1' }, { now }) + } finally { + console.log = originalLog + } + const stored = aw.get('websiteProjects', 'p3') + const t = aw.all('workorders').find((w) => w.$id === stored.ticketId) assert.equal(t.type, 'Webpage'); assert.equal(r.backfilled, 1) + // I2: die Antwort muss die gerade erst automatisch angelegte ticketId zeigen, nicht den Stand + // von vor dem Anlegen. + assert.equal(r.project.ticketId, stored.ticketId) +}) +test('Kunde erneut zuweisen ohne Ticket: kein zweites Ticket, kein doppeltes Arbeitsblatt', async () => { + // p3 hat aus dem vorigen Test bereits ein automatisch angelegtes Ticket - ein verzoegerter + // zweiter Klick (gleicher Kunde, keine ticketId) darf es nicht durch ein leeres ueberschreiben + // (Review I3). "Verzoegert" reicht hier als einfaches Abwarten des ersten Aufrufs. + const before = { workorders: aw.all('workorders').length, worksheets: aw.all('worksheets').length } + const ticketIdBefore = aw.get('websiteProjects', 'p3').ticketId + const r = await assignProject('p3', { customerId: 'c1' }, { now }) + assert.equal(aw.all('workorders').length, before.workorders) + assert.equal(aw.all('worksheets').length, before.worksheets) + assert.equal(aw.get('websiteProjects', 'p3').ticketId, ticketIdBefore) + assert.equal(r.project.ticketId, ticketIdBefore) }) test('Doppelklick: ein Umzug, keine doppelten Arbeitsblätter', async () => { await Promise.all([assignProject('p4', { customerId: 'c1', ticketId: 't1' }, { now }), assignProject('p4', { customerId: 'c1', ticketId: 't1' }, { now })]) @@ -155,3 +206,48 @@ test('Gitea-Ausfall: Zuweisung bleibt', async () => { test('unbekanntes Projekt', async () => { await assert.rejects(assignProject('nope', { ticketId: 't1' }, { now }), (e) => e.status === 404) }) +test('Appwrite-Ausfall beim Ticket-Anlegen: Zuweisung bleibt, Warnung statt Abbruch', async () => { + aw.failNext({ method: 'POST', pathIncludes: 'workorders', status: 500 }) + const r = await assignProject('p5', { customerId: 'c1' }, { now }) + assert.equal(r.ok, true) + assert.equal(aw.get('websiteProjects', 'p5').customerId, 'c1') + assert.match(r.warnings[0], /^Pushes konnten nicht nachgetragen werden \(/) +}) +test('Appwrite-Ausfall beim Arbeitsblatt-Anlegen: Zuweisung bleibt, Warnung statt Abbruch', async () => { + aw.failNext({ method: 'POST', pathIncludes: 'worksheets', status: 500 }) + const r = await assignProject('p6', { customerId: 'c1', ticketId: 't1' }, { now }) + assert.equal(r.ok, true) + assert.equal(aw.get('websiteProjects', 'p6').ticketId, 't1') + assert.match(r.warnings[0], /^Pushes konnten nicht nachgetragen werden \(/) +}) +test('Nachlesen nach Umzug schlaegt fehl: Antwort zeigt trotzdem den neuen Namen', async () => { + // p7 zieht hier tatsaechlich um (JUSN -> WEBklar); das GET danach (Controller-Ruling R13/R14) + // soll scheitern. Gezielt getimt: warten, bis das GET VOR dem Umzug und das PATCH AUS dem + // Umzug (`updateProjectDocument` in repoMove.js) durch sind, dann erst failNext scharf machen - + // so trifft es sicher das GET NACH dem Umzug und keines der beiden vorherigen. + const target = aw.calls.length + 2 + const p = reconcileProject('p7', { now }) + await waitForAppwriteCalls(target) + aw.failNext({ method: 'GET', pathIncludes: 'p7', status: 500 }) + const r = await p + assert.deepEqual(r.moved, { from: 'JUSN/reread-fail', to: 'WEBklar/reread-fail' }) + assert.deepEqual(r.warnings, []) + assert.equal(r.project.repoFullName, 'WEBklar/reread-fail') + assert.equal(r.project.giteaRepoUrl, `${gitea.baseUrl}/WEBklar/reread-fail`) + assert.equal(r.project.giteaRepoName, 'reread-fail') +}) +test('Appwrite-Fehler beim Nachlesen unter der Sperre in assignProject: Zuweisung bleibt, Warnung statt Abbruch', async () => { + // Gezielt getimt wie oben: warten, bis die Existenzpruefung und das Speichern der Zuweisung + // (beide in `assignProject`) durch sind, dann erst failNext scharf machen - so trifft es sicher + // das GET, das der Reconcile-Kern (`reconcileProjectLocked`) direkt danach selbst macht. + const target = aw.calls.length + 2 + const p = assignProject('p8', { customerId: 'c1' }, { now }) + await waitForAppwriteCalls(target) + aw.failNext({ method: 'GET', pathIncludes: 'p8', status: 500 }) + const r = await p + assert.equal(r.ok, true) + assert.equal(r.moved, null) + assert.equal(r.backfilled, 0) + assert.equal(aw.get('websiteProjects', 'p8').customerId, 'c1') + assert.match(r.warnings[0], /^Zuweisung gespeichert, aber Umzug und Nachtragen sind fehlgeschlagen \(/) +})