SF-3745 Do not store or retrieve chapters not requested for drafts

[pmachapman]

This PR adds support for partial book drafting by:

• Only retrieving chapters that are requested in the draft (when chapters are specified)
• Updating the frontend UI and backend logic to handle scripture ranges that contain chapters (Serval already supports this)

---

---

This change is 

[codecov]

Codecov Report

❌ Patch coverage is 70.79646% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.27%. Comparing base (aadcc7d) to head (baf8286).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...SIL.XForge.Scripture/Services/MachineApiService.cs	63.33%	26 Missing and 7 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3744      +/-   ##
==========================================
- Coverage   81.32%   81.27%   -0.05%     
==========================================
  Files         620      620              
  Lines       39086    39159      +73     
  Branches     6375     6397      +22     
==========================================
+ Hits        31787    31828      +41     
- Misses       6329     6342      +13     
- Partials      970      989      +19     

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

[devin-ai-integration]

Devin Review found 0 new potential issues.

View 8 additional findings in Devin Review.

[marksvc]

@marksvc reviewed all commit messages and made 6 comments.
Reviewable status: 0 of 12 files reviewed, 7 unresolved discussions (waiting on pmachapman).

---

src/SIL.XForge.Scripture/ClientApp/src/app/shared/utils.spec.ts line 111 at r2 (raw file):

      expect(booksFromScriptureRange('GEN')).toEqual([1]);
      expect(booksFromScriptureRange('GEN10,11,16-19;EXO')).toEqual([1, 2]);
      expect(booksFromScriptureRange('GEN;NOT_A_BOOK;EXO')).toEqual([1, 2]);

(Just saying) Ah, but what about booksFromScriptureRange('GENERAL_PROTECTION_FAULT'), which I suspect would give [1]? :-) Probably not very important.

---

src/SIL.XForge.Scripture/ClientApp/src/app/shared/utils.ts line 186 at r2 (raw file):

 * @returns The number range as a list of numbers, e.g. [1,2,5,6,7,8,10]
 */
