Merge pull request #220 from WycliffeAssociates/fix-docx-book-repeat-…

…issue Fix docx book repeat issue
WycliffeAssociates · Sep 14, 2024 · d2e5927 · d2e5927
2 parents 73967aa + 1daaf38
commit d2e5927
Show file tree

Hide file tree

Showing 4 changed files with 219 additions and 169 deletions.
diff --git a/...document/domain/assembly_strategies_docx/assembly_strategies_lang_then_book_by_chapter.py b/...document/domain/assembly_strategies_docx/assembly_strategies_lang_then_book_by_chapter.py
@@ -51,33 +51,31 @@ def assemble_content_by_lang_then_book(
     chunk_size, to interleaving strategy to do the actual
     interleaving.
     """
-    book_id_map = dict((id, pos) for pos, id in enumerate(BOOK_NAMES.keys()))
     composers: list[Composer] = []
-    most_lang_codes = max(
-        [
-            [usfm_book.lang_code for usfm_book in usfm_books],
-            [tn_book.lang_code for tn_book in tn_books],
-            [tq_book.lang_code for tq_book in tq_books],
-            [tw_book.lang_code for tw_book in tw_books],
-            [bc_book.lang_code for bc_book in bc_books],
-        ],
-        key=lambda x: len(x),
+    book_id_map = dict((id, pos) for pos, id in enumerate(BOOK_NAMES.keys()))
+    all_lang_codes = (
+        {usfm_book.lang_code for usfm_book in usfm_books}
+        .union(tn_book.lang_code for tn_book in tn_books)
+        .union(tq_book.lang_code for tq_book in tq_books)
+        .union(tw_book.lang_code for tw_book in tw_books)
+        .union(bc_book.lang_code for bc_book in bc_books)
     )
-    most_book_codes = max(
-        [
-            [usfm_book.book_code for usfm_book in usfm_books],
-            [tn_book.book_code for tn_book in tn_books],
-            [tq_book.book_code for tq_book in tq_books],
-            [tw_book.book_code for tw_book in tw_books],
-            [bc_book.book_code for bc_book in bc_books],
-        ],
-        key=lambda x: len(x),
+    most_lang_codes = list(all_lang_codes)
+    # Collect and deduplicate book codes
+    all_book_codes = (
+        {usfm_book.book_code for usfm_book in usfm_books}
+        .union(tn_book.book_code for tn_book in tn_books)
+        .union(tq_book.book_code for tq_book in tq_books)
+        .union(tw_book.book_code for tw_book in tw_books)
+        .union(bc_book.book_code for bc_book in bc_books)
+    )
+    most_book_codes = list(all_book_codes)
+    # Cache book_id_map lookup
+    book_codes_sorted = sorted(
+        most_book_codes, key=lambda book_code: book_id_map[book_code]
     )
     for lang_code in most_lang_codes:
-        for book_code in sorted(
-            most_book_codes,
-            key=lambda book_code: book_id_map[book_code],
-        ):
+        for book_code in book_codes_sorted:
             selected_usfm_books = [
                 usfm_book
                 for usfm_book in usfm_books
@@ -117,55 +115,55 @@ def assemble_content_by_lang_then_book(
                 if bc_book.lang_code == lang_code and bc_book.book_code == book_code
             ]
             bc_book = selected_bc_books[0] if selected_bc_books else None
-        if usfm_book is not None:
-            composers.append(
-                assemble_usfm_by_book(
-                    usfm_book,
-                    tn_book,
-                    tq_book,
-                    tw_book,
-                    usfm_book2,
-                    bc_book,
+            if usfm_book is not None:
+                composers.append(
+                    assemble_usfm_by_book(
+                        usfm_book,
+                        tn_book,
+                        tq_book,
+                        tw_book,
+                        usfm_book2,
+                        bc_book,
+                    )
                 )
-            )
-        elif usfm_book is None and tn_book is not None:
-            composers.append(
-                assemble_tn_by_book(
-                    usfm_book,
-                    tn_book,
-                    tq_book,
-                    tw_book,
-                    usfm_book2,
-                    bc_book,
+            elif usfm_book is None and tn_book is not None:
+                composers.append(
+                    assemble_tn_by_book(
+                        usfm_book,
+                        tn_book,
+                        tq_book,
+                        tw_book,
+                        usfm_book2,
+                        bc_book,
+                    )
                 )
-            )
-        elif usfm_book is None and tn_book is None and tq_book is not None:
-            composers.append(
-                assemble_tq_by_book(
-                    usfm_book,
-                    tn_book,
-                    tq_book,
-                    tw_book,
-                    usfm_book2,
-                    bc_book,
+            elif usfm_book is None and tn_book is None and tq_book is not None:
+                composers.append(
+                    assemble_tq_by_book(
+                        usfm_book,
+                        tn_book,
+                        tq_book,
+                        tw_book,
+                        usfm_book2,
+                        bc_book,
+                    )
                 )
-            )
-        elif (
-            usfm_book is None
-            and tn_book is None
-            and tq_book is None
-            and (tw_book is not None or bc_book is not None)
-        ):
-            composers.append(
-                assemble_tw_by_book(
-                    usfm_book,
-                    tn_book,
-                    tq_book,
-                    tw_book,
-                    usfm_book2,
-                    bc_book,
+            elif (
+                usfm_book is None
+                and tn_book is None
+                and tq_book is None
+                and (tw_book is not None or bc_book is not None)
+            ):
+                composers.append(
+                    assemble_tw_by_book(
+                        usfm_book,
+                        tn_book,
+                        tq_book,
+                        tw_book,
+                        usfm_book2,
+                        bc_book,
+                    )
                 )
-            )
     first_composer = composers[0]
     for composer in composers[1:]:
         first_composer.append(composer.doc)

diff --git a/frontend/src/routes/transfer/[repo]/+page.svelte b/frontend/src/routes/transfer/[repo]/+page.svelte
@@ -209,6 +209,13 @@
             getName(item)
           ])
           if ($langCodesStore[0]) {
+            // We use "all" as a sentinal to tell the backend
+            // resource_types endpoint to use all OT and NT books. We
+            // do this so that we don't have to actually pass a comma delimited
+            // list of all OT and NT books since that makes the URL
+            // really long. Such a long URL was working but is stylistically
+            // not preferred and it /could/, for some browsers, potentially
+            // cause an issue.
             getResourceTypesAndNames($langCodesStore[0], [["all","all"]]).then(
               (resourceTypesAndNames) => {
                 // Filter down to the resource type provided by the user

diff --git a/frontend/tests/e2e/test.ts b/frontend/tests/e2e/test.ts
@@ -1,121 +1,121 @@
 import { expect, test } from '@playwright/test'
 
 test('test ui part 1', async ({ page }) => {
-    await page.goto('http://localhost:8001/languages')
-    await page.getByText('Tiếng Việt (Vietnamese)').click()
-    await page.getByText('অসমীয়া (Assamese) as').click()
-    await page.getByRole('button', { name: 'Next' }).click()
-    await page.getByRole('button', { name: 'New Testament' }).click()
-    await page.getByText('Galatians').click()
-    await page.getByText('Luke').click()
-    await page.getByRole('button', { name: 'Next' }).click()
-    await page
-        .getByText(/.*Unlocked Literal Bible.*/)
-        .first()
-        .click()
-    await page.getByText('Unlocked Literal Bible').nth(1).click()
-    await page.getByText('Translation Notes').first().click()
-    await page.getByText('Translation Notes').nth(1).click()
-    await page.getByText('Translation Questions').first().click()
-    await page.getByText('Translation Questions').nth(1).click()
-    // await page.getByText('Translation Words').first().click()
-    // await page.getByText('Translation Words').nth(1).click()
-    await page.getByRole('button', { name: 'Next' }).click()
-    await page.getByRole('button', { name: 'Generate File' }).click()
-    await expect(page.locator('body')).toContainText('Assamese')
-    await expect(page.locator('body')).toContainText('Vietnamese')
-    await expect(page.locator('body')).toContainText('Galatians')
-    await expect(page.locator('body')).toContainText('Translation Notes')
-    await expect(page.locator('body')).toContainText('Translation Questions')
-    // await expect(page.locator('body')).toContainText('Translation Words')
-    await expect(page.locator('body')).toContainText(/.*Unlocked Literal Bible.*/)
+  await page.goto('http://localhost:8001/languages')
+  await page.getByText('Tiếng Việt (Vietnamese)').click()
+  await page.getByText('অসমীয়া (Assamese) as').click()
+  await page.getByRole('button', { name: 'Next' }).click()
+  await page.getByRole('button', { name: 'New Testament' }).click()
+  await page.getByText('Galatians').click()
+  await page.getByText('Luke').click()
+  await page.getByRole('button', { name: 'Next' }).click()
+  await page
+    .getByText(/.*Unlocked Literal Bible.*/)
+    .first()
+    .click()
+  await page.getByText('Unlocked Literal Bible').nth(1).click()
+  await page.getByText('Translation Notes').first().click()
+  await page.getByText('Translation Notes').nth(1).click()
+  await page.getByText('Translation Questions').first().click()
+  await page.getByText('Translation Questions').nth(1).click()
+  // await page.getByText('Translation Words').first().click()
+  // await page.getByText('Translation Words').nth(1).click()
+  await page.getByRole('button', { name: 'Next' }).click()
+  await page.getByRole('button', { name: 'Generate File' }).click()
+  await expect(page.locator('body')).toContainText('Assamese')
+  await expect(page.locator('body')).toContainText('Vietnamese')
+  await expect(page.locator('body')).toContainText('Galatians')
+  await expect(page.locator('body')).toContainText('Translation Notes')
+  await expect(page.locator('body')).toContainText('Translation Questions')
+  // await expect(page.locator('body')).toContainText('Translation Words')
+  await expect(page.locator('body')).toContainText(/.*Unlocked Literal Bible.*/)
 })
 
 test('test ui part 2', async ({ page }) => {
-    await page.goto('http://localhost:8001/languages')
-    await page.getByText('English').click()
-    await page.getByText('Español Latin America (Latin').click()
-    await page.getByRole('button', { name: 'Next' }).click()
-    await page.getByText('Galatians').click()
-    await page.getByRole('button', { name: 'Next' }).click()
-    await page
-        .getByText(/.*Unlocked Literal Bible.*/)
-        .first()
-        .click()
-    await page
-        .getByText(/.*Unlocked Literal Bible.*/)
-        .nth(1)
-        .click()
-    await page
-        .getByText(/.*Translation Notes.*/)
-        .nth(1)
-        .click()
-    await page
-        .getByText(/.*Translation Notes.*/)
-        .nth(2)
-        .click()
-    await page.getByRole('button', { name: 'Next' }).click()
-    await page.getByText('PDF').click()
-    await page.getByText('Interleave content by chapter').click()
-    await page.getByRole('button', { name: 'Generate File' }).click()
+  await page.goto('http://localhost:8001/languages')
+  await page.getByText('English').click()
+  await page.getByText('Español Latin America (Latin').click()
+  await page.getByRole('button', { name: 'Next' }).click()
+  await page.getByText('Galatians').click()
+  await page.getByRole('button', { name: 'Next' }).click()
+  await page
+    .getByText(/.*Unlocked Literal Bible.*/)
+    .first()
+    .click()
+  await page
+    .getByText(/.*Unlocked Literal Bible.*/)
+    .nth(1)
+    .click()
+  await page
+    .getByText(/.*Translation Notes.*/)
+    .nth(1)
+    .click()
+  await page
+    .getByText(/.*Translation Notes.*/)
+    .nth(2)
+    .click()
+  await page.getByRole('button', { name: 'Next' }).click()
+  await page.getByText('PDF').click()
+  await page.getByText('Interleave content by chapter').click()
+  await page.getByRole('button', { name: 'Generate File' }).click()
 })
 
 test('test books retained in basket on back button to languages and then forward', async ({
-    page
+  page
 }) => {
-    await page.goto('http://localhost:8001/languages')
-    await page.getByText(/.*Amharic.*/).click()
-    await page.getByRole('button', { name: 'Heart' }).click()
-    await page.getByPlaceholder('Search Languages').click()
-    await page.getByPlaceholder('Search Languages').fill('adh')
-    await page.getByLabel(/.*Adhola.*/).check()
-    await page.getByRole('button', { name: 'Next' }).click()
-    await page.getByText('2 John').click()
-    await page.getByText('2 John').nth(1).click()
-    await page
-        .locator('div')
-        .filter({ hasText: /.*Adhola.*/ })
-        .first()
-        .click()
-    await page.locator('.flex-shrink-0 > div:nth-child(4)').click()
-    await page.getByRole('link', { name: 'Languages' }).click()
-    await page.getByRole('button', { name: 'Next' }).click()
-    await page.getByText('2 John').nth(1).click()
-    await page
-        .locator('div')
-        .filter({ hasText: /.*Adhola.*/ })
-        .first()
-        .click()
+  await page.goto('http://localhost:8001/languages')
+  await page.getByText(/.*Amharic.*/).click()
+  await page.getByRole('button', { name: 'Heart' }).click()
+  await page.getByPlaceholder('Search Languages').click()
+  await page.getByPlaceholder('Search Languages').fill('adh')
+  await page.getByLabel(/.*Adhola.*/).check()
+  await page.getByRole('button', { name: 'Next' }).click()
+  await page.getByText('2 John').click()
+  await page.getByText('2 John').nth(1).click()
+  await page
+    .locator('div')
+    .filter({ hasText: /.*Adhola.*/ })
+    .first()
+    .click()
+  await page.locator('.flex-shrink-0 > div:nth-child(4)').click()
+  await page.getByRole('link', { name: 'Languages' }).click()
+  await page.getByRole('button', { name: 'Next' }).click()
+  await page.getByText('2 John').nth(1).click()
+  await page
+    .locator('div')
+    .filter({ hasText: /.*Adhola.*/ })
+    .first()
+    .click()
 })
 
 test('test transfer from biel', async ({ page }) => {
-    await page.goto(
-        'http://localhost:8001/transfer/repo_url=https%3A%2F%2Fcontent.bibletranslationtools.org%2Fchunga_moses%2Fleb-x-bisa_col_text_reg&book_name=Colossians'
-    )
-    await expect(page.getByText('Bisa')).toBeVisible()
-    await expect(page.getByText('Colossians')).toBeVisible()
+  await page.goto(
+    'http://localhost:8001/transfer/repo_url=https%3A%2F%2Fcontent.bibletranslationtools.org%2Fchunga_moses%2Fleb-x-bisa_col_text_reg&book_name=Colossians'
+  )
+  await expect(page.getByText('Bisa')).toBeVisible()
+  await expect(page.getByText('Colossians')).toBeVisible()
 })
 
 test('test transfer from biel 2', async ({ page }) => {
-    await page.goto(
-        'http://localhost:8001/transfer/repo_url=https:%2F%2Fcontent.bibletranslationtools.org%2FWycliffeAssociates%2Fen_ulb'
-    )
-    await expect(page.getByText('English')).toBeVisible()
-    await expect(page.getByText('Genesis')).toBeVisible()
-    await expect(page.getByText('Deuteronomy')).toBeVisible()
-    await expect(page.getByText('(60) items hidden')).toBeVisible()
+  await page.goto(
+    'http://localhost:8001/transfer/repo_url=https:%2F%2Fcontent.bibletranslationtools.org%2FWycliffeAssociates%2Fen_ulb'
+  )
+  await expect(page.getByText('English')).toBeVisible()
+  await expect(page.getByText('Genesis')).toBeVisible()
+  await expect(page.getByText('Deuteronomy')).toBeVisible()
+  await expect(page.getByText('(60) items hidden')).toBeVisible()
 })
 
 test('test es-419 resource types', async ({ page }) => {
-    await page.goto('http://localhost:8001/languages')
-    await page.getByText(/.*Español.*/).click()
-    await page.getByRole('button', { name: 'Next' }).click()
-    await page.getByText('Matthew').click()
-    await page.getByRole('button', { name: 'Next' }).click()
-    await page.getByLabel('Translation Notes').check()
-    await page.getByLabel('Translation Questions').check()
-    // await page.getByLabel('Translation Words').check()
-    await page.locator('span').filter({ hasText: 'Español Latin America (Latin American Spanish)' })
-    await page.getByRole('button', { name: 'Next' }).click()
-    await page.getByRole('button', { name: 'Generate File' }).click()
+  await page.goto('http://localhost:8001/languages')
+  await page.getByText(/.*Español.*/).click()
+  await page.getByRole('button', { name: 'Next' }).click()
+  await page.getByText('Matthew').click()
+  await page.getByRole('button', { name: 'Next' }).click()
+  await page.getByLabel('Translation Notes').check()
+  await page.getByLabel('Translation Questions').check()
+  // await page.getByLabel('Translation Words').check()
+  await page.locator('span').filter({ hasText: 'Español Latin America (Latin American Spanish)' })
+  await page.getByRole('button', { name: 'Next' }).click()
+  await page.getByRole('button', { name: 'Generate File' }).click()
 })