From fb9e69d5080f0d5bab98f24c3253d9f4af5db565 Mon Sep 17 00:00:00 2001 From: Anthony LC Date: Fri, 23 Aug 2024 13:20:19 +0200 Subject: [PATCH 1/3] =?UTF-8?q?=E2=99=BB=EF=B8=8F(backend)=20document=20li?= =?UTF-8?q?st=20order=20by=20updated=5Fat=20desc?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Document list is now ordered by updated_at in descending order. Test cases were improved as well. --- CHANGELOG.md | 5 + src/backend/core/api/viewsets.py | 1 + .../documents/test_api_documents_list.py | 201 +++++++----------- 3 files changed, 77 insertions(+), 130 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b8427adae..f12d833b67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ and this project adheres to ## [Unreleased] +## Changed + +- ♻️ Change ordering docs datagrid #195 + + ## [1.2.0] - 2024-08-06 ## Added diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index 31ccf82312..04d860b3f4 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -310,6 +310,7 @@ class DocumentViewSet( access_model_class = models.DocumentAccess resource_field_name = "document" queryset = models.Document.objects.all() + ordering = ["-updated_at"] def perform_create(self, serializer): """ diff --git a/src/backend/core/tests/documents/test_api_documents_list.py b/src/backend/core/tests/documents/test_api_documents_list.py index 30d8febd01..26adb8c375 100644 --- a/src/backend/core/tests/documents/test_api_documents_list.py +++ b/src/backend/core/tests/documents/test_api_documents_list.py @@ -169,87 +169,26 @@ def test_api_documents_list_authenticated_distinct(): assert content["results"][0]["id"] == str(document.id) -def test_api_documents_order_created_at_desc(): +def test_api_documents_order_updated_at_desc_default(): """ - Test that the endpoint GET documents is sorted in 'created_at' descending order by default. - """ - user = factories.UserFactory() - client = APIClient() - client.force_login(user) - - documents_created = [ - document.created_at.isoformat().replace("+00:00", "Z") - for document in factories.DocumentFactory.create_batch(5, is_public=True) - ] - - documents_created.sort(reverse=True) - - response = client.get( - "/api/v1.0/documents/", - ) - - assert response.status_code == 200 - - response_data = response.json() - response_document_created = [ - document["created_at"] for document in response_data["results"] - ] - - assert ( - response_document_created == documents_created - ), "created_at values are not sorted from newest to oldest" - - -def test_api_documents_order_created_at_asc(): - """ - Test that the 'created_at' field is sorted in ascending order - when the 'ordering' query parameter is set. - """ - user = factories.UserFactory() - client = APIClient() - client.force_login(user) - - documents_created = [ - document.created_at.isoformat().replace("+00:00", "Z") - for document in factories.DocumentFactory.create_batch(5, is_public=True) - ] - - documents_created.sort() - - response = client.get( - "/api/v1.0/documents/?ordering=created_at", - ) - - assert response.status_code == 200 - - response_data = response.json() - response_document_created = [ - document["created_at"] for document in response_data["results"] - ] - - assert ( - response_document_created == documents_created - ), "created_at values are not sorted from oldest to newest" - - -def test_api_documents_order_updated_at_desc(): - """ - Test that the 'updated_at' field is sorted in descending order - when the 'ordering' query parameter is set. + Test that the endpoint GET documents is sorted in 'updated_at' descending order by default. """ user = factories.UserFactory() client = APIClient() client.force_login(user) + # Updated at next year to ensure the order is correct documents_updated = [ document.updated_at.isoformat().replace("+00:00", "Z") - for document in factories.DocumentFactory.create_batch(5, is_public=True) + for document in factories.DocumentFactory.create_batch( + 5, is_public=True, updated_at=fake.date_time_this_year(before_now=False) + ) ] documents_updated.sort(reverse=True) response = APIClient().get( - "/api/v1.0/documents/?ordering=-updated_at", + "/api/v1.0/documents/", ) assert response.status_code == 200 @@ -264,97 +203,99 @@ def test_api_documents_order_updated_at_desc(): ), "updated_at values are not sorted from newest to oldest" -def test_api_documents_order_updated_at_asc(): - """ - Test that the 'updated_at' field is sorted in ascending order - when the 'ordering' query parameter is set. - """ - user = factories.UserFactory() - client = APIClient() - client.force_login(user) - - documents_updated = [ - document.updated_at.isoformat().replace("+00:00", "Z") - for document in factories.DocumentFactory.create_batch(5, is_public=True) - ] - - documents_updated.sort() - - response = APIClient().get( - "/api/v1.0/documents/?ordering=updated_at", - ) - assert response.status_code == 200 - - response_data = response.json() - - response_document_updated = [ - document["updated_at"] for document in response_data["results"] - ] - - assert ( - response_document_updated == documents_updated - ), "updated_at values are not sorted from oldest to newest" - - -def test_api_documents_order_title_desc(): +@pytest.mark.parametrize( + "ordering_field, factory_field", + [ + ("-created_at", "created_at"), + ("-updated_at", "updated_at"), + ("-title", "title"), + ], +) +def test_api_documents_ordering_desc(ordering_field, factory_field): """ - Test that the 'title' field is sorted in descending order + Test that the specified field is sorted in descending order when the 'ordering' query parameter is set. """ user = factories.UserFactory() client = APIClient() client.force_login(user) - documents_title = [ - factories.DocumentFactory(is_public=True, title=fake.sentence(nb_words=4)).title - for _ in range(5) - ] - - documents_title.sort(reverse=True) + if factory_field == "title": + documents_field_values = [ + factories.DocumentFactory( + is_public=True, title=fake.sentence(nb_words=4) + ).title + for _ in range(5) + ] + else: + documents_field_values = [ + getattr(document, factory_field).isoformat().replace("+00:00", "Z") + for document in factories.DocumentFactory.create_batch(5, is_public=True) + ] + + documents_field_values.sort(reverse=True) - response = APIClient().get( - "/api/v1.0/documents/?ordering=-title", + response = client.get( + f"/api/v1.0/documents/?ordering={ordering_field}" + if ordering_field != "-created_at" + else "/api/v1.0/documents/", ) assert response.status_code == 200 response_data = response.json() - response_documents_title = [ - document["title"] for document in response_data["results"] + response_documents_field_values = [ + document[factory_field] for document in response_data["results"] ] assert ( - response_documents_title == documents_title - ), "title values are not sorted descending" - - -def test_api_documents_order_title_asc(): + response_documents_field_values == documents_field_values + ), f"{factory_field} values are not sorted as expected" + + +@pytest.mark.parametrize( + "field", + [ + ("updated_at"), + ("title"), + ("created_at"), + ], +) +def test_api_documents_ordering_asc(field): """ - Test that the 'title' field is sorted in ascending order + Test that the specified field is sorted in ascending order when the 'ordering' query parameter is set. """ user = factories.UserFactory() client = APIClient() client.force_login(user) - documents_title = [ - factories.DocumentFactory(is_public=True, title=fake.sentence(nb_words=4)).title - for _ in range(5) - ] - - documents_title.sort() + if field == "title": + documents_field_values = [ + factories.DocumentFactory( + is_public=True, title=fake.sentence(nb_words=4) + ).title + for _ in range(5) + ] + else: + documents_field_values = [ + getattr(document, field).isoformat().replace("+00:00", "Z") + for document in factories.DocumentFactory.create_batch(5, is_public=True) + ] + + documents_field_values.sort() - response = APIClient().get( - "/api/v1.0/documents/?ordering=title", + response = client.get( + f"/api/v1.0/documents/?ordering={field}", ) assert response.status_code == 200 response_data = response.json() - response_documents_title = [ - document["title"] for document in response_data["results"] + response_documents_field_values = [ + document[field] for document in response_data["results"] ] assert ( - response_documents_title == documents_title - ), "title values are not sorted ascending" + response_documents_field_values == documents_field_values + ), f"{field} values are not sorted as expected" From 628c536d31a0e97c3cc80ad0c88aa4e290dfa1e4 Mon Sep 17 00:00:00 2001 From: Anthony LC Date: Fri, 23 Aug 2024 13:20:32 +0200 Subject: [PATCH 2/3] =?UTF-8?q?=E2=99=BB=EF=B8=8F(frontend)=20datagrid=20o?= =?UTF-8?q?rdered=20by=20updated=5Fat=20desc?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The datagrid is now ordered by updated_at desc. --- .../__tests__/app-impress/doc-grid.spec.ts | 207 ++++++++++-------- .../docs/docs-grid/components/DocsGrid.tsx | 7 +- 2 files changed, 119 insertions(+), 95 deletions(-) diff --git a/src/frontend/apps/e2e/__tests__/app-impress/doc-grid.spec.ts b/src/frontend/apps/e2e/__tests__/app-impress/doc-grid.spec.ts index d7a1b4ab79..35eec8a3df 100644 --- a/src/frontend/apps/e2e/__tests__/app-impress/doc-grid.spec.ts +++ b/src/frontend/apps/e2e/__tests__/app-impress/doc-grid.spec.ts @@ -49,111 +49,130 @@ test.describe('Documents Grid', () => { nameColumn: 'Document name', ordering: 'title', cellNumber: 1, + orderDefault: '', + orderDesc: '&ordering=-title', + orderAsc: '&ordering=title', }, { nameColumn: 'Created at', ordering: 'created_at', cellNumber: 2, + orderDefault: '', + orderDesc: '&ordering=-created_at', + orderAsc: '&ordering=created_at', }, { nameColumn: 'Updated at', ordering: 'updated_at', cellNumber: 3, + orderDefault: '&ordering=-updated_at', + orderDesc: '&ordering=updated_at', + orderAsc: '', }, - ].forEach(({ nameColumn, ordering, cellNumber }) => { - test(`checks datagrid ordering ${ordering}`, async ({ page }) => { - const responsePromise = page.waitForResponse( - (response) => - response.url().includes(`/documents/?page=1`) && - response.status() === 200, - ); - - const responsePromiseOrderingDesc = page.waitForResponse( - (response) => - response.url().includes(`/documents/?page=1&ordering=-${ordering}`) && - response.status() === 200, - ); - - const responsePromiseOrderingAsc = page.waitForResponse( - (response) => - response.url().includes(`/documents/?page=1&ordering=${ordering}`) && - response.status() === 200, - ); - - const datagrid = page - .getByLabel('Datagrid of the documents page 1') - .getByRole('table'); - const thead = datagrid.locator('thead'); - - const response = await responsePromise; - expect(response.ok()).toBeTruthy(); - - const docNameRow1 = datagrid - .getByRole('row') - .nth(1) - .getByRole('cell') - .nth(cellNumber); - const docNameRow2 = datagrid - .getByRole('row') - .nth(2) - .getByRole('cell') - .nth(cellNumber); - - await expect(datagrid.getByLabel('Loading data')).toBeHidden(); - - // Initial state - await expect(docNameRow1).toHaveText(/.*/); - await expect(docNameRow2).toHaveText(/.*/); - const initialDocNameRow1 = await docNameRow1.textContent(); - const initialDocNameRow2 = await docNameRow2.textContent(); - - expect(initialDocNameRow1).toBeDefined(); - expect(initialDocNameRow2).toBeDefined(); - - // Ordering ASC - await thead.getByText(nameColumn).click(); - - const responseOrderingAsc = await responsePromiseOrderingAsc; - expect(responseOrderingAsc.ok()).toBeTruthy(); - - await expect(datagrid.getByLabel('Loading data')).toBeHidden(); - - await expect(docNameRow1).toHaveText(/.*/); - await expect(docNameRow2).toHaveText(/.*/); - const textDocNameRow1Asc = await docNameRow1.textContent(); - const textDocNameRow2Asc = await docNameRow2.textContent(); - expect( - textDocNameRow1Asc && - textDocNameRow2Asc && - textDocNameRow1Asc.localeCompare(textDocNameRow2Asc, 'en', { - caseFirst: 'false', - ignorePunctuation: true, - }) <= 0, - ).toBeTruthy(); - - // Ordering Desc - await thead.getByText(nameColumn).click(); - - const responseOrderingDesc = await responsePromiseOrderingDesc; - expect(responseOrderingDesc.ok()).toBeTruthy(); - - await expect(datagrid.getByLabel('Loading data')).toBeHidden(); - - await expect(docNameRow1).toHaveText(/.*/); - await expect(docNameRow2).toHaveText(/.*/); - const textDocNameRow1Desc = await docNameRow1.textContent(); - const textDocNameRow2Desc = await docNameRow2.textContent(); - - expect( - textDocNameRow1Desc && - textDocNameRow2Desc && - textDocNameRow1Desc.localeCompare(textDocNameRow2Desc, 'en', { - caseFirst: 'false', - ignorePunctuation: true, - }) >= 0, - ).toBeTruthy(); - }); - }); + ].forEach( + ({ + nameColumn, + ordering, + cellNumber, + orderDefault, + orderDesc, + orderAsc, + }) => { + test(`checks datagrid ordering ${ordering}`, async ({ page }) => { + const responsePromise = page.waitForResponse( + (response) => + response.url().includes(`/documents/?page=1${orderDefault}`) && + response.status() === 200, + ); + + const responsePromiseOrderingDesc = page.waitForResponse( + (response) => + response.url().includes(`/documents/?page=1${orderDesc}`) && + response.status() === 200, + ); + + const responsePromiseOrderingAsc = page.waitForResponse( + (response) => + response.url().includes(`/documents/?page=1${orderAsc}`) && + response.status() === 200, + ); + + // Checks the initial state + const datagrid = page + .getByLabel('Datagrid of the documents page 1') + .getByRole('table'); + const thead = datagrid.locator('thead'); + + const response = await responsePromise; + expect(response.ok()).toBeTruthy(); + + const docNameRow1 = datagrid + .getByRole('row') + .nth(1) + .getByRole('cell') + .nth(cellNumber); + const docNameRow2 = datagrid + .getByRole('row') + .nth(2) + .getByRole('cell') + .nth(cellNumber); + + await expect(datagrid.getByLabel('Loading data')).toBeHidden(); + + // Initial state + await expect(docNameRow1).toHaveText(/.*/); + await expect(docNameRow2).toHaveText(/.*/); + const initialDocNameRow1 = await docNameRow1.textContent(); + const initialDocNameRow2 = await docNameRow2.textContent(); + + expect(initialDocNameRow1).toBeDefined(); + expect(initialDocNameRow2).toBeDefined(); + + // Ordering ASC + await thead.getByText(nameColumn).click(); + + const responseOrderingAsc = await responsePromiseOrderingAsc; + expect(responseOrderingAsc.ok()).toBeTruthy(); + + await expect(datagrid.getByLabel('Loading data')).toBeHidden(); + + await expect(docNameRow1).toHaveText(/.*/); + await expect(docNameRow2).toHaveText(/.*/); + const textDocNameRow1Asc = await docNameRow1.textContent(); + const textDocNameRow2Asc = await docNameRow2.textContent(); + expect( + textDocNameRow1Asc && + textDocNameRow2Asc && + textDocNameRow1Asc.localeCompare(textDocNameRow2Asc, 'en', { + caseFirst: 'false', + ignorePunctuation: true, + }) <= 0, + ).toBeTruthy(); + + // Ordering Desc + await thead.getByText(nameColumn).click(); + + const responseOrderingDesc = await responsePromiseOrderingDesc; + expect(responseOrderingDesc.ok()).toBeTruthy(); + + await expect(datagrid.getByLabel('Loading data')).toBeHidden(); + + await expect(docNameRow1).toHaveText(/.*/); + await expect(docNameRow2).toHaveText(/.*/); + const textDocNameRow1Desc = await docNameRow1.textContent(); + const textDocNameRow2Desc = await docNameRow2.textContent(); + + expect( + textDocNameRow1Desc && + textDocNameRow2Desc && + textDocNameRow1Desc.localeCompare(textDocNameRow2Desc, 'en', { + caseFirst: 'false', + ignorePunctuation: true, + }) >= 0, + ).toBeTruthy(); + }); + }, + ); test('checks the pagination', async ({ page }) => { const responsePromisePage1 = page.waitForResponse( diff --git a/src/frontend/apps/impress/src/features/docs/docs-grid/components/DocsGrid.tsx b/src/frontend/apps/impress/src/features/docs/docs-grid/components/DocsGrid.tsx index 54e7dd6bee..90fb9d68b6 100644 --- a/src/frontend/apps/impress/src/features/docs/docs-grid/components/DocsGrid.tsx +++ b/src/frontend/apps/impress/src/features/docs/docs-grid/components/DocsGrid.tsx @@ -53,7 +53,12 @@ export const DocsGrid = () => { const pagination = usePagination({ pageSize: PAGE_SIZE, }); - const [sortModel, setSortModel] = useState([]); + const [sortModel, setSortModel] = useState([ + { + field: 'updated_at', + sort: 'desc', + }, + ]); const { page, pageSize, setPagesCount } = pagination; const [docs, setDocs] = useState([]); From ce91a22763b722397dc420ddb4c36964a9488ec8 Mon Sep 17 00:00:00 2001 From: Anthony LC Date: Fri, 23 Aug 2024 13:21:26 +0200 Subject: [PATCH 3/3] =?UTF-8?q?=F0=9F=93=8C(e2e)=20pin=20pdf-parse=20to=20?= =?UTF-8?q?1.1.1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pdf-parse was not pinned to a specific version. This could lead to unexpected behavior if the package is updated. This change pins pdf-parse to version 1.1.1. --- src/frontend/apps/e2e/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/frontend/apps/e2e/package.json b/src/frontend/apps/e2e/package.json index 3632295c93..edd7925f4a 100644 --- a/src/frontend/apps/e2e/package.json +++ b/src/frontend/apps/e2e/package.json @@ -21,6 +21,6 @@ "dependencies": { "convert-stream": "1.0.2", "jsdom": "24.1.1", - "pdf-parse": "^1.1.1" + "pdf-parse": "1.1.1" } }