SF-3191 Improve latest draft retrieval when webhook fails

[kylebuss]

We want to ensure the latest draft is viewable for our users. If the webhook from Serval fails to process the latest selected translation books will not display. This PR now checks when the user navigates to the drafting page if we are missing data and retrieves it. It utilizes the ScriptureRangeParser from SIL.Machine to parse the scripture range that is sent to Serval for the latest draft.

Marked as "Testing not required" but feel free to change this if you believe it needs testing in addition to developer testing. I've added how to force the issue to occur in Jira

---

This change is 

[codecov]

Codecov Report

Attention: Patch coverage is 96.55172% with 1 line in your changes missing coverage. Please review.

Project coverage is 82.82%. Comparing base (e241ec3) to head (351f310).

Files with missing lines	Patch %	Lines
...SIL.XForge.Scripture/Services/MachineApiService.cs	96.55%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3132      +/-   ##
==========================================
+ Coverage   82.81%   82.82%   +0.01%     
==========================================
  Files         564      564              
  Lines       32708    32736      +28     
  Branches     5275     5257      -18     
==========================================
+ Hits        27087    27114      +27     
- Misses       4838     4849      +11     
+ Partials      783      773      -10     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[RaymondLuong3]

I really would rather be using a library. What is wrong with ScriptureRangeParser.ToChapters() in SIL.Machine that makes it not work for us?

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @kylebuss and @pmachapman)

---

src/SIL.XForge.Scripture/Helpers/ScriptureRangeParser.cs line 22 at r1 (raw file):

        @"^([A-Z\d]{3}|OT|NT)(, ?([A-Z\d]{3}|OT|NT))*$",
        RegexOptions.Compiled
    );

This doesn't look like it is handling when a book is subtracted, like GEN-DEU;-NUM

Code quote:

    private readonly Regex CommaSeparatedBooks = new Regex(
        @"^([A-Z\d]{3}|OT|NT)(, ?([A-Z\d]{3}|OT|NT))*$",
        RegexOptions.Compiled
    );

---

src/SIL.XForge.Scripture/Helpers/ScriptureRangeParser.cs line 98 at r1 (raw file):

                break;
            case "NT":
                AddAllBooksForTestament(book);

This can just be collapsed since the logic is the same. Also, book is a misleading name. Perhaps token would be better

Code quote:

            case "OT":
                AddAllBooksForTestament(book);
                break;
            case "NT":
                AddAllBooksForTestament(book);

[Enkidu93]

I would think Machine's ScriptureRangeParser would be sufficient. If it does in fact not meet your needs, do you think we could easily make the updates in Machine necessary to meet your needs, @kylebuss?

[kylebuss]

I thought I had replied to this but apparently I forgot to publish.

Yes Raymond and Eli, after looking closer at Machine's ScriptureRangeParser.GetChapters() it should meet our needs. We really only need the list of BookId's from the scripture range and this method will give us that as the key plus its chapters.

I've added the latest NuGet package for SIL.Machine and a call to the parser in MachineApiService.cs.

[kylebuss]

@Enkidu93 kind of a side question, but would it be difficult to add a retry to the Serval webhook if it receives a failed message or a bad response code?

[Enkidu93]

@Enkidu93 kind of a side question, but would it be difficult to add a retry to the Serval webhook if it receives a failed message or a bad response code?

It should already. It retries up to 20 times over about 5 hours - or at least it should be!

[kylebuss]

Awesome, thanks for the info!

Reviewable status: 0 of 5 files reviewed, all discussions resolved (waiting on @pmachapman and @RaymondLuong3)

[pmachapman]

Reviewable status: 0 of 5 files reviewed, 2 unresolved discussions (waiting on @Github-advanced-security[bot] and @RaymondLuong3)

---

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

        // Ensure that the user has permission
        await EnsureProjectPermissionAsync(curUserId, sfProjectId, isServalAdmin);

This returns a project, so you could just use the project returned here instead of getting it below.

