SF-3746 Allow navigating to any drafted book if it exists

[RaymondLuong3]

This feature allows a user to navigate to any drafted book on the project. It adds a layer to estimate the number of chapters expected for each book based on the english versifications. This will be improve further in the future to allow navigating to any chapter that exists in a draft/resource in view.

Missing book

Chapter does not exist

---

---

This change is 

[devin-ai-integration]

Devin Review found 4 potential issues.

View 6 additional findings in Devin Review.

[devin-ai-integration]

🟡 Incorrect keys '5ES' and '6ES' in chapterCounts should be '5EZ' and '6EZ'

The new chapterCounts table uses keys '5ES' and '6ES' for 5 Ezra and 6 Ezra, but the canonical book IDs used throughout the codebase (in verseCounts at progress.service.ts:102-103, and in all localization files like checking_en.json:155-156) are '5EZ' and '6EZ'. When expectedBookChapters is called with Canon.bookNumberToId(bookNum) for these books, it will look up '5EZ'/'6EZ' in chapterCounts, find no match, and fall back to 1 instead of the correct value 2.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[codecov]

Codecov Report

❌ Patch coverage is 82.85714% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.32%. Comparing base (aadcc7d) to head (9085851).
⚠️ Report is 1 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...late/editor/editor-draft/editor-draft.component.ts	0.00%	4 Missing ⚠️
...rc/app/shared/progress-service/progress.service.ts	50.00%	0 Missing and 1 partial ⚠️
...re/ClientApp/src/app/shared/text/text.component.ts	92.85%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3740      +/-   ##
==========================================
- Coverage   81.32%   81.32%   -0.01%     
==========================================
  Files         620      620              
  Lines       39086    39100      +14     
  Branches     6375     6378       +3     
==========================================
+ Hits        31787    31798      +11     
+ Misses       6329     6317      -12     
- Partials      970      985      +15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[pmachapman]

Just some minor fixes!

@pmachapman reviewed 7 files and all commit messages, and made 5 comments.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on RaymondLuong3).

---

src/SIL.XForge.Scripture/ClientApp/src/app/shared/progress-service/progress.service.ts line 100 at r2 (raw file):

  DNT: 424,
  BLT: 42,
  '3ES': 944,

While you are updating this file, would you please be able to fix this value to:

'3ES': 434,

(3ES = 1ES, not 2ES as I mistakenly thought when I wrote this)

Code quote:

'3ES': 944,

---

src/SIL.XForge.Scripture/ClientApp/src/app/shared/progress-service/progress.service.ts line 212 at r2 (raw file):

  DNT: 12,
  BLT: 1,
  DAG: 14,

Can you please add the following values (based on vul.vrs):

BLT: 1,
'3ES': 9,
EZA: 12,
'5EZ': 2,
'6EZ': 2,
DAG: 14,

(this will fix the Devin warning below)

Code quote:

  BLT: 1,
  DAG: 14,

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.ts line 819 at r2 (raw file):

              // Reload config if the checksum has been reset on the server
              if (this.projectUserConfigDoc.data.selectedSegmentChecksum == null) {
                await this.loadProjectUserConfig(bookNum);

I think devin may be right under certain edge cases? Should this be:

await this.loadProjectUserConfig(this._bookNum);

Code quote:

await this.loadProjectUserConfig(bookNum);

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.ts line 843 at r2 (raw file):

        const allChapters: number = Math.max(
          this.text?.chapters[this.text.chapters.length - 1].number ?? 1,

Just in case the chapters array is empty, this should be:

this.text?.chapters[this.text.chapters.length - 1]?.number ?? 1,

Code quote:

this.text?.chapters[this.text.chapters.length - 1].number ?? 1,

[RaymondLuong3]

@RaymondLuong3 made 5 comments and resolved 2 discussions.
Reviewable status: 5 of 7 files reviewed, 5 unresolved discussions (waiting on pmachapman).

---

src/SIL.XForge.Scripture/ClientApp/src/app/shared/progress-service/progress.service.ts line 100 at r2 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

While you are updating this file, would you please be able to fix this value to:

'3ES': 434,

(3ES = 1ES, not 2ES as I mistakenly thought when I wrote this)

Done.

---

src/SIL.XForge.Scripture/ClientApp/src/app/shared/progress-service/progress.service.ts line 212 at r2 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

Can you please add the following values (based on vul.vrs):

BLT: 1,
'3ES': 9,
EZA: 12,
'5EZ': 2,
'6EZ': 2,
DAG: 14,

(this will fix the Devin warning below)

Done.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.ts line 819 at r2 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

I think devin may be right under certain edge cases? Should this be:

await this.loadProjectUserConfig(this._bookNum);

Done.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.ts line 843 at r2 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

Just in case the chapters array is empty, this should be:

this.text?.chapters[this.text.chapters.length - 1]?.number ?? 1,

Done.

[pmachapman]

@pmachapman reviewed 2 files and all commit messages, made 1 comment, and resolved 5 discussions.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on RaymondLuong3).

[pmachapman]

@pmachapman reviewed 3 files and all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on RaymondLuong3).

