From 87d5e6aaaf8e6de9b846188fc0518ffb7d5a9fec Mon Sep 17 00:00:00 2001 From: Kevin Szuchet Date: Tue, 23 May 2023 12:31:26 +0200 Subject: [PATCH 1/2] refactor: Remove unnecessary DISTINCT clauses --- src/ports/lists/component.ts | 4 ++-- test/unit/lists-component.spec.ts | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ports/lists/component.ts b/src/ports/lists/component.ts index 03e4eeb..c4ad604 100644 --- a/src/ports/lists/component.ts +++ b/src/ports/lists/component.ts @@ -50,7 +50,7 @@ export function createListsComponent( { requiredPermission, considerDefaultList = true, userAddress }: GetListOptions ): Promise { const getListQuery = SQL` - SELECT DISTINCT favorites.lists.*, favorites.acl.permission AS permission, COUNT(DISTINCT favorites.picks.item_id) AS count_items + SELECT favorites.lists.*, favorites.acl.permission AS permission, COUNT(favorites.picks.item_id) AS count_items FROM favorites.lists LEFT JOIN favorites.picks ON favorites.lists.id = favorites.picks.list_id AND favorites.picks.user_address = ${userAddress} LEFT JOIN favorites.acl ON favorites.lists.id = favorites.acl.list_id` @@ -156,7 +156,7 @@ export function createListsComponent( async function getLists(params: GetListsParameters): Promise { const { userAddress, limit, offset, sortBy = ListSortBy.CREATED_AT, sortDirection = ListSortDirection.DESC } = params const query = SQL` - SELECT l.*, COUNT(*) OVER() as lists_count, l.user_address = ${DEFAULT_LIST_USER_ADDRESS} as is_default_list, COUNT(DISTINCT p.item_id) AS items_count + SELECT l.*, COUNT(*) OVER() as lists_count, l.user_address = ${DEFAULT_LIST_USER_ADDRESS} as is_default_list, COUNT(p.item_id) AS items_count FROM favorites.lists l LEFT JOIN favorites.picks p ON l.id = p.list_id AND p.user_address = ${userAddress} WHERE l.user_address = ${userAddress} OR l.user_address = ${DEFAULT_LIST_USER_ADDRESS} diff --git a/test/unit/lists-component.spec.ts b/test/unit/lists-component.spec.ts index aa1a462..70ca79d 100644 --- a/test/unit/lists-component.spec.ts +++ b/test/unit/lists-component.spec.ts @@ -611,7 +611,7 @@ describe('when getting a list', () => { expect(dbQueryMock).toHaveBeenCalledWith( expect.objectContaining({ text: expect.stringContaining( - 'SELECT DISTINCT favorites.lists.*, favorites.acl.permission AS permission, COUNT(DISTINCT favorites.picks.item_id) AS count_items' + 'SELECT favorites.lists.*, favorites.acl.permission AS permission, COUNT(favorites.picks.item_id) AS count_items' ) }) ) @@ -677,7 +677,7 @@ describe('when getting a list', () => { expect(dbQueryMock).toHaveBeenCalledWith( expect.objectContaining({ text: expect.stringContaining( - 'SELECT DISTINCT favorites.lists.*, favorites.acl.permission AS permission, COUNT(DISTINCT favorites.picks.item_id) AS count_items' + 'SELECT favorites.lists.*, favorites.acl.permission AS permission, COUNT(favorites.picks.item_id) AS count_items' ) }) ) @@ -757,7 +757,7 @@ describe('when getting a list', () => { expect(dbQueryMock).toHaveBeenCalledWith( expect.objectContaining({ text: expect.stringContaining( - 'SELECT DISTINCT favorites.lists.*, favorites.acl.permission AS permission, COUNT(DISTINCT favorites.picks.item_id) AS count_items' + 'SELECT favorites.lists.*, favorites.acl.permission AS permission, COUNT(favorites.picks.item_id) AS count_items' ) }) ) @@ -823,7 +823,7 @@ describe('when getting a list', () => { expect(dbQueryMock).toHaveBeenCalledWith( expect.objectContaining({ text: expect.stringContaining( - 'SELECT DISTINCT favorites.lists.*, favorites.acl.permission AS permission, COUNT(DISTINCT favorites.picks.item_id) AS count_items' + 'SELECT favorites.lists.*, favorites.acl.permission AS permission, COUNT(favorites.picks.item_id) AS count_items' ) }) ) From e0ccae2ee1866ad527784cad652634dbb5c0db80 Mon Sep 17 00:00:00 2001 From: Kevin Szuchet Date: Tue, 23 May 2023 12:34:53 +0200 Subject: [PATCH 2/2] fix: Limit the rows when getting a list sorting them by permission --- src/ports/lists/component.ts | 1 + test/unit/lists-component.spec.ts | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/src/ports/lists/component.ts b/src/ports/lists/component.ts index c4ad604..53e0a97 100644 --- a/src/ports/lists/component.ts +++ b/src/ports/lists/component.ts @@ -71,6 +71,7 @@ export function createListsComponent( } getListQuery.append(SQL` GROUP BY favorites.lists.id, favorites.acl.permission`) + getListQuery.append(SQL` ORDER BY favorites.acl.permission ASC LIMIT 1`) const result = await pg.query(getListQuery) diff --git a/test/unit/lists-component.spec.ts b/test/unit/lists-component.spec.ts index 70ca79d..ed4302a 100644 --- a/test/unit/lists-component.spec.ts +++ b/test/unit/lists-component.spec.ts @@ -649,6 +649,12 @@ describe('when getting a list', () => { text: expect.stringContaining('GROUP BY favorites.lists.id, favorites.acl.permission') }) ) + + expect(dbQueryMock).toHaveBeenCalledWith( + expect.objectContaining({ + text: expect.stringContaining('ORDER BY favorites.acl.permission ASC LIMIT 1') + }) + ) }) it('should resolve with the list', () => { @@ -717,6 +723,12 @@ describe('when getting a list', () => { text: expect.stringContaining('GROUP BY favorites.lists.id, favorites.acl.permission') }) ) + + expect(dbQueryMock).toHaveBeenCalledWith( + expect.objectContaining({ + text: expect.stringContaining('ORDER BY favorites.acl.permission ASC LIMIT 1') + }) + ) }) it('should resolve with the list', () => { @@ -797,6 +809,12 @@ describe('when getting a list', () => { text: expect.stringContaining('GROUP BY favorites.lists.id, favorites.acl.permission') }) ) + + expect(dbQueryMock).toHaveBeenCalledWith( + expect.objectContaining({ + text: expect.stringContaining('ORDER BY favorites.acl.permission ASC LIMIT 1') + }) + ) }) it('should resolve with the list', () => { @@ -870,6 +888,12 @@ describe('when getting a list', () => { text: expect.stringContaining('GROUP BY favorites.lists.id, favorites.acl.permission') }) ) + + expect(dbQueryMock).toHaveBeenCalledWith( + expect.objectContaining({ + text: expect.stringContaining('ORDER BY favorites.acl.permission ASC LIMIT 1') + }) + ) }) it('should resolve with the list', () => {