export function expandNumbers(numberRange: string): number[] {

I see that we are guarding against returning transformations of things like abc and a,b[1]. What about if numberRange is the empty string, what do you think we should return? Probably an empty list? (Rather than return [0])

[1] Oh, from the test I see invalid is transformed to 0, rather than NaN and filtered out. So I may be wrong about how it would handle empty string. We may do well to at least specify the behaviour of expandNumbers('') in the test permutations.

---

src/SIL.XForge.Scripture/ClientApp/src/app/core/sf-project.service.ts line 87 at r2 (raw file):

    const scriptureRanges: string[] = scriptureRange.split(';').filter(book => book.startsWith(bookId));

    // If the book is not present, the

Looks like this comment is missing the end.

---

src/SIL.XForge.Scripture/ClientApp/src/app/shared/utils.ts line 192 at r2 (raw file):

      if (part.includes('-')) {
        const [start, end] = part.split('-').map(Number);
        return Array.from({ length: end - start + 1 }, (_, i) => start + i);

(Just saying) I suspect we could get some interesting results for arguments like i-2 or 5-0, but I won't be too concerned about that here :)

---

src/SIL.XForge.Scripture/Services/MachineApiService.cs line 2581 at r2 (raw file):

    /// <param name="bookNum">The book number.</param>
    /// <param name="chapterNum">The chapter number.</param>
    /// <returns>The builds containing the specified book.</returns>

Nit: needs "and chapter".

---

src/SIL.XForge.Scripture/Services/MachineApiService.cs line 2446 at r2 (raw file):

    /// <param name="chapters">The collection of chapters.</param>
    /// <returns>A chapter range string.</returns>
    private static string GetChaptersAsRange(IEnumerable<int> chapters)

(Just saying.) Good job making all these back-and-forth processors. I might have thought we could have a Scripture range data type that we could pass around, and then serialize or deserialize it when going to-from the DB and Serval and UI. It seems that could be less fragile than passing around text-based ranges with semicolon, comma, and dash markers. I'm not going to argue here that we should move to that instead; and it could well be that if I saw how that implementation looked I would not be happy with it :). But I'll mention it as it occurs to me.

Hmm, and it may be that we are mostly doing the transformations at these boundary layers already.

[pmachapman]

@pmachapman made 5 comments, resolved 4 discussions, and dismissed @github-advanced-security[bot] from a discussion.
Reviewable status: 0 of 12 files reviewed, 3 unresolved discussions (waiting on marksvc).

---

src/SIL.XForge.Scripture/ClientApp/src/app/core/sf-project.service.ts line 87 at r2 (raw file):

Previously, marksvc wrote…

Looks like this comment is missing the end.

Done. Thanks!

---

src/SIL.XForge.Scripture/ClientApp/src/app/shared/utils.ts line 186 at r2 (raw file):

Previously, marksvc wrote…

I see that we are guarding against returning transformations of things like abc and a,b[1]. What about if numberRange is the empty string, what do you think we should return? Probably an empty list? (Rather than return [0])

[1] Oh, from the test I see invalid is transformed to 0, rather than NaN and filtered out. So I may be wrong about how it would handle empty string. We may do well to at least specify the behaviour of expandNumbers('') in the test permutations.

invalid is dropped in that test (the 0 was beside the invalid). I've updated the test to have the zero elsewhere.

I've added some tests for empty strings, and tweaked the logic to return an empty array in these cases.

---

src/SIL.XForge.Scripture/ClientApp/src/app/shared/utils.ts line 192 at r2 (raw file):

Previously, marksvc wrote…

(Just saying) I suspect we could get some interesting results for arguments like i-2 or 5-0, but I won't be too concerned about that here :)

I've added some tests for these cases.

---

src/SIL.XForge.Scripture/ClientApp/src/app/shared/utils.spec.ts line 111 at r2 (raw file):

Previously, marksvc wrote…

(Just saying) Ah, but what about booksFromScriptureRange('GENERAL_PROTECTION_FAULT'), which I suspect would give [1]? :-) Probably not very important.

It would also parse Genesis correctly. I think I'm OK with this, but if you think I should block it, I could add a regex?

---

src/SIL.XForge.Scripture/Services/MachineApiService.cs line 2581 at r2 (raw file):

Previously, marksvc wrote…

Nit: needs "and chapter".

Done. Thanks!

[devin-ai-integration]

Devin Review found 0 new potential issues.

View 8 additional findings in Devin Review.

[devin-ai-integration]

Devin Review found 3 new potential issues.

View 9 additional findings in Devin Review.

[marksvc]

@marksvc reviewed 12 files and all commit messages, made 3 comments, and resolved 2 discussions.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on pmachapman).

---

src/SIL.XForge.Scripture/ClientApp/src/app/shared/utils.spec.ts line 111 at r2 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

It would also parse Genesis correctly. I think I'm OK with this, but if you think I should block it, I could add a regex?

No.

---

src/SIL.XForge.Scripture/ClientApp/src/app/shared/utils.spec.ts line 124 at r3 (raw file):

    it('should return an empty array for invalid number ranges', () => {
      expect(expandNumbers('')).toEqual([]);
      expect(expandNumbers('3-1')).toEqual([]);

(Just saying.) Nice.

---

src/SIL.XForge.Scripture/Services/MachineApiService.cs line 1927 at r3 (raw file):

                ParseScriptureRange(currentScriptureRange, scriptureRangeParser, ref booksWithDrafts);
                ParseScriptureRange(draftedScriptureRange, scriptureRangeParser, ref booksWithDrafts);
                draftedScriptureRange = GetScriptureRange(booksWithDrafts);

Sorry to ask for this later on in the code review. Can you add a test demonstrating this method updating the drafted Scripture range as expected? For example, if

• Drafted Scripture Range starts as GEN1,3-4,7,EXO,LEV
• A new draft happened, and its Current Scripture Range is GEN2,4,5,LEV
• After this method runs, the Drafted Scripture Range should be GEN1-5,7;EXO;LEV