[devin-ai-integration]

🚩 loadingStarted() may not have a corresponding loadingFinished() for draft-only books

At editor.component.ts:782, loadingStarted() is called. For draft-only books where this.text == null, changeText() early-returns at line 1787-1790 without the flow reaching any code that emits the loaded event from the text component or calls loadingFinished(). The loaded.emit() added at text.component.ts:1170 handles the case where projectHasText() returns false, but when target.id is set to undefined, bindQuill() returns early at text.component.ts:1165 without emitting loaded. This could leave the DataLoadingComponent's loading indicator active indefinitely for draft-only books. The new test at line 344-383 only checks bookName and chapters.length without verifying the loading state.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🚩 hasSource getter returns false for draft-only books for non-Paratext users, hiding source panel

The hasSource getter at editor.component.ts:659-672 returns false when this.text == null && !this.isParatextUserRole. This means for non-Paratext-role users navigating to a draft-only book, the source panel is hidden, even though the source project likely has content for that book (since it's in the drafting range). This may be an unintended UX regression — the user navigated to a draft-only book expecting to see the draft, but both source and target panels show no content. The hasSource logic may need updating to consider this._bookNum and draft configuration rather than relying solely on this.text.

(Refers to lines 659-672)

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🚩 Removed tests for navigation guards that no longer exist

Two tests were removed from editor.component.spec.ts: 'navigates to alternate chapter if specified chapter does not exist' and 'should navigate to projects route if url book is not in project'. These tests validated behavior that no longer applies — the PR intentionally allows navigating to books/chapters that don't exist in the project texts (for draft viewing). The removal is consistent with the intent, but the tests should be replaced with new tests verifying the correct behavior when navigating to non-existent books/chapters (beyond the single new test at line 344).

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🚩 The chapters array is now generated from 1 to max, which may include non-existent chapters

At editor.component.ts:846-850, this.chapters is now generated as a contiguous range [1, 2, ..., allChapters] using Math.max between the project's actual last chapter and the expected chapter count from expectedBookChapters(). Previously, this.chapters was derived from this.text.chapters.map(c => c.number), which only included chapters that actually exist in the project. The new approach means that if a project has chapters 1 and 3 but not 2, the chapters array will now include chapter 2. This is a behavioral change — the user could navigate to a chapter that doesn't exist in the project. This appears intentional for the draft navigation feature (allowing navigation to chapters that may have drafts), but it represents a significant change in how chapters are enumerated.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🚩 editor-draft returns empty ops array for unloaded text docs

At editor-draft.component.ts:437, when textDoc.isLoaded is false, the observable returns of([] as DeltaOperation[]). This empty array flows into the targetOps$ stream. Downstream code at editor-draft.component.ts:356 uses this.hasContent(this.targetDelta?.ops) to determine if there's existing content to warn about overwriting. An empty array means hasContent returns false, so the overwrite warning won't be shown for unloaded docs. This is probably the desired behavior — if the text doc doesn't exist (e.g., a chapter only available as a draft), there's nothing to overwrite. However, if the doc exists but is temporarily unloaded (e.g., offline), this could silently allow overwriting without warning.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🚩 this.text being undefined has cascading effects on several getters

With the removal of the redirect when a book isn't in the project (editor.component.ts:836 old lines removed), this.text can now be undefined for draft-only books. Several getters use this.text and will behave differently:

• hasChapterEditPermission at line 513 calls hasChapterEditPermissionForText(this.text, this.chapter) which returns undefined when text is undefined (meaning permission is unknown)
• hasEditRight at line 501 checks hasChapterEditPermission === true, so it returns false when text is undefined — this is probably fine for draft-only books
• setSegment() at line 1836 returns early when this.text == null
• onTargetUpdated at line 988 checks this.text != null before writing config

These all seem to degrade gracefully, but this represents a semantic shift where this.text being undefined is now a normal operational state rather than an error state.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

Devin Review found 1 new potential issue.

View 4 additional findings in Devin Review.

[devin-ai-integration]

🚩 Draft-only books show 'error' placeholder in target editor

When a user navigates to a book that only exists in draftedScriptureRange (not in the project's texts array), this.text is undefined at editor.component.ts:836. This causes changeText() at line 1787 to early-return and set target.id = undefined. In the text component, bindQuill() sees _id == null and sets loadingState = 'no-content' without calling projectHasText(), so this.project remains whatever it was previously (or undefined). The placeholder getter at text.component.ts:353 then returns 'text.error' ("An error occurred while loading the text") because this.id == null. For a draft-only book, this message is technically misleading — the user intentionally navigated there. However, the draft tab is likely the primary UI surface for these books, and the main editor area is secondary. This may be acceptable UX but is worth the reviewer verifying the intended experience.

---

Was this helpful? React with 👍 or 👎 to provide feedback.