diff --git a/srv/developer-service.js b/srv/developer-service.js index b4d242ce5..1c2006113 100644 --- a/srv/developer-service.js +++ b/srv/developer-service.js @@ -14,7 +14,7 @@ import { maybeAutoSendCompletion } from './lib/ngds-autosend.js'; import { stampSubmissionId } from './lib/task-record-submission-id.js'; import { rollUpParentsForCompletion } from './lib/completion-rollup.js'; import { renderMarkdown } from './lib/markdown.js'; -import { buildTaskMap, effectiveLegacyId, expandGroupTutorials, buildPathItems, buildPathSegments, fetchGroupTitles, groupChildLegacyIds } from './lib/path-item-task-resolver.js'; +import { buildTaskMap, effectiveLegacyId, expandGroupTutorials, buildPathItems, buildPathSegments, fetchGroupTitles, groupChildLegacyIds, buildRecordMap } from './lib/path-item-task-resolver.js'; import { annotateUnlocked } from './lib/app-space-segments.js'; import { handleRequestAccountMerge, handleConfirmAccountMerge } from './lib/account-merge-request.js'; @@ -600,8 +600,11 @@ export default class DeveloperService extends cds.ApplicationService { taskLegacyId: { in: recordLegacyIds } }); } - const recordMap = new Map(); - for (const r of userRecords) recordMap.set(`${r.taskType}:${r.taskLegacyId}`, r); + // #600: a user can hold a COMPLETED row + SUPERSEDED rows for the same + // task; buildRecordMap keeps the most-done one deterministically (prefer + // COMPLETED) instead of the old unordered last-write-wins, which could + // surface a SUPERSEDED row and render a done task as incomplete. + const recordMap = buildRecordMap(userRecords); // Prefer the mission-side link (Missions.event_ID) the admin UI actually // sets; fall back to the forward Events.mission_ID lookup for legacy data. @@ -711,8 +714,11 @@ export default class DeveloperService extends cds.ApplicationService { taskLegacyId: { in: recordLegacyIds } }); } - const recordMap = new Map(); - for (const r of userRecords) recordMap.set(`${r.taskType}:${r.taskLegacyId}`, r); + // #600: a user can hold a COMPLETED row + SUPERSEDED rows for the same + // task; buildRecordMap keeps the most-done one deterministically (prefer + // COMPLETED) instead of the old unordered last-write-wins, which could + // surface a SUPERSEDED row and render a done task as incomplete. + const recordMap = buildRecordMap(userRecords); return { eventId: event.legacyId, diff --git a/srv/lib/path-item-task-resolver.js b/srv/lib/path-item-task-resolver.js index 443c384fe..b43c2e44f 100644 --- a/srv/lib/path-item-task-resolver.js +++ b/srv/lib/path-item-task-resolver.js @@ -281,6 +281,48 @@ export function groupChildLegacyIds(groupTutorials) { return [...ids]; } +/** + * Collapse a user's TaskRecords to one record per `${taskType}:${taskLegacyId}`, + * keyed the way buildPathItems/shapeItem look them up. + * + * A user can hold MULTIPLE records for the same task — a current COMPLETED row + * plus one or more SUPERSEDED rows from prior completions (issue #600 "has-ever- + * completed"), and occasionally IN_PROGRESS. The old code did a plain + * `for (r of userRecords) map.set(key, r)` — last-write-wins over an UNORDERED + * query, so which row survived was nondeterministic. When a SUPERSEDED (or + * in-progress) row happened to land last, the path item rendered as not-complete + * even though the user had a COMPLETED record — e.g. app-space showing "2 of 4" + * ABAP tutorials done when all four were COMPLETED in-window (reported + * 2026-10-26). Prefer the most "done" record deterministically instead. + * + * Preference (highest wins): COMPLETED > SUPERSEDED > anything else; ties broken + * by latest completionDate. This picks the record that proves the task is done; + * the #2866 in-window downgrade (effectiveStatus) still applies downstream on + * whichever record we keep. + * + * @param {Array} userRecords TaskRecords rows (taskType, taskLegacyId, status, completionDate) + * @returns {Map} `${taskType}:${taskLegacyId}` → best record + */ +export function buildRecordMap(userRecords) { + const STATUS_RANK = { COMPLETED: 3, SUPERSEDED: 2 }; + const rankOf = (r) => STATUS_RANK[r?.status] ?? 1; + const map = new Map(); + for (const r of userRecords) { + const key = `${r.taskType}:${r.taskLegacyId}`; + const cur = map.get(key); + if (!cur) { map.set(key, r); continue; } + const rr = rankOf(r), rc = rankOf(cur); + if (rr > rc) { map.set(key, r); continue; } + if (rr === rc) { + // tie-break: latest completionDate wins (null sorts earliest) + const dr = r?.completionDate ? Date.parse(r.completionDate) : -Infinity; + const dc = cur?.completionDate ? Date.parse(cur.completionDate) : -Infinity; + if (dr > dc) map.set(key, r); + } + } + return map; +} + /** * Batch-fetch the titles for Groups referenced by path items (for fork lane * labels). Returns an empty Map when there are no GROUP items. diff --git a/test/lib/build-record-map.test.js b/test/lib/build-record-map.test.js new file mode 100644 index 000000000..98a8f1de7 --- /dev/null +++ b/test/lib/build-record-map.test.js @@ -0,0 +1,48 @@ +import { describe, it, expect } from 'vitest'; +import { buildRecordMap } from '../../srv/lib/path-item-task-resolver.js'; + +describe('buildRecordMap', () => { + it('prefers a COMPLETED record over SUPERSEDED for the same task, regardless of order', () => { + // Order reversed from "natural" so last-write-wins would pick SUPERSEDED. + const records = [ + { taskType: 'TUTORIAL', taskLegacyId: 10000027, status: 'COMPLETED', completionDate: '2026-10-26T21:08:00Z' }, + { taskType: 'TUTORIAL', taskLegacyId: 10000027, status: 'SUPERSEDED', completionDate: '2026-10-26T21:03:00Z' }, + ]; + const map = buildRecordMap(records); + expect(map.get('TUTORIAL:10000027').status).toBe('COMPLETED'); + + // And with COMPLETED first (so plain last-write-wins would have dropped it): + const map2 = buildRecordMap([records[1], records[0]]); + expect(map2.get('TUTORIAL:10000027').status).toBe('COMPLETED'); + }); + + it('breaks status ties by latest completionDate', () => { + const records = [ + { taskType: 'TUTORIAL', taskLegacyId: 5, status: 'SUPERSEDED', completionDate: '2023-01-01T00:00:00Z' }, + { taskType: 'TUTORIAL', taskLegacyId: 5, status: 'SUPERSEDED', completionDate: '2026-09-23T00:00:00Z' }, + ]; + expect(buildRecordMap(records).get('TUTORIAL:5').completionDate).toBe('2026-09-23T00:00:00Z'); + }); + + it('keeps separate entries per (taskType, taskLegacyId)', () => { + const records = [ + { taskType: 'TUTORIAL', taskLegacyId: 1, status: 'COMPLETED', completionDate: '2026-01-01T00:00:00Z' }, + { taskType: 'GROUP', taskLegacyId: 1, status: 'COMPLETED', completionDate: '2026-01-01T00:00:00Z' }, + { taskType: 'TUTORIAL', taskLegacyId: 2, status: 'IN_PROGRESS' }, + ]; + const map = buildRecordMap(records); + expect(map.size).toBe(3); + expect(map.get('TUTORIAL:1').taskType).toBe('TUTORIAL'); + expect(map.get('GROUP:1').taskType).toBe('GROUP'); + }); + + it('ranks COMPLETED > SUPERSEDED > other (e.g. IN_PROGRESS)', () => { + const records = [ + { taskType: 'TUTORIAL', taskLegacyId: 9, status: 'IN_PROGRESS' }, + { taskType: 'TUTORIAL', taskLegacyId: 9, status: 'COMPLETED', completionDate: '2026-10-26T21:00:00Z' }, + { taskType: 'TUTORIAL', taskLegacyId: 9, status: 'SUPERSEDED', completionDate: '2026-10-27T21:00:00Z' }, + ]; + // SUPERSEDED has a later date, but COMPLETED outranks it on status. + expect(buildRecordMap(records).get('TUTORIAL:9').status).toBe('COMPLETED'); + }); +});