chore: Add item tests to ensure that the middelware works. Fix middle…

…are for items. Add new test utils.

spec/utils.ts

@@ -2,11 +2,15 @@ import { Authenticator, AuthIdentity } from 'dcl-crypto'
 import { Model, QueryPart } from 'decentraland-server'
 import { env } from 'decentraland-commons'
 import { isManager } from '../src/ethereum/api/tpw'
+import { collectionAPI } from '../src/ethereum/api/collection'
+import { isPublished } from '../src/utils/eth'
 import { AUTH_CHAIN_HEADER_PREFIX } from '../src/middleware/authentication'
 import { Collection } from '../src/Collection/Collection.model'
 import { collectionAttributesMock } from './mocks/collections'
 import { wallet } from './mocks/wallet'
 import { Ownable } from '../src/Ownable/Ownable'
+import { Item } from '../src/Item/Item.model'
+import { dbItemMock } from './mocks/items'
 
 export function buildURL(
   uri: string,
@@ -106,14 +110,16 @@ export function mockCollectionAuthorizationMiddleware(
     throw new Error('Collection.findOne is not mocked')
   }
 
-  if (isThirdParty && !(isManager as jest.Mock).mock) {
-    throw new Error('isManager is not mocked')
-  }
-
   ;(Collection.findOne as jest.Mock).mockImplementationOnce((givenId) =>
-    givenId === id && isAuthorized ? collectionToReturn : undefined
+    Promise.resolve(
+      givenId === id && isAuthorized ? collectionToReturn : undefined
+    )
   )
   if (isThirdParty) {
+    if (!(isManager as jest.Mock).mock) {
+      throw new Error('isManager is not mocked')
+    }
+
     ;(isManager as jest.MockedFunction<typeof isManager>).mockResolvedValueOnce(
       isAuthorized
     )
@@ -136,27 +142,21 @@ export function mockItemAuthorizationMiddleware(
   isThirdParty = false,
   isAuthorized = true
 ) {
-  const collectionToReturn = {
-    ...collectionAttributesMock,
+  const itemToReturn = {
+    ...dbItemMock,
     urn_suffix: isThirdParty ? 'third-party' : null,
     eth_address,
   }
 
-  if (!(Collection.findByOwnerOfItem as jest.Mock).mock) {
-    throw new Error('Collection.findByOwnerOfItem is not mocked')
-  }
-
-  if (isThirdParty && !(isManager as jest.Mock).mock) {
-    throw new Error('isManager is not mocked')
-  }
-
-  ;(Collection.findByOwnerOfItem as jest.Mock).mockImplementationOnce(
-    (givenId) =>
-      givenId === id && isAuthorized ? collectionToReturn : undefined
+  ;(Item.findOne as jest.Mock).mockImplementationOnce((givenId) =>
+    Promise.resolve(givenId === id && isAuthorized ? itemToReturn : undefined)
   )
 
   if (isThirdParty) {
-    ;(isManager as jest.MockedFunction<typeof isManager>).mockResolvedValueOnce(
+    mockCollectionAuthorizationMiddleware(
+      dbItemMock.collection_id!,
+      eth_address,
+      isThirdParty,
       isAuthorized
     )
   }
@@ -199,3 +199,16 @@ export function mockOwnableCanUpsert(
       Promise.resolve(canUpsert(conditions) ? 1 : 0)
     )
 }
+
+export function mockIsCollectionPublished(
+  id: string,
+  isCollectionPublished: boolean
+) {
+  ;(collectionAPI.fetchCollection as jest.Mock).mockImplementationOnce(
+    (givenId) =>
+      Promise.resolve(id === givenId && isCollectionPublished ? {} : undefined)
+  )
+  if (isCollectionPublished) {
+    ;(isPublished as jest.Mock).mockResolvedValueOnce(isCollectionPublished)
+  }
+}

src/Collection/Collection.model.ts

@@ -1,6 +1,4 @@
 import { Model, raw, SQL } from 'decentraland-server'
-
-import { Item } from '../Item/Item.model'
 import { CollectionAttributes } from './Collection.types'
 
 export class Collection extends Model<CollectionAttributes> {
@@ -20,22 +18,6 @@ export class Collection extends Model<CollectionAttributes> {
       WHERE id = ANY(${ids})`)
   }
 
-  static async findByOwnerOfItem(
-    itemId: string
-  ): Promise<CollectionAttributes> {
-    const collections = await this.query<CollectionAttributes>(SQL`
-    SELECT *
-      FROM ${raw(this.tableName)}
-      WHERE ${raw(this.tableName)}.id IN (SELECT collection_id FROM ${raw(
-      Item.tableName
-    )}) WHERE ${raw(Item.tableName)}.id = ${itemId}) LIMIT 1`)
-
-    if (collections.length === 0) {
-      throw new Error('Collection not found')
-    }
-    return collections[0]
-  }
-
   /**
    * Checks if a collection name is valid.
    * A collection name is valid if there's no other collection that has the given

src/Collection/Collection.router.spec.ts

@@ -5,6 +5,7 @@ import {
   mockExistsMiddleware,
   mockOwnableCanUpsert,
   mockCollectionAuthorizationMiddleware,
+  mockIsCollectionPublished,
 } from '../../spec/utils'
 import {
   collectionAttributesMock,
@@ -19,7 +20,6 @@ import { isManager } from '../ethereum/api/tpw'
 import { collectionAPI } from '../ethereum/api/collection'
 import { isCommitteeMember } from '../Committee'
 import { app } from '../server'
-import { isPublished } from '../utils/eth'
 import { Collection } from './Collection.model'
 import { thirdPartyAPI } from '../ethereum/api/thirdParty'
 import {
@@ -420,10 +420,7 @@ describe('Collection router', () => {
             ...dbCollection,
             lock: currentDate,
           })
-          ;(collectionAPI.fetchCollection as jest.Mock).mockResolvedValueOnce(
-            undefined
-          )
-          ;(isPublished as jest.Mock).mockResolvedValueOnce(false)
+          mockIsCollectionPublished(dbCollection.id, false)
           jest.spyOn(Date, 'now').mockReturnValueOnce(currentDate)
         })
 
@@ -481,10 +478,7 @@ describe('Collection router', () => {
             ...dbCollection,
             lock: null,
           })
-          ;(isPublished as jest.Mock).mockResolvedValueOnce(false)
-          ;(collectionAPI.fetchCollection as jest.Mock).mockResolvedValueOnce(
-            undefined
-          )
+          mockIsCollectionPublished(dbCollection.id, false)
         })
 
         it('should upsert the collection and respond with a 200 and the upserted collection', () => {
@@ -827,10 +821,7 @@ describe('Collection router', () => {
         beforeEach(() => {
           const lockDate = new Date()
           dbCollection.lock = lockDate
-          ;(collectionAPI.fetchCollection as jest.MockedFunction<
-            typeof collectionAPI.fetchCollection
-          >).mockResolvedValueOnce(null)
-          ;(isPublished as jest.Mock).mockReturnValueOnce(false)
+          mockIsCollectionPublished(dbCollection.id, false)
           jest.spyOn(Date, 'now').mockReturnValueOnce(lockDate.getTime())
         })
 
@@ -859,10 +850,7 @@ describe('Collection router', () => {
         beforeEach(() => {
           const lockDate = new Date()
           dbCollection.lock = lockDate
-          ;(collectionAPI.fetchCollection as jest.MockedFunction<
-            typeof collectionAPI.fetchCollection
-          >).mockResolvedValueOnce(null)
-          ;(isPublished as jest.Mock).mockReturnValueOnce(false)
+          mockIsCollectionPublished(dbCollection.id, false)
           jest
             .spyOn(Date, 'now')
             .mockReturnValueOnce(lockDate.getTime() + 1000 * 60 * 60 * 24)

src/Collection/Collection.router.ts

@@ -46,7 +46,7 @@ export class CollectionRouter extends Router {
     id: string,
     ethAddress: string
   ): Promise<boolean> => {
-    return this.service.isCollectionByIdOwnedOrManagedBy(id, ethAddress)
+    return this.service.isOwnedOrManagedBy(id, ethAddress)
   }
 
   mount() {

src/Collection/Collection.service.ts

@@ -189,24 +189,11 @@ export class CollectionService {
     ])
   }
 
-  public findCollectionThatOwnsItem(
-    itemId: string
-  ): Promise<CollectionAttributes> {
-    return Collection.findByOwnerOfItem(itemId)
-  }
-
-  public async isCollectionByIdOwnedOrManagedBy(
+  public async isOwnedOrManagedBy(
     id: string,
     ethAddress: string
   ): Promise<boolean> {
     const collection = await Collection.findOne<CollectionAttributes>(id)
-    return this.isCollectionOwnedOrManagedBy(collection, ethAddress)
-  }
-
-  public async isCollectionOwnedOrManagedBy(
-    collection: CollectionAttributes | undefined,
-    ethAddress: string
-  ): Promise<boolean> {
     if (collection && collection.third_party_id && collection.urn_suffix) {
       return isManager(
         getThirdPartyCollectionURN(

src/Item/Item.router.spec.ts

@@ -6,6 +6,8 @@ import {
   buildURL,
   mockExistsMiddleware,
   mockOwnableCanUpsert,
+  mockItemAuthorizationMiddleware,
+  mockIsCollectionPublished,
 } from '../../spec/utils'
 import { collectionAttributesMock } from '../../spec/mocks/collections'
 import {
@@ -19,7 +21,6 @@ import { wallet } from '../../spec/mocks/wallet'
 import { isCommitteeMember } from '../Committee'
 import { app } from '../server'
 import { Collection } from '../Collection/Collection.model'
-import { isPublished } from '../utils/eth'
 import { collectionAPI } from '../ethereum/api/collection'
 import { Item } from './Item.model'
 import { hasAccess } from './access'
@@ -469,8 +470,7 @@ describe('Item router', () => {
           contract_address: Wallet.createRandom().address,
           lock: new Date(),
         })
-        mockCollectionApi.fetchCollection.mockResolvedValueOnce(null)
-        ;(isPublished as jest.Mock).mockResolvedValueOnce(false)
+        mockIsCollectionPublished(collectionAttributesMock.id, false)
       })
 
       describe('and the item is being changed', () => {
@@ -583,8 +583,7 @@ describe('Item router', () => {
           contract_address: Wallet.createRandom().address,
         })
 
-        mockCollectionApi.fetchCollection.mockResolvedValueOnce(null)
-        ;(isPublished as jest.Mock).mockResolvedValueOnce(false)
+        mockIsCollectionPublished(collectionAttributesMock.id, false)
         mockItem.prototype.upsert.mockResolvedValueOnce(dbItem)
       })
 
@@ -602,4 +601,93 @@ describe('Item router', () => {
       })
     })
   })
+
+  describe('when deleting an item', () => {
+    beforeEach(() => {
+      url = `/items/${dbItem.id}`
+    })
+
+    describe('and the user is not authorized', () => {
+      beforeEach(() => {
+        mockExistsMiddleware(Item, dbItem.id)
+        mockItemAuthorizationMiddleware(dbItem.id, wallet.address, false, false)
+      })
+
+      it('should respond with a 401 and a message signaling that the user is not authorized', () => {
+        return server
+          .delete(buildURL(url))
+          .set(createAuthHeaders('delete', url))
+          .expect(401)
+          .then((response: any) => {
+            expect(response.body).toEqual({
+              error: `Unauthorized user ${wallet.address} for items ${dbItem.id}`,
+              data: { ethAddress: wallet.address, tableName: Item.tableName },
+              ok: false,
+            })
+          })
+      })
+    })
+
+    describe("and the item doesn't have a collection", () => {
+      beforeEach(() => {
+        mockExistsMiddleware(Item, dbItem.id)
+        mockItemAuthorizationMiddleware(dbItem.id, wallet.address, false, true)
+        ;(Item.findOne as jest.MockedFunction<
+          typeof Item.findOne
+        >).mockResolvedValueOnce({ ...dbItem, collection_id: null })
+      })
+
+      it('should respond with a 200 and delete the item', () => {
+        return server
+          .delete(buildURL(url))
+          .set(createAuthHeaders('delete', url))
+          .expect(200)
+          .then((response: any) => {
+            expect(response.body).toEqual({
+              data: true,
+              ok: true,
+            })
+
+            expect(Item.delete).toHaveBeenCalledWith({ id: dbItem.id })
+          })
+      })
+    })
+
+    describe('and the item has a collection', () => {
+      beforeEach(() => {
+        mockExistsMiddleware(Item, dbItem.id)
+        mockItemAuthorizationMiddleware(dbItem.id, wallet.address, false, true)
+        ;(Item.findOne as jest.MockedFunction<
+          typeof Item.findOne
+        >).mockResolvedValueOnce(dbItem)
+        ;(Collection.findOne as jest.MockedFunction<
+          typeof Collection.findOne
+        >).mockResolvedValueOnce(collectionAttributesMock)
+      })
+
+      describe('and its collection is already published', () => {
+        beforeEach(() => {
+          mockIsCollectionPublished(collectionAttributesMock.id, true)
+        })
+
+        it('should respond with a 401 and a message signaling that the item is already published', () => {
+          return server
+            .delete(buildURL(url))
+            .set(createAuthHeaders('delete', url))
+            .expect(401)
+            .then((response: any) => {
+              expect(response.body).toEqual({
+                error: "The item was published. It can't be deleted",
+                data: {
+                  id: dbItem.id,
+                  blockchain_item_id: dbItem.blockchain_item_id,
+                  contract_address: collectionAttributesMock.contract_address,
+                },
+                ok: false,
+              })
+            })
+        })
+      })
+    })
+  })
 })

src/Item/Item.router.ts

@@ -1,7 +1,6 @@
 import { Request, Response } from 'express'
 import { utils } from 'decentraland-commons'
 import { server } from 'decentraland-server'
-
 import { Router } from '../common/Router'
 import { HTTPError, STATUS_CODES } from '../common/HTTPError'
 import { collectionAPI } from '../ethereum/api/collection'

src/Item/Item.service.ts

@@ -1,4 +1,6 @@
 import { CollectionService } from '../Collection/Collection.service'
+import { Item } from './Item.model'
+import { ItemAttributes } from './Item.types'
 
 export class ItemService {
   private collectionService = new CollectionService()
@@ -7,12 +9,16 @@ export class ItemService {
     id: string,
     ethAddress: string
   ): Promise<boolean> {
-    const collection = await this.collectionService.findCollectionThatOwnsItem(
-      id
-    )
-    return this.collectionService.isCollectionOwnedOrManagedBy(
-      collection,
-      ethAddress
-    )
+    const dbItem = await Item.findOne<ItemAttributes>(id)
+    if (dbItem?.urn_suffix && dbItem?.collection_id) {
+      return this.collectionService.isOwnedOrManagedBy(
+        dbItem?.collection_id,
+        ethAddress
+      )
+    } else if (dbItem) {
+      return dbItem.eth_address === dbItem.eth_address
+    }
+
+    return false
   }
 }