fix: Change status to 200 and return the updated list after the PUT

decentraland · Jun 7, 2023 · ed90617 · ed90617
1 parent 017fd48
commit ed90617
Show file tree

Hide file tree

Showing 4 changed files with 78 additions and 12 deletions.
diff --git a/src/controllers/handlers/lists-handlers.ts b/src/controllers/handlers/lists-handlers.ts
@@ -591,7 +591,7 @@ export async function updateListHandler(
     const updateListResult = await lists.updateList(params.id, userAddress, body)
 
     return {
-      status: StatusCode.UPDATED,
+      status: StatusCode.OK,
       body: {
         ok: true,
         data: fromDBListToList(updateListResult)

diff --git a/src/ports/lists/component.ts b/src/ports/lists/component.ts
@@ -190,12 +190,10 @@ export function createListsComponent(components: Pick<AppComponents, 'pg' | 'sna
 
         updateQuery.append(SQL` WHERE id = ${id} AND user_address = ${userAddress} RETURNING *`)
 
-        const [updatedListResult] = await Promise.all([
-          client.query<DBList>(shouldUpdate ? updateQuery : getListQuery(id, { userAddress })),
-          client.query(accessQuery)
-        ])
+        const [updateResult] = await Promise.all([shouldUpdate && client.query<DBList>(updateQuery), client.query(accessQuery)])
+        const updatedListResult = await client.query<DBList>(getListQuery(id, { userAddress }))
 
-        validateListExists(id, updatedListResult)
+        validateListExists(id, updateResult || updatedListResult)
 
         return updatedListResult.rows[0]
       },

diff --git a/test/unit/lists-component.spec.ts b/test/unit/lists-component.spec.ts
@@ -1095,6 +1095,9 @@ describe('when updating a list', () => {
 
       // Access Mock Query
       dbClientQueryMock.mockResolvedValueOnce(undefined)
+
+      // Get Updated List Mock Query
+      dbClientQueryMock.mockResolvedValueOnce({ rowCount: 0 })
     })
 
     it('should throw a list not found error', async () => {
@@ -1156,6 +1159,9 @@ describe('when updating a list', () => {
 
         // Delete Access Mock Query
         dbClientQueryMock.mockResolvedValueOnce({ rowCount: 0 })
+
+        // Get Updated List Mock Query
+        dbClientQueryMock.mockResolvedValueOnce({ rowCount: 1, rows: [updatedList] })
       })
 
       it('should not throw an error because this means the list is already private', () => {
@@ -1188,6 +1194,12 @@ describe('when updating a list', () => {
 
         // Delete Access Mock Query
         dbClientQueryMock.mockResolvedValueOnce({ rowCount: 1 })
+
+        // Get Updated List Mock Query
+        dbClientQueryMock.mockResolvedValueOnce({
+          rowCount: 1,
+          rows: [dbList]
+        })
       })
 
       describe('and the updated list has only an updated name without a new description', () => {
@@ -1347,6 +1359,12 @@ describe('when updating a list', () => {
 
         // Delete Access Mock Query
         dbClientQueryMock.mockResolvedValueOnce({ rowCount: 1 })
+
+        // Get Updated List Mock Query
+        dbClientQueryMock.mockResolvedValueOnce({
+          rowCount: 1,
+          rows: [dbList]
+        })
       })
 
       describe('and the updated list has only an updated name without a new description', () => {
@@ -1412,6 +1430,31 @@ describe('when updating a list', () => {
           )
         })
 
+        it('should get the updated list with its last privacy configuration', () => {
+          expect(dbClientQueryMock).toHaveBeenCalledWith(
+            expect.objectContaining({
+              strings: expect.arrayContaining([
+                expect.stringContaining(
+                  'SELECT favorites.lists.id, favorites.lists.name, favorites.lists.description, favorites.lists.user_address, favorites.lists.created_at, favorites.lists.updated_at, favorites.acl.permission AS permission, COUNT(favorites.picks.item_id) AS items_count, COUNT(favorites.acl.permission) = 0 AS is_private'
+                ),
+                expect.stringContaining('FROM favorites.lists'),
+                expect.stringContaining(
+                  'LEFT JOIN favorites.picks ON favorites.lists.id = favorites.picks.list_id AND (favorites.picks.user_address ='
+                ),
+                expect.stringContaining('OR favorites.picks.user_address = favorites.lists.user_address)'),
+                expect.stringContaining('LEFT JOIN favorites.acl ON favorites.lists.id = favorites.acl.list_id'),
+                expect.stringContaining('WHERE favorites.lists.id ='),
+                expect.stringContaining('(favorites.lists.user_address ='),
+                expect.stringContaining('OR favorites.lists.user_address ='),
+                expect.stringContaining(')'),
+                expect.stringContaining('GROUP BY favorites.lists.id, favorites.acl.permission'),
+                expect.stringContaining('ORDER BY favorites.acl.permission ASC LIMIT 1')
+              ]),
+              values: [userAddress, listId, userAddress, DEFAULT_LIST_USER_ADDRESS]
+            })
+          )
+        })
+
         it('should resolve with the updated list', () => {
           expect(result).toEqual(dbList)
         })
@@ -1447,6 +1490,31 @@ describe('when updating a list', () => {
           )
         })
 
+        it('should get the updated list with its last privacy configuration', () => {
+          expect(dbClientQueryMock).toHaveBeenCalledWith(
+            expect.objectContaining({
+              strings: expect.arrayContaining([
+                expect.stringContaining(
+                  'SELECT favorites.lists.id, favorites.lists.name, favorites.lists.description, favorites.lists.user_address, favorites.lists.created_at, favorites.lists.updated_at, favorites.acl.permission AS permission, COUNT(favorites.picks.item_id) AS items_count, COUNT(favorites.acl.permission) = 0 AS is_private'
+                ),
+                expect.stringContaining('FROM favorites.lists'),
+                expect.stringContaining(
+                  'LEFT JOIN favorites.picks ON favorites.lists.id = favorites.picks.list_id AND (favorites.picks.user_address ='
+                ),
+                expect.stringContaining('OR favorites.picks.user_address = favorites.lists.user_address)'),
+                expect.stringContaining('LEFT JOIN favorites.acl ON favorites.lists.id = favorites.acl.list_id'),
+                expect.stringContaining('WHERE favorites.lists.id ='),
+                expect.stringContaining('(favorites.lists.user_address ='),
+                expect.stringContaining('OR favorites.lists.user_address ='),
+                expect.stringContaining(')'),
+                expect.stringContaining('GROUP BY favorites.lists.id, favorites.acl.permission'),
+                expect.stringContaining('ORDER BY favorites.acl.permission ASC LIMIT 1')
+              ]),
+              values: [userAddress, listId, userAddress, DEFAULT_LIST_USER_ADDRESS]
+            })
+          )
+        })
+
         it('should resolve with the updated list', () => {
           expect(result).toEqual(dbList)
         })
@@ -1475,19 +1543,19 @@ describe('when updating a list', () => {
         is_private: false
       }
 
-      // Update List Mock Query
+      // Delete Access Mock Query
+      dbClientQueryMock.mockResolvedValueOnce({ rowCount: 1 })
+
+      // Get Updated List Mock Query
       dbClientQueryMock.mockResolvedValueOnce({
         rowCount: 1,
         rows: [dbList]
       })
 
-      // Delete Access Mock Query
-      dbClientQueryMock.mockResolvedValueOnce({ rowCount: 1 })
-
       result = await listsComponent.updateList(listId, userAddress, updatedList)
     })
 
-    it('should get the list instead of updating it', () => {
+    it('should only get the list instead of updating it', () => {
       expect(dbClientQueryMock).toHaveBeenCalledWith(
         expect.objectContaining({
           strings: expect.arrayContaining([

diff --git a/test/unit/lists-handler.spec.ts b/test/unit/lists-handler.spec.ts
@@ -1435,7 +1435,7 @@ describe('when updating a list', () => {
 
     it('should convert the updated database list into a list and return it with the status 204', () => {
       return expect(result).resolves.toEqual({
-        status: StatusCode.UPDATED,
+        status: StatusCode.OK,
         body: {
           ok: true,
           data: {