Code quote:

await EnsureProjectPermissionAsync(curUserId, sfProjectId, isServalAdmin);

---

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

                            }
                        });
                }

I like your approach, but I have concerns about this running every time the latest build is retrieved.

Given a scenario when a user has a book with with one 2 chapters (say Ezra 2 and 3), generates a draft, and then afterwards adds the remaining chapters, this will check for drafts every single time this method is called.

I wonder if we should take your logic here, and use it to replace this block in PreTranslationService.UpdatePreTranslationStatusAsync():

        Dictionary<int, HashSet<int>> bookChapters = [];
        foreach (
            Pretranslation preTranslation in await translationEnginesClient.GetAllPretranslationsAsync(
                translationEngineId,
                corpusId,
                textId: null,
                cancellationToken
            )
        )
        {

as your check is much more efficient, and perhaps just record the buildId as a "lastPretranslatedBuildId" or similar in sf_project_secrets, and only update if the buildId we retrieve here is different to the last one recorded?

Code quote:

                SFProject project = await projectService.GetProjectAsync(sfProjectId);

                Dictionary<string, List<int>> scriptureRanges = ScriptureRangeParser.GetChapters(
                    project.TranslateConfig.DraftConfig.LastSelectedTranslationScriptureRange
                );

                string[] scriptureRangeIds = [.. scriptureRanges.Keys];

                foreach (string bookId in Canon.AllBookIds.Where(bookId => scriptureRangeIds.Contains(bookId)))
                {
                    int bookNum = Canon.BookIdToNumber(bookId);

                    project
                        .Texts.Where(text => text.BookNum == bookNum)
                        .ForEach(async info =>
                        {
                            if (info.Chapters.Any(c => !c.HasDraft ?? false))
                            {
                                await RetrievePreTranslationStatusAsync(sfProjectId, cancellationToken);
                                return;
                            }
                        });
                }

[kylebuss]

Reviewable status: 0 of 5 files reviewed, 2 unresolved discussions (waiting on @Github-advanced-security[bot], @pmachapman, and @RaymondLuong3)

---

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

Previously, pmachapman (Peter Chapman) wrote…

This returns a project, so you could just use the project returned here instead of getting it below.

Cool. I will look into this!

[kylebuss]

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

Previously, pmachapman (Peter Chapman) wrote…

I like your approach, but I have concerns about this running every time the latest build is retrieved.

Given a scenario when a user has a book with with one 2 chapters (say Ezra 2 and 3), generates a draft, and then afterwards adds the remaining chapters, this will check for drafts every single time this method is called.

I wonder if we should take your logic here, and use it to replace this block in PreTranslationService.UpdatePreTranslationStatusAsync():

        Dictionary<int, HashSet<int>> bookChapters = [];
        foreach (
            Pretranslation preTranslation in await translationEnginesClient.GetAllPretranslationsAsync(
                translationEngineId,
                corpusId,
                textId: null,
                cancellationToken
            )
        )
        {

as your check is much more efficient, and perhaps just record the buildId as a "lastPretranslatedBuildId" or similar in sf_project_secrets, and only update if the buildId we retrieve here is different to the last one recorded?

I have updated the following from our discussion:

1. Use EnsureProjectPermissionsAsync to get the project.
2. Use the scripture ranges from Serval rather than LastSelectedTranslationScriptureRange.
3. Use a Linq query to determine if any chapters HasDraft status is null or false when it should be true.

I have not gotten the chance to setup projects to test the scenario if a target/source is missing chapters and will update you once I've tested this.

[kylebuss]

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

Previously, kylebuss (Kyle Buss) wrote…

I have updated the following from our discussion:

1. Use EnsureProjectPermissionsAsync to get the project.
2. Use the scripture ranges from Serval rather than LastSelectedTranslationScriptureRange.
3. Use a Linq query to determine if any chapters HasDraft status is null or false when it should be true.

I have not gotten the chance to setup projects to test the scenario if a target/source is missing chapters and will update you once I've tested this.

Reverted this back to draft status. Need to review existing behavior as testing the target/source missing chapters I observed an interesting behavior in the UI and just want to confirm if this change caused the behavior or not.

[kylebuss]

@pmachapman @RaymondLuong3 This is ready for further review. I've verified that the behavior I was seeing matches what happens on master.

The latest changes should handle both scenarios if the target or source is missing chapters. I did not investigate it but on both master and this branch if the book was empty (Titus) the draft only added the first chapter and not all the chapters.

Reviewable status: 0 of 5 files reviewed, 2 unresolved discussions (waiting on @Github-advanced-security[bot], @pmachapman, and @RaymondLuong3)

[pmachapman]

Reviewed 4 of 4 files at r4, all commit messages.
Reviewable status: 4 of 5 files reviewed, 5 unresolved discussions (waiting on @RaymondLuong3)

---

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

                            .ToDictionary(kvp => kvp.Key, kvp => kvp.Value, StringComparer.OrdinalIgnoreCase);
                    }
                }

