feat: Add mapping completion in curations (#767)

* feat: Add mapping completion in curations * fix: Update mapping statement query * fix: Query
decentraland · Aug 28, 2024 · f6bf391 · f6bf391
1 parent 8589fbf
commit f6bf391
Show file tree

Hide file tree

Showing 13 changed files with 147 additions and 41 deletions.
diff --git a/migrations/1724772165494_add-mapping-completion-to-item-curation.ts b/migrations/1724772165494_add-mapping-completion-to-item-curation.ts
@@ -0,0 +1,15 @@
+/* eslint-disable @typescript-eslint/naming-convention */
+import { MigrationBuilder } from 'node-pg-migrate'
+import { ItemCuration } from '../src/Curation/ItemCuration'
+
+const columnName = 'is_mapping_complete'
+
+export async function up(pgm: MigrationBuilder): Promise<void> {
+  pgm.addColumn(ItemCuration.tableName, {
+    [columnName]: { type: 'boolean', notNull: false },
+  })
+}
+
+export async function down(pgm: MigrationBuilder): Promise<void> {
+  pgm.dropColumn(ItemCuration.tableName, columnName)
+}
diff --git a/spec/mocks/collections.ts b/spec/mocks/collections.ts
@@ -13,7 +13,9 @@ import {
 import { toUnixTimestamp } from '../../src/utils/parse'
 import { wallet } from './wallet'
 
-export const dbCollectionMock: CollectionAttributes = {
+export const dbCollectionMock: CollectionAttributes & {
+  is_mapping_complete: boolean
+} = {
   id: uuidv4(),
   name: 'Standard Mocked Collection',
   eth_address: wallet.address,
@@ -30,6 +32,7 @@ export const dbCollectionMock: CollectionAttributes = {
   third_party_id: null,
   linked_contract_address: null,
   linked_contract_network: null,
+  is_mapping_complete: false,
   reviewed_at: new Date(),
   created_at: new Date(),
   updated_at: new Date(),

diff --git a/spec/mocks/itemCuration.ts b/spec/mocks/itemCuration.ts
@@ -10,4 +10,5 @@ export const itemCurationMock: ItemCurationAttributes = {
   created_at: new Date(),
   updated_at: new Date(),
   content_hash: 'aHash',
+  is_mapping_complete: null,
 }
diff --git a/spec/mocks/items.ts b/spec/mocks/items.ts
@@ -24,6 +24,7 @@ import { buildTPItemURN } from '../../src/Item/utils'
 import { CollectionAttributes } from '../../src/Collection'
 import { decodeThirdPartyItemURN, isTPCollection } from '../../src/utils/urn'
 import { CatalystItem } from '../../src/ethereum/api/peer'
+import { ItemCurationAttributes } from '../../src/Curation/ItemCuration'
 import { dbCollectionMock, dbTPCollectionMock } from './collections'
 
 export type ResultItem = Omit<FullItem, 'created_at' | 'updated_at'> & {
@@ -80,7 +81,8 @@ export function asResultItem(item: ItemAttributes): ResultItem {
 export function toResultTPItem(
   itemAttributes: ItemAttributes,
   dbCollection: CollectionAttributes,
-  catalystItem?: Wearable & Partial<ThirdPartyProps>
+  catalystItem?: Wearable & Partial<ThirdPartyProps>,
+  curation?: ItemCurationAttributes
 ): ResultItem {
   if (
     !dbCollection.third_party_id ||
@@ -105,7 +107,7 @@ export function toResultTPItem(
     updated_at: itemAttributes.updated_at.toISOString(),
     is_approved: !!catalystItem,
     in_catalyst: !!catalystItem,
-    is_published: !!catalystItem,
+    is_published: !!catalystItem || !!curation,
     urn,
     blockchain_item_id: decodeThirdPartyItemURN(urn).item_urn_suffix,
     total_supply: 0,

diff --git a/src/Collection/Collection.model.ts b/src/Collection/Collection.model.ts
@@ -202,6 +202,28 @@ export class Collection extends Model<CollectionAttributes> {
       : SQL``
   }
 
+  /**
+   * Builds the statement to check if the collection requires the mapping migration to be completed.
+   * The item migration will be required to be completed if:
+   * - The collection has items and any of its items don't have a migration.
+   * - The collection has items with migrations but there are items that weren't approved and uploaded.
+   */
+  static isMappingCompleteTableStatement() {
+    return SQL`SELECT NOT EXISTS 
+      (SELECT mappings_info.is_mapping_complete FROM
+        (SELECT DISTINCT ON (items.id) items.id, item_curations.updated_at, item_curations.is_mapping_complete
+          FROM ${raw(Item.tableName)} items
+          LEFT JOIN ${raw(
+            ItemCuration.tableName
+          )} item_curations ON items.id = item_curations.item_id
+          WHERE items.collection_id = collections.id
+          ORDER BY items.id, item_curations.updated_at DESC) mappings_info
+        WHERE mappings_info.is_mapping_complete = false OR mappings_info.is_mapping_complete IS NULL)
+      OR NOT EXISTS (SELECT 1 FROM ${raw(
+        Item.tableName
+      )} items WHERE items.collection_id = collections.id)`
+  }
+
   /**
    * Finds all the Collections that given parameters. It sorts and paginates the results.
    * If the status is APPROVED, the remoteIds will be the ones with `isApproved` true.
@@ -231,7 +253,8 @@ export class Collection extends Model<CollectionAttributes> {
       SELECT collections.*, COUNT(*) OVER() as collection_count, (SELECT COUNT(*) FROM ${raw(
         Item.tableName
       )} 
-        WHERE items.collection_id = collections.id) as item_count
+        WHERE items.collection_id = collections.id) as item_count,
+        (${SQL`${this.isMappingCompleteTableStatement()}`}) as is_mapping_complete
         FROM (
           SELECT DISTINCT on (c.id) c.* FROM ${raw(Collection.tableName)} c
             ${SQL`${this.getPublishedJoinStatement(isPublished)}`}  
@@ -268,7 +291,7 @@ export class Collection extends Model<CollectionAttributes> {
 
   static findByIds(ids: string[]) {
     return this.query<CollectionWithItemCount>(SQL`
-    SELECT *, (SELECT COUNT(*) FROM ${raw(
+    SELECT *, (${SQL`${this.isMappingCompleteTableStatement()}`}) as is_mapping_complete, (SELECT COUNT(*) FROM ${raw(
       Item.tableName
     )} WHERE items.collection_id = collections.id) as item_count
       FROM ${raw(this.tableName)}

diff --git a/src/Collection/Collection.router.spec.ts b/src/Collection/Collection.router.spec.ts
@@ -384,6 +384,7 @@ describe('Collection router', () => {
             lock: null,
             created_at: expectedCollectionToUpsert.created_at,
             updated_at: expectedCollectionToUpsert.updated_at,
+            is_mapping_complete: false,
           }))
         })
 
@@ -483,6 +484,7 @@ describe('Collection router', () => {
               lock: null,
               created_at: expectedCollectionToUpsert.created_at,
               updated_at: expectedCollectionToUpsert.updated_at,
+              is_mapping_complete: false,
             }))
           })
 
@@ -752,6 +754,7 @@ describe('Collection router', () => {
             lock: null,
             created_at: expectedCollectionToUpsert.created_at,
             updated_at: expectedCollectionToUpsert.updated_at,
+            is_mapping_complete: false,
           }))
           ;(Collection.isValidName as jest.Mock).mockResolvedValueOnce(true)
           ;(Collection.findOne as jest.Mock).mockResolvedValueOnce({
@@ -1450,8 +1453,8 @@ describe('Collection router', () => {
               sort: CollectionSort.CREATED_AT_DESC,
               thirdPartyIds: [],
               remoteIds: [],
-              q: "text",
-              isPublished: undefined
+              q: 'text',
+              isPublished: undefined,
             })
           })
       })
@@ -2245,6 +2248,7 @@ describe('Collection router', () => {
                     created_at: expect.any(Date),
                     updated_at: expect.any(Date),
                     content_hash: item.local_content_hash,
+                    is_mapping_complete: false,
                   },
                 ])
                 expect((ItemCuration.create as jest.Mock).mock.calls).toEqual(

diff --git a/src/Collection/Collection.service.ts b/src/Collection/Collection.service.ts
@@ -268,6 +268,7 @@ export class CollectionService {
           content_hash: item.local_content_hash,
           created_at: now,
           updated_at: now,
+          is_mapping_complete: item.mappings !== null,
         }
         itemCurationIds.push(itemCuration.id)
         promises.push(ItemCuration.create<ItemCurationAttributes>(itemCuration))

diff --git a/src/Curation/Curation.router.spec.ts b/src/Curation/Curation.router.spec.ts
@@ -826,11 +826,6 @@ describe('when handling a request', () => {
             .spyOn(service, 'getLatestById')
             .mockResolvedValueOnce({ id: 'curationId' } as any)
 
-          jest.spyOn(Item, 'findOne').mockResolvedValueOnce({
-            id: 'itemId',
-            local_content_hash: 'hash1',
-          } as any)
-
           collectionUpdateSpy = jest
             .spyOn(CollectionCuration, 'updateByItemId')
             .mockResolvedValueOnce({ rowCount: 1 })
@@ -840,26 +835,70 @@ describe('when handling a request', () => {
             .mockResolvedValueOnce(expectedCuration)
         })
 
-        it('should resolve with the updated curation', async () => {
-          await expect(router.updateItemCuration(req)).resolves.toStrictEqual(
-            expectedCuration
-          )
-        })
+        describe('and the item has mappings', () => {
+          beforeEach(() => {
+            jest.spyOn(Item, 'findOne').mockResolvedValueOnce({
+              id: 'itemId',
+              local_content_hash: 'hash1',
+              mappings: { type: 'any' },
+            } as any)
+          })
+
+          it('should resolve with the updated curation', async () => {
+            await expect(router.updateItemCuration(req)).resolves.toStrictEqual(
+              expectedCuration
+            )
+          })
 
-        it('should call the update method with the right data', async () => {
-          await router.updateItemCuration(req)
+          it('should call the update method with the right data', async () => {
+            await router.updateItemCuration(req)
 
-          expect(itemUpdateSpy).toHaveBeenCalledWith('curationId', {
-            status: CurationStatus.REJECTED,
-            content_hash: 'hash1',
-            updated_at: expect.any(Date),
+            expect(itemUpdateSpy).toHaveBeenCalledWith('curationId', {
+              status: CurationStatus.REJECTED,
+              content_hash: 'hash1',
+              updated_at: expect.any(Date),
+              is_mapping_complete: true,
+            })
+          })
+
+          it('should update the updated_at property of the collection curation', async () => {
+            await router.updateItemCuration(req)
+
+            expect(collectionUpdateSpy).toHaveBeenCalledWith(req.params.id)
           })
         })
 
-        it('should update the updated_at property of the collection curation', async () => {
-          await router.updateItemCuration(req)
+        describe('and the item does not have mappings', () => {
+          beforeEach(() => {
+            jest.spyOn(Item, 'findOne').mockResolvedValueOnce({
+              id: 'itemId',
+              local_content_hash: 'hash1',
+              mappings: null,
+            } as any)
+          })
+
+          it('should resolve with the updated curation', async () => {
+            await expect(router.updateItemCuration(req)).resolves.toStrictEqual(
+              expectedCuration
+            )
+          })
+
+          it('should call the update method with the right data', async () => {
+            await router.updateItemCuration(req)
 
-          expect(collectionUpdateSpy).toHaveBeenCalledWith(req.params.id)
+            expect(itemUpdateSpy).toHaveBeenCalledWith('curationId', {
+              status: CurationStatus.REJECTED,
+              content_hash: 'hash1',
+              updated_at: expect.any(Date),
+              is_mapping_complete: false,
+            })
+          })
+
+          it('should update the updated_at property of the collection curation', async () => {
+            await router.updateItemCuration(req)
+
+            expect(collectionUpdateSpy).toHaveBeenCalledWith(req.params.id)
+          })
         })
       })
     })
@@ -1108,6 +1147,7 @@ describe('when handling a request', () => {
             created_at: expect.any(Date),
             updated_at: expect.any(Date),
             content_hash: item.local_content_hash,
+            is_mapping_complete: false,
           })
         })
       })

diff --git a/src/Curation/Curation.router.ts b/src/Curation/Curation.router.ts
@@ -427,7 +427,7 @@ export class CurationRouter extends Router {
       }
     }
 
-    let fieldsToUpdate: Partial<
+    const fieldsToUpdate: Partial<
       CollectionCurationAttributes & ItemCurationAttributes
     > = {
       ...(curationJSON.assignee !== undefined
@@ -446,10 +446,11 @@ export class CurationRouter extends Router {
     }
 
     if (type === CurationType.ITEM) {
-      fieldsToUpdate = {
-        ...fieldsToUpdate,
-        content_hash: await this.getItemCurationContentHash(id),
-      }
+      const itemData = await this.getItemCurationContentHashAndMappingCompletion(
+        id
+      )
+      fieldsToUpdate.content_hash = itemData.content_hash
+      fieldsToUpdate.is_mapping_complete = itemData.is_mapping_complete
     }
 
     return curationService.updateById(curation.id, fieldsToUpdate)
@@ -507,22 +508,31 @@ export class CurationRouter extends Router {
       }
     }
     if (type === CurationType.ITEM) {
+      const itemData = await this.getItemCurationContentHashAndMappingCompletion(
+        id
+      )
       attributes.item_id = id
-      attributes.content_hash = await this.getItemCurationContentHash(id)
+      attributes.content_hash = itemData.content_hash
+      attributes.is_mapping_complete = itemData.is_mapping_complete
     }
 
     return curationService.getModel().create(attributes)
   }
 
-  private getItemCurationContentHash = async (id: string) => {
+  private getItemCurationContentHashAndMappingCompletion = async (
+    id: string
+  ) => {
     const dbItem = await Item.findOne<ThirdPartyItemAttributes>(id)
     if (!dbItem)
       throw new HTTPError(
         'There is no curation associated to that item',
         { id },
         STATUS_CODES.badRequest
       )
-    return dbItem.local_content_hash
+    return {
+      content_hash: dbItem.local_content_hash,
+      is_mapping_complete: dbItem.mappings !== null,
+    }
   }
 
   /* This method updates the video field of smart wearables

diff --git a/src/Curation/ItemCuration/ItemCuration.types.ts b/src/Curation/ItemCuration/ItemCuration.types.ts
@@ -7,6 +7,7 @@ export type ItemCurationAttributes = {
   status: CurationStatus
   created_at: Date
   updated_at: Date
+  is_mapping_complete: boolean | null
 }
 
 export type ItemCurationWithTotalCount = ItemCurationAttributes & {

diff --git a/src/Item/Item.router.spec.ts b/src/Item/Item.router.spec.ts
@@ -292,7 +292,7 @@ describe('Item router', () => {
                 ),
                 isMappingComplete: false,
               },
-              resultTPItemPublished,
+              { ...resultTPItemPublished, is_published: true },
             ],
             ok: true,
           })
@@ -623,8 +623,11 @@ describe('Item router', () => {
           .then((response: any) => {
             expect(response.body).toEqual({
               data: [
-                { ...resultingTPItem, isMappingComplete: true },
-                resultTPItemPublished,
+                {
+                  ...resultingTPItem,
+                  isMappingComplete: true,
+                },
+                { ...resultTPItemPublished, is_published: true },
                 resultTPItemNotPublished,
               ],
               ok: true,

diff --git a/src/Item/Item.service.ts b/src/Item/Item.service.ts
@@ -382,7 +382,8 @@ export class ItemService {
       item = Bridge.mergeTPItem(
         dbItem,
         collection as ThirdPartyCollectionAttributes,
-        catalystItems[0]
+        catalystItems[0],
+        lastItemCuration
       )
     }