From 87da5d815be36e9d7d9a71228505b1783852f3a4 Mon Sep 17 00:00:00 2001 From: linearcombination <4829djaskdfj@gmail.com> Date: Fri, 13 Sep 2024 10:18:48 -0700 Subject: [PATCH 1/7] Fix docx book repeat issue There was a bug wherein when two books were chosen the resulting document would only include one of the books twice. This commit fixes that and adds an end to end test. --- ...ly_strategies_lang_then_book_by_chapter.py | 134 +++++++++--------- tests/e2e/test_api.py | 45 ++++++ 2 files changed, 111 insertions(+), 68 deletions(-) diff --git a/backend/document/domain/assembly_strategies_docx/assembly_strategies_lang_then_book_by_chapter.py b/backend/document/domain/assembly_strategies_docx/assembly_strategies_lang_then_book_by_chapter.py index 56483933..8813debe 100755 --- a/backend/document/domain/assembly_strategies_docx/assembly_strategies_lang_then_book_by_chapter.py +++ b/backend/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/tests/e2e/test_api.py b/tests/e2e/test_api.py index dc4913aa..1dd79e5c 100644 --- a/tests/e2e/test_api.py +++ b/tests/e2e/test_api.py @@ -2,6 +2,7 @@ import os import pathlib +import re import pytest import requests @@ -1650,3 +1651,47 @@ def test_en_bc_col_language_book_order_with_no_email_1c() -> None: }, ) check_finished_document_without_verses_success(response, suffix="pdf") + + +def test_en_ulb_1jn_en_ulb_3jn_language_book_order_with_no_email_1c() -> None: + with TestClient(app=app, base_url=settings.api_test_url()) as client: + response = client.post( + "/documents", + json={ + # "email_address": settings.TO_EMAIL_ADDRESS, + "assembly_strategy_kind": model.AssemblyStrategyEnum.LANGUAGE_BOOK_ORDER, + "assembly_layout_kind": model.AssemblyLayoutEnum.ONE_COLUMN, + "layout_for_print": False, + "chunk_size": model.ChunkSizeEnum.CHAPTER, + "generate_pdf": False, + "generate_epub": False, + "generate_docx": False, + "resource_requests": [ + { + "lang_code": "en", + "resource_type": "ulb", + "book_code": "1jn", + }, + { + "lang_code": "en", + "resource_type": "ulb", + "book_code": "3jn", + }, + ], + }, + ) + finished_document_request_key = check_result( + response, suffix="html", poll_duration=4 + ) + html_filepath = os.path.join( + settings.DOCUMENT_OUTPUT_DIR, + "{}.html".format(finished_document_request_key), + ) + with open(html_filepath, "r") as fin: + html = fin.read() + body_match = re.search(r"(.*?)", html, re.DOTALL) + assert body_match, "Body not found in HTML" + body_content = body_match.group(1) + assert ( + "1 John" in body_content and "3 John" in body_content + ), "Document should have had both 1 John and 3 John in it, but it didn't" From 40fceeeed47c6f7fbca6ab3b82e675d6942b914e Mon Sep 17 00:00:00 2001 From: linearcombination <4829djaskdfj@gmail.com> Date: Sat, 14 Sep 2024 12:37:58 -0700 Subject: [PATCH 2/7] Add source comment --- frontend/src/routes/transfer/[repo]/+page.svelte | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/frontend/src/routes/transfer/[repo]/+page.svelte b/frontend/src/routes/transfer/[repo]/+page.svelte index 6e2a2ad7..b54c6b88 100644 --- 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 From d9deb7c289a3faae6a7a9d291f29be248e796e5f Mon Sep 17 00:00:00 2001 From: linearcombination <4829djaskdfj@gmail.com> Date: Sat, 14 Sep 2024 12:44:20 -0700 Subject: [PATCH 3/7] Tweak to test to help github actions server pass test One test is failing on github actions server for some unknown reason while it tests fine locally. github actions server environment seems to have more issues than anywhere else. --- frontend/tests/e2e/test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/tests/e2e/test.ts b/frontend/tests/e2e/test.ts index 2b55ae19..8f397825 100644 --- a/frontend/tests/e2e/test.ts +++ b/frontend/tests/e2e/test.ts @@ -39,7 +39,7 @@ test('test ui part 2', async ({ page }) => { await page.getByText('Galatians').click() await page.getByRole('button', { name: 'Next' }).click() await page - .getByText(/.*Unlocked Literal Bible.*/) + .getByText("Unlocked Literal Bible") .first() .click() await page From 7591a1845b984d33aa1b780b88ad47bf982ff955 Mon Sep 17 00:00:00 2001 From: linearcombination <4829djaskdfj@gmail.com> Date: Sat, 14 Sep 2024 13:18:45 -0700 Subject: [PATCH 4/7] Put test back to what it was before last commit Still failed on github actions, so change it back. In next commit we will skip this test for github actions sake since it passes elsewhere. --- frontend/tests/e2e/test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/tests/e2e/test.ts b/frontend/tests/e2e/test.ts index 8f397825..2b55ae19 100644 --- a/frontend/tests/e2e/test.ts +++ b/frontend/tests/e2e/test.ts @@ -39,7 +39,7 @@ test('test ui part 2', async ({ page }) => { await page.getByText('Galatians').click() await page.getByRole('button', { name: 'Next' }).click() await page - .getByText("Unlocked Literal Bible") + .getByText(/.*Unlocked Literal Bible.*/) .first() .click() await page From 92686360c454ac9cb4647d7cab531e9a84ef16e4 Mon Sep 17 00:00:00 2001 From: linearcombination <4829djaskdfj@gmail.com> Date: Sat, 14 Sep 2024 13:19:48 -0700 Subject: [PATCH 5/7] Skip one front end test that only fails on github actions server --- frontend/tests/e2e/test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/tests/e2e/test.ts b/frontend/tests/e2e/test.ts index 2b55ae19..6e83e300 100644 --- a/frontend/tests/e2e/test.ts +++ b/frontend/tests/e2e/test.ts @@ -31,7 +31,7 @@ test('test ui part 1', async ({ page }) => { await expect(page.locator('body')).toContainText(/.*Unlocked Literal Bible.*/) }) -test('test ui part 2', async ({ page }) => { +test.skip('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() From eb4b3c69cdfd8c017dc55649d4f76b732cf42616 Mon Sep 17 00:00:00 2001 From: linearcombination <4829djaskdfj@gmail.com> Date: Sat, 14 Sep 2024 13:57:52 -0700 Subject: [PATCH 6/7] Code formatting by prettier --- frontend/tests/e2e/test.ts | 202 ++++++++++++++++++------------------- 1 file changed, 101 insertions(+), 101 deletions(-) diff --git a/frontend/tests/e2e/test.ts b/frontend/tests/e2e/test.ts index 6e83e300..47e1fafc 100644 --- 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.skip('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() }) From 1daaf386f87eb4a98b5d14cdb6e57c77e6961ffb Mon Sep 17 00:00:00 2001 From: linearcombination <4829djaskdfj@gmail.com> Date: Sat, 14 Sep 2024 14:13:00 -0700 Subject: [PATCH 7/7] Put frontend test back in play --- frontend/tests/e2e/test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/tests/e2e/test.ts b/frontend/tests/e2e/test.ts index 47e1fafc..f0c9d9f1 100644 --- a/frontend/tests/e2e/test.ts +++ b/frontend/tests/e2e/test.ts @@ -31,7 +31,7 @@ test('test ui part 1', async ({ page }) => { await expect(page.locator('body')).toContainText(/.*Unlocked Literal Bible.*/) }) -test.skip('test ui part 2', async ({ page }) => { +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()