You could probably simplify this to:

                Dictionary<string, List<int>> scriptureRanges = (translationBuild.Pretranslate ?? [])
                    .SelectMany(pc => pc.SourceFilters ?? [])
                    .SelectMany(pcf => ScriptureRangeParser.GetChapters(pcf.ScriptureRange))
                    .ToDictionary(kvp => kvp.Key, kvp => kvp.Value, StringComparer.OrdinalIgnoreCase);

However I notice that the values are not merged if scripture ranges share the same book. Serval supports (and we will eventually implement) translating some chapters with one source and other chapters with another. Also, we need to use the correct versification for the project when calculating the project chapters, and as noted in the next comment, ScriptureRangeParser.GetChapters() returns an empty list if all of the chapters in a book are selected (at least in my tests?).

We should instead handle it something like this:

 // TODO: Get the correct versification for the project from ParatextData's ProjectSettings.Versification
ScrVers versification = ScrVers.English;
ScriptureRangeParser scriptureRangeParser = new ScriptureRangeParser(versification);

foreach (PretranslateCorpus ptc in translationBuild.Pretranslate ?? [])
{
    // We are using the TranslationBuild.Pretranslate.SourceFilters.ScriptureRange to find the
    // books selected for drafting. Some projects may have used the now obsolete field
    // TranslationBuild.Pretranslate.ScriptureRange and will not get checked for webhook failures.
    foreach (ParallelCorpusFilter source in ptc.SourceFilters ?? [])
    {
        foreach ((string book, List<int> bookChapters) in scriptureRangeParser.GetChapters(source.ScriptureRange))
        {
            // Ensure that if chapters is blank, it contains every chapter in the book
            List<int> chapters = bookChapters;
            if (chapters.Count == 0)
            {
                chapters = [.. Enumerable.Range(1, versification.GetLastChapter(Canon.BookIdToNumber(book)))];
            }

            // Set or merge the list of chapters
            if (!scriptureRanges.TryGetValue(book, out List<int> existingChapters))
            {
                scriptureRanges[book] = chapters;
            }
            else
            {
                // Merge new chapters into existing list, avoiding duplicates
                foreach (int chapter in chapters.Where(chapter => !existingChapters.Contains(chapter)))
                {
                    existingChapters.Add(chapter);
                }
            }
        }
    }
}

Code quote:

                IList<PretranslateCorpus> pretranslateCorpus = translationBuild.Pretranslate;

                foreach (PretranslateCorpus ptc in pretranslateCorpus)
                {
                    // We are using the TranslationBuild.Pretranslate.SourceFilters.ScriptureRange to find the
                    // books selected for drafting. Some projects may have used the now obsolete field
                    // TranslationBuild.Pretranslate.ScriptureRange and will not get checked for webhook failures.
                    foreach (ParallelCorpusFilter source in ptc.SourceFilters)
                    {
                        Dictionary<string, List<int>>? scriptureRange = ScriptureRangeParser.GetChapters(
                            source.ScriptureRange
                        );

                        scriptureRanges = scriptureRanges
                            .Union(scriptureRange)
                            .ToDictionary(kvp => kvp.Key, kvp => kvp.Value, StringComparer.OrdinalIgnoreCase);
                    }
                }

