SF-3746 Allow navigating to any drafted book if it exists by RaymondLuong3 · Pull Request #3740 · sillsdev/web-xforge

RaymondLuong3 · 2026-03-12T22:01:46Z

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 · 2026-03-12T22:06:40Z

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.ts

🔴 changeText() crashes via non-null assertion on this.source when this.text is null

Before this PR, changeText() was never reached with this.text == null because the code navigated away when the book wasn't in the project. Now that guard is removed, so when a user navigates to a book that only exists in draftedScriptureRange, this.text is undefined (set at editor.component.ts:840), changeText() is called via loadProjectUserConfig (line 2125), and enters the null guard at line 1791. At line 1792, this.source!.id = undefined uses a non-null assertion, but this.source can be undefined — e.g., when hasSource returns false (because this.text == null at line 665), the source tab won't render the #source ViewChild. This causes a runtime crash.

(Refers to lines 1791-1794)

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

I don't think this is a issue we will see.

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.ts

src/SIL.XForge.Scripture/ClientApp/src/assets/i18n/checking_en.json

devin-ai-integration · 2026-03-12T22:06:43Z

src/SIL.XForge.Scripture/ClientApp/src/app/shared/progress-service/progress.service.ts

+const chapterCounts: Record<string, number> = {
+  GEN: 50,
+  EXO: 40,
+  LEV: 27,
+  NUM: 36,
+  DEU: 34,
+  JOS: 24,
+  JDG: 21,
+  RUT: 4,
+  '1SA': 31,
+  '2SA': 24,
+  '1KI': 22,
+  '2KI': 25,
+  '1CH': 29,
+  '2CH': 36,
+  EZR: 10,
+  NEH: 13,
+  EST: 10,
+  JOB: 42,
+  PSA: 150,
+  PRO: 31,
+  ECC: 12,
+  SNG: 8,
+  ISA: 66,
+  JER: 52,
+  LAM: 5,
+  EZK: 48,
+  DAN: 12,
+  HOS: 14,
+  JOL: 3,
+  AMO: 9,
+  OBA: 1,
+  JON: 4,
+  MIC: 7,
+  NAM: 3,
+  HAB: 3,
+  ZEP: 3,
+  HAG: 2,
+  ZEC: 14,
+  MAL: 4,
+  MAT: 28,
+  MRK: 16,
+  LUK: 24,
+  JHN: 21,
+  ACT: 28,
+  ROM: 16,
+  '1CO': 16,
+  '2CO': 13,
+  GAL: 6,
+  EPH: 6,
+  PHP: 4,
+  COL: 4,
+  '1TH': 5,
+  '2TH': 3,
+  '1TI': 6,
+  '2TI': 4,
+  TIT: 3,
+  PHM: 1,
+  HEB: 13,
+  JAS: 5,
+  '1PE': 5,
+  '2PE': 3,
+  '1JN': 5,
+  '2JN': 1,
+  '3JN': 1,
+  JUD: 1,
+  REV: 22,
+  TOB: 14,
+  JDT: 16,
+  ESG: 10,
+  WIS: 19,
+  SIR: 51,
+  BAR: 6,
+  LJE: 1,
+  S3Y: 1,
+  SUS: 1,
+  BEL: 1,
+  '1MA': 16,
+  '2MA': 15,
+  '3MA': 7,
+  '4MA': 18,
+  '1ES': 9,
+  '2ES': 16,
+  MAN: 1,
+  PS2: 1,
+  ODA: 14,
+  PSS: 18,
+  JSA: 24,
+  JDB: 21,
+  TBS: 14,
+  SST: 1,
+  DNT: 12,
+  BLT: 1,
+  DAG: 14,
+  PS3: 4,
+  '2BA': 77,
+  LBA: 9,
+  JUB: 34,
+  ENO: 42,
+  '1MQ': 36,
+  '2MQ': 20,
+  '3MQ': 10,
+  REP: 6,
+  '4BA': 5,
+  LAO: 1
+};


🟡 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 · 2026-03-12T22:10:37Z

Codecov Report

❌ Patch coverage is 81.81818% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.32%. Comparing base (b4cc938) to head (f8b3ef4).
⚠️ Report is 3 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.31%   81.32%           
=======================================
  Files         620      620           
  Lines       39076    39090   +14     
  Branches     6375     6376    +1     
=======================================
+ Hits        31775    31790   +15     
  Misses       6316     6316           
+ Partials      985      984    -1

☔ 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.

RaymondLuong3 · 2026-03-19T16:41:59Z

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.ts

I don't think this is a issue we will see.

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).

RaymondLuong3 added the will require testing PR should not be merged until testers confirm testing is complete label Mar 12, 2026

devin-ai-integration bot reviewed Mar 12, 2026

View reviewed changes

This comment was marked as resolved.

Sign in to view

pmachapman self-assigned this Mar 17, 2026

pmachapman requested changes Mar 17, 2026

View reviewed changes

RaymondLuong3 added 3 commits March 19, 2026 10:08

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

54118e7

Code review fixes

beef445

Code review fixes

f8b3ef4

RaymondLuong3 force-pushed the feature/sf-3746-navigate-draft-books branch from c5c1636 to f8b3ef4 Compare March 19, 2026 16:40

RaymondLuong3 commented Mar 19, 2026

View reviewed changes

pmachapman approved these changes Mar 22, 2026

View reviewed changes

pmachapman added ready to test and removed will require testing PR should not be merged until testers confirm testing is complete labels Mar 22, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

SF-3746 Allow navigating to any drafted book if it exists#3740
RaymondLuong3 wants to merge 3 commits intomasterfrom
feature/sf-3746-navigate-draft-books

RaymondLuong3 commented Mar 12, 2026 •

edited by irahopkinson

Loading

Uh oh!

devin-ai-integration bot left a comment

Uh oh!

devin-ai-integration bot Mar 12, 2026

Uh oh!

RaymondLuong3 Mar 19, 2026

Uh oh!

Uh oh!

Uh oh!

devin-ai-integration bot Mar 12, 2026 •

edited

Loading

Uh oh!

codecov bot commented Mar 12, 2026 •

edited

Loading

Uh oh!

This comment was marked as resolved.

Uh oh!

pmachapman left a comment

Uh oh!

RaymondLuong3 left a comment

Uh oh!

RaymondLuong3 Mar 19, 2026

Uh oh!

pmachapman left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

RaymondLuong3 commented Mar 12, 2026 • edited by irahopkinson Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

devin-ai-integration bot left a comment

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration bot Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

RaymondLuong3 Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

devin-ai-integration bot Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

This comment was marked as resolved.

Uh oh!

pmachapman left a comment

Choose a reason for hiding this comment

Uh oh!

RaymondLuong3 left a comment

Choose a reason for hiding this comment

Uh oh!

RaymondLuong3 Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

pmachapman left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

RaymondLuong3 commented Mar 12, 2026 •

edited by irahopkinson

Loading

devin-ai-integration bot Mar 12, 2026 •

edited

Loading

codecov bot commented Mar 12, 2026 •

edited

Loading