Raised by @kaseywright during review of #281. Not a regression from that PR — it predates it — but it has no home yet, so filing it here rather than leaving it in a review thread.
The inconsistency
Three code paths resolve "which chapter is this?" and they disagree about whether bible_id is part of the key.
Scoped by bible — chapter-assignments.repository.ts, the rollup query's bible_texts join:
.innerJoin(
bible_texts,
and(
eq(bible_texts.bibleId, chapter_assignments.bibleId),
eq(bible_texts.bookId, chapter_assignments.bookId),
eq(bible_texts.chapterNumber, chapter_assignments.chapterNumber)
)
)Not scoped by bible — chapter-assignments.repository.ts, findForVerse, which matches an assignment on the unit plus book and chapter only:
.where(
and(
eq(chapter_assignments.projectUnitId, projectUnitId),
eq(chapter_assignments.bookId, bibleText.bookId),
eq(chapter_assignments.chapterNumber, bibleText.chapterNumber)
)
)Also not scoped by bible — verse-audio.repository.ts, listByChapter, which reaches bible_texts through the recording's own bibleTextId and filters on book and chapter alone.
Why it matters
A project unit whose chapter_assignments.bible_id differs from the bible behind its bible_texts rows resolves fine through findForVerse (so the authorization and verse-audio paths accept it) but drops out of the rollup, because the rollup's join requires the two to agree. Same underlying row, two different answers about whether the chapter exists.
Today this is latent — it needs a unit where the two diverge, which the current ingestion flow does not normally produce. It becomes reachable the moment a project can source text from more than one bible for the same book, or a unit's bible is reassigned after assignments exist.
What needs deciding
Which behaviour is correct, then make all three agree:
bible_id is part of the chapter key — add the predicate to findForVerse and listByChapter, and decide what the API should do for a unit whose assignment and texts disagree (probably surface it rather than silently return nothing).bible_id is not part of the chapter key — drop it from the rollup join and let a unit's texts stand on their own.
Worth a look at whether a DB constraint can make the divergent state unrepresentable instead, which would be a better outcome than three queries independently remembering to agree.
Raised by @kaseywright during review of #281. Not a regression from that PR — it predates it — but it has no home yet, so filing it here rather than leaving it in a review thread.
The inconsistency
Three code paths resolve "which chapter is this?" and they disagree about whether
bible_idis part of the key.Scoped by bible —
chapter-assignments.repository.ts, the rollup query'sbible_textsjoin:Not scoped by bible —
chapter-assignments.repository.ts,findForVerse, which matches an assignment on the unit plus book and chapter only:Also not scoped by bible —
verse-audio.repository.ts,listByChapter, which reachesbible_textsthrough the recording's ownbibleTextIdand filters on book and chapter alone.Why it matters
A project unit whose
chapter_assignments.bible_iddiffers from the bible behind itsbible_textsrows resolves fine throughfindForVerse(so the authorization and verse-audio paths accept it) but drops out of the rollup, because the rollup's join requires the two to agree. Same underlying row, two different answers about whether the chapter exists.Today this is latent — it needs a unit where the two diverge, which the current ingestion flow does not normally produce. It becomes reachable the moment a project can source text from more than one bible for the same book, or a unit's bible is reassigned after assignments exist.
What needs deciding
Which behaviour is correct, then make all three agree:
bible_idis part of the chapter key — add the predicate tofindForVerseandlistByChapter, and decide what the API should do for a unit whose assignment and texts disagree (probably surface it rather than silently return nothing).bible_idis not part of the chapter key — drop it from the rollup join and let a unit's texts stand on their own.Worth a look at whether a DB constraint can make the divergent state unrepresentable instead, which would be a better outcome than three queries independently remembering to agree.