---

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

                            && text.Chapters.Where(chapter => scriptureRanges[book].Contains(chapter.Number))
                                .All(c => c.HasDraft ?? false)
                        );

You are checking for the opposite of what the variable is called (your logic is saying "Do all chapters have HasDraft set to true).

This instead should look something like:

                bool hasDraftMissingFromChapters = Canon
                    .AllBookIds.Where(scriptureRanges.ContainsKey)
                    .All(book => project.Texts.Any(text =>
                        text.BookNum == Canon.BookIdToNumber(book)
                        && text.Chapters.Where(chapter => scriptureRanges[book].Contains(chapter.Number))
                            .All(c => !(c.HasDraft ?? false))
                    ));

Before fixing this bug, perhaps create a test case that will fail and expose this bug?

Code quote:

                bool hasDraftMissingFromChapters = Canon
                    .AllBookIds.Where(book => scriptureRangeIds.Contains(book))
                    .All(book =>
                    {
                        int bookNum = Canon.BookIdToNumber(book);
                        return project.Texts.Any(text =>
                            text.BookNum == bookNum
                            && text.Chapters.Where(chapter => scriptureRanges[book].Contains(chapter.Number))
                                .All(c => c.HasDraft ?? false)
                        );

---

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

                {
                    // Chapters HasDraft is missing or false but should be true, retrieve the pre-translation status to update them.
                    await RetrievePreTranslationStatusAsync(sfProjectId, cancellationToken);

I created a draft for 2PE, and every time I viewed the build page, this ran because ScriptureRangeParser.GetChapters(source.ScriptureRange) returns an empty list for a book if no subset of chapters are specified, and your check performed the opposite of what it should. (see both code suggestions above)

Code quote:

await RetrievePreTranslationStatusAsync(sfProjectId, cancellationToken);

---

test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs line 1099 at r4 (raw file):

                                            ScriptureRange = "GEN",
                                        },
                                    ],

This is too simplistic. As a large part of your logic is about handling multiple parallel corpus filters, can you create tests that expand these checks? Perhaps values like:

                                    SourceFilters =
                                    [
                                        new ParallelCorpusFilter
                                        {
                                            Corpus = { Id = "corpusId1", Url = "https://example.com" },
                                            ScriptureRange = "GEN1-2",
                                        },
                                        new ParallelCorpusFilter
                                        {
                                            Corpus = { Id = "corpusId2", Url = "https://example.com" },
                                            ScriptureRange = "GEN3-4;EXO",
                                        },
                                        new ParallelCorpusFilter
                                        {
                                            Corpus = { Id = "corpusId2", Url = "https://example.com" },
                                            ScriptureRange = "EXO",
                                        },
                                        new ParallelCorpusFilter
                                        {
                                            Corpus = { Id = "corpusId2", Url = "https://example.com" },
                                            ScriptureRange = "LEV",
                                        },
                                    ],

(have a think on other edge cases too - for example, I notice that you check the book ids against Canon.AllBookIds - Have ParallelCorpusFilter where the ScriptureRange is empty or bogus values like "INVALID")

Code quote:

                                    SourceFilters =
                                    [
                                        new ParallelCorpusFilter
                                        {
                                            Corpus = new ResourceLink { Id = "corpusId", Url = "https://example.com" },
                                            ScriptureRange = "GEN",
                                        },
                                    ],

---

test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs line 1107 at r4 (raw file):

            );
        SFProject project = env.Projects.Get(Project01);
        project.Texts[0].Chapters[1].HasDraft = true;

This is kind of hacky. Also, the fact that the tests did not pick up the logic error I noted in hasDraftMissingFromChapters means that the test is not a good test. Can you implement tests that have all chapters with drafts, so code errors like the logic error in hasDraftMissingFromChapters can be picked up?

Code quote:

        SFProject project = env.Projects.Get(Project01);
        project.Texts[0].Chapters[1].HasDraft = true;

To fix the problem, we need to remove the assignment to the scriptureRangeIds variable since it is not used anywhere in the provided code snippet. This will clean up the code and eliminate the unnecessary assignment.

• Remove the line that assigns a value to scriptureRangeIds.
• Ensure that the removal does not affect the existing functionality of the code.

To fix the problem, we need to simplify the Boolean expression by removing the unnecessary ternary conditional. Specifically, we will replace the expression scriptureRangeBooks.Count > 0 ? scriptureRangeBooks.All(...) : false with scriptureRangeBooks.Count > 0 && scriptureRangeBooks.All(...). This change will maintain the existing functionality while making the code more readable.

[kylebuss]

Reviewable status: 2 of 7 files reviewed, 10 unresolved discussions (waiting on @pmachapman and @RaymondLuong3)

---

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

Previously, pmachapman (Peter Chapman) wrote…

You could probably simplify this to:

                Dictionary<string, List<int>> scriptureRanges = (translationBuild.Pretranslate ?? [])
                    .SelectMany(pc => pc.SourceFilters ?? [])
                    .SelectMany(pcf => ScriptureRangeParser.GetChapters(pcf.ScriptureRange))
                    .ToDictionary(kvp => kvp.Key, kvp => kvp.Value, StringComparer.OrdinalIgnoreCase);

However I notice that the values are not merged if scripture ranges share the same book. Serval supports (and we will eventually implement) translating some chapters with one source and other chapters with another. Also, we need to use the correct versification for the project when calculating the project chapters, and as noted in the next comment, ScriptureRangeParser.GetChapters() returns an empty list if all of the chapters in a book are selected (at least in my tests?).

We should instead handle it something like this:

 // TODO: Get the correct versification for the project from ParatextData's ProjectSettings.Versification
ScrVers versification = ScrVers.English;
ScriptureRangeParser scriptureRangeParser = new ScriptureRangeParser(versification);

foreach (PretranslateCorpus ptc in translationBuild.Pretranslate ?? [])
{
    // We are using the TranslationBuild.Pretranslate.SourceFilters.ScriptureRange to find the
    // books selected for drafting. Some projects may have used the now obsolete field
    // TranslationBuild.Pretranslate.ScriptureRange and will not get checked for webhook failures.
    foreach (ParallelCorpusFilter source in ptc.SourceFilters ?? [])
    {
        foreach ((string book, List<int> bookChapters) in scriptureRangeParser.GetChapters(source.ScriptureRange))
        {
            // Ensure that if chapters is blank, it contains every chapter in the book
            List<int> chapters = bookChapters;
            if (chapters.Count == 0)
            {
                chapters = [.. Enumerable.Range(1, versification.GetLastChapter(Canon.BookIdToNumber(book)))];
            }

            // Set or merge the list of chapters
            if (!scriptureRanges.TryGetValue(book, out List<int> existingChapters))
            {
                scriptureRanges[book] = chapters;
            }
            else
            {
                // Merge new chapters into existing list, avoiding duplicates
                foreach (int chapter in chapters.Where(chapter => !existingChapters.Contains(chapter)))
                {
                    existingChapters.Add(chapter);
                }
            }
        }
    }
}

Done.

---

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

Previously, pmachapman (Peter Chapman) wrote…

You are checking for the opposite of what the variable is called (your logic is saying "Do all chapters have HasDraft set to true).

This instead should look something like:

                bool hasDraftMissingFromChapters = Canon
                    .AllBookIds.Where(scriptureRanges.ContainsKey)
                    .All(book => project.Texts.Any(text =>
                        text.BookNum == Canon.BookIdToNumber(book)
                        && text.Chapters.Where(chapter => scriptureRanges[book].Contains(chapter.Number))
                            .All(c => !(c.HasDraft ?? false))
                    ));

Before fixing this bug, perhaps create a test case that will fail and expose this bug?

Good catch. I think this should actually be .Any(c => !c.HasDraft ?? false), I've updated the comment to clarify we want to see any chapters in the scripture range is set to false or null.

---

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

Previously, pmachapman (Peter Chapman) wrote…

I created a draft for 2PE, and every time I viewed the build page, this ran because ScriptureRangeParser.GetChapters(source.ScriptureRange) returns an empty list for a book if no subset of chapters are specified, and your check performed the opposite of what it should. (see both code suggestions above)

Done.

---

test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs line 1389 at r5 (raw file):

        SFProject project = env.Projects.Get(Project01);
        project.Texts[0].Chapters[1].HasDraft = true;
        env.ConfigureTranslationBuild(CompletedTranslationBuild);

This line can be removed because we set GetAllBuildsAsync() above.

Code quote:

env.ConfigureTranslationBuild(CompletedTranslationBuild);

---

test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs line 1400 at r5 (raw file):

        // verify RetrievePreTranslationStatusAsync was called once
        await env.Service.Received().RetrievePreTranslationStatusAsync(Project01, CancellationToken.None);

This does not appear to be failing the test when it's ran and RetrieveTranslationStatusAsync() has not been called.

Code quote:

await env.Service.Received().RetrievePreTranslationStatusAsync(Project01, CancellationToken.None);

---

test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs line 1410 at r5 (raw file):

        Assert.AreEqual(MachineApi.GetBuildHref(Project01, Build01), actual.Href);
        Assert.AreEqual(Project01, actual.Engine.Id);
        Assert.AreEqual(MachineApi.GetEngineHref(Project01), actual.Engine.Href);

We can use TestEnvironment.AssertCoreBuildProperties(CompletedTranslationBuild, actual); here as it checks the same assertions.

Code quote:

        Assert.IsNotNull(actual);
        Assert.AreEqual(message, actual.Message);
        Assert.AreEqual(percentCompleted, actual.PercentCompleted);
        Assert.AreEqual(revision, actual.Revision);
        Assert.AreEqual(state.ToString().ToUpperInvariant(), actual.State);
        Assert.AreEqual(buildDtoId, actual.Id);
        Assert.AreEqual(MachineApi.GetBuildHref(Project01, Build01), actual.Href);
        Assert.AreEqual(Project01, actual.Engine.Id);
        Assert.AreEqual(MachineApi.GetEngineHref(Project01), actual.Engine.Href);

---

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

                                scriptureRanges[book].AddRange(existingChapters);
                            }
                        }

This is duplicating the book chapters when there are multiple SourceFilters that have the same book. When having more than one source filter with EXO in the scripture range, the number of chapters was 80 rather than 40.

Code quote:

                        foreach (
                            (string book, List<int> bookChapters) in scriptureRangeParser.GetChapters(
                                source.ScriptureRange
                            )
                        )
                        {
                            int bookNum = Canon.BookIdToNumber(book);
                            scriptureRangeBooks.Add(book, bookNum);
                            // Ensure that if chapters is blank, it contains every chapter in the book
                            List<int> chapters = bookChapters;
                            if (chapters.Count == 0)
                            {
                                chapters = [.. Enumerable.Range(1, versification.GetLastChapter(bookNum))];
                            }

                            // Set or merge the list of chapters
                            if (!scriptureRanges.TryGetValue(book, out List<int> existingChapters))
                            {
                                scriptureRanges[book] = chapters;
                            }
                            else
                            {
                                // Merge new chapters into existing list, avoiding duplicates
                                foreach (int chapter in chapters.Where(chapter => !existingChapters.Contains(chapter)))
                                {
                                    existingChapters.Add(chapter);
                                }
                                // add existing chapters to the books chapter list
                                scriptureRanges[book].AddRange(existingChapters);
                            }
                        }