From 1048ff8c2f4a40e83b77b0863f57e6663d536ddf Mon Sep 17 00:00:00 2001 From: Juanma Hidalgo Date: Fri, 18 Feb 2022 19:55:08 +0100 Subject: [PATCH] feat: Update how is_published for TP items is calculated --- src/Item/Item.router.spec.ts | 34 ++++++++++++++++++++++++++++------ src/Item/Item.router.ts | 14 +++++++++----- src/Item/Item.service.ts | 21 ++++++++++++--------- src/Item/utils.ts | 2 +- src/ethereum/api/Bridge.ts | 36 ++++++++++++++++++++++++++++-------- 5 files changed, 78 insertions(+), 29 deletions(-) diff --git a/src/Item/Item.router.spec.ts b/src/Item/Item.router.spec.ts index d1d030fa..5c08d5c3 100644 --- a/src/Item/Item.router.spec.ts +++ b/src/Item/Item.router.spec.ts @@ -33,7 +33,7 @@ import { tpWearableMock, wearableMock } from '../../spec/mocks/peer' import { wallet } from '../../spec/mocks/wallet' import { isCommitteeMember } from '../Committee' import { app } from '../server' -import { ItemCuration } from '../Curation/ItemCuration' +import { ItemCuration, ItemCurationAttributes } from '../Curation/ItemCuration' import { Collection } from '../Collection/Collection.model' import { collectionAPI } from '../ethereum/api/collection' import { peerAPI, Wearable } from '../ethereum/api/peer' @@ -83,13 +83,16 @@ const server = supertest(app.getApp()) describe('Item router', () => { let dbItem: ItemAttributes + let dbItemCuration: ItemCurationAttributes let dbTPItem: ThirdPartyItemAttributes let dbItemNotPublished: ItemAttributes let dbTPItemNotPublished: ThirdPartyItemAttributes + let dbTPItemPublished: ThirdPartyItemAttributes let resultingItem: ResultItem let resultingTPItem: ResultItem let resultItemNotPublished: ResultItem let resultTPItemNotPublished: ResultItem + let resultTPItemPublished: ResultItem let wearable: Wearable let tpWearable: Wearable let itemFragment: ItemFragment @@ -118,10 +121,20 @@ describe('Item router', () => { beneficiary: '', urn_suffix: '', } + dbTPItemPublished = { + ...dbTPItem, + id: uuidv4(), + urn_suffix: '3', + } + dbItemCuration = { ...itemCurationMock, item_id: dbTPItemPublished.id } resultingItem = toResultItem(dbItem, itemFragment, wearable) resultingTPItem = toResultTPItem(dbTPItem, dbTPCollectionMock) resultItemNotPublished = asResultItem(dbItemNotPublished) - resultTPItemNotPublished = asResultItem(dbTPItemNotPublished) + resultTPItemNotPublished = asResultItem(dbTPItemNotPublished) // no itemCuration & no catalyst, should be regular Item + resultTPItemPublished = { + ...toResultTPItem(dbTPItemPublished, dbTPCollectionMock), + is_approved: false, + } }) afterEach(() => { @@ -233,7 +246,9 @@ describe('Item router', () => { dbTPItemMock, dbTPItemNotPublishedMock, dbItemNotPublished, + dbTPItemPublished, ]) + ;(ItemCuration.find as jest.Mock).mockResolvedValueOnce([dbItemCuration]) ;(collectionAPI.fetchItems as jest.Mock).mockResolvedValueOnce([ itemFragment, ]) @@ -271,6 +286,7 @@ describe('Item router', () => { dbTPItemNotPublishedMock.urn_suffix! ), }, + resultTPItemPublished, ], ok: true, }) @@ -297,6 +313,7 @@ describe('Item router', () => { ;(Collection.findByIds as jest.Mock).mockResolvedValueOnce([ dbTPCollectionMock, ]) // for third parties + ;(ItemCuration.find as jest.Mock).mockResolvedValueOnce([dbItemCuration]) mockItemConsolidation([dbItem], [wearable]) ;(peerAPI.fetchWearables as jest.Mock).mockResolvedValueOnce([tpWearable]) url = `/${wallet.address}/items` @@ -373,6 +390,7 @@ describe('Item router', () => { beforeEach(() => { ;(Item.find as jest.Mock).mockResolvedValueOnce([ dbTPItem, + dbTPItemPublished, dbTPItemNotPublished, ]) ;(Collection.findOne as jest.Mock).mockResolvedValueOnce( @@ -381,9 +399,9 @@ describe('Item router', () => { ;(Collection.findByIds as jest.Mock).mockResolvedValueOnce([ dbTPCollectionMock, ]) - ;(ItemCuration.findLastCreatedByCollectionIdAndStatus as jest.Mock).mockResolvedValueOnce( - itemCurationMock - ) + ;(ItemCuration.findByCollectionId as jest.Mock).mockResolvedValueOnce([ + dbItemCuration, + ]) mockItemConsolidation([dbItemMock], [tpWearable]) url = `/collections/${dbCollectionMock.id}/items` }) @@ -395,7 +413,11 @@ describe('Item router', () => { .expect(200) .then((response: any) => { expect(response.body).toEqual({ - data: [resultingTPItem, resultTPItemNotPublished], + data: [ + resultingTPItem, + resultTPItemPublished, + resultTPItemNotPublished, + ], ok: true, }) }) diff --git a/src/Item/Item.router.ts b/src/Item/Item.router.ts index 2ea5930b..cbe8f956 100644 --- a/src/Item/Item.router.ts +++ b/src/Item/Item.router.ts @@ -37,6 +37,8 @@ import { UnauthorizedToChangeToCollectionError, UnauthorizedToUpsertError, } from './Item.errors' +import { ItemCuration } from '../Curation/ItemCuration' +import { ItemCurationAttributes } from '../Curation/ItemCuration/ItemCuration.types' export class ItemRouter extends Router { // To be removed once we move everything to the item service @@ -153,17 +155,18 @@ export class ItemRouter extends Router { } // TODO: We need to paginate this. To do it, we'll have to fetch remote items via the paginated dbItemIds - const [allItems, remoteItems] = await Promise.all([ + const [allItems, remoteItems, , itemCurations] = await Promise.all([ Item.find(), collectionAPI.fetchItems(), - thirdPartyAPI.fetchItems(), + thirdPartyAPI.fetchItems(), //TODO, UNUSED + ItemCuration.find(), ]) const { items, tpItems } = this.itemService.splitItems(allItems) const [fullItems, fullTPItems] = await Promise.all([ Bridge.consolidateItems(items, remoteItems), - Bridge.consolidateTPItems(tpItems), + Bridge.consolidateTPItems(tpItems, itemCurations), ]) // TODO: sorting (we're not breaking pagination) @@ -182,15 +185,16 @@ export class ItemRouter extends Router { ) } - const [dbItems, remoteItems, dbTPItems] = await Promise.all([ + const [dbItems, remoteItems, dbTPItems, itemCurations] = await Promise.all([ Item.find({ eth_address }), collectionAPI.fetchItemsByAuthorizedUser(eth_address), this.itemService.getTPItemsByManager(eth_address), + ItemCuration.find(), ]) const [items, tpItems] = await Promise.all([ Bridge.consolidateItems(dbItems, remoteItems), - Bridge.consolidateTPItems(dbTPItems), + Bridge.consolidateTPItems(dbTPItems, itemCurations), ]) // TODO: list.concat(list2) will not break pagination (when we add it), but it will break any order we have beforehand. diff --git a/src/Item/Item.service.ts b/src/Item/Item.service.ts index c6d0f9af..deb74697 100644 --- a/src/Item/Item.service.ts +++ b/src/Item/Item.service.ts @@ -169,15 +169,18 @@ export class ItemService { dbCollection: ThirdPartyCollectionAttributes, dbItems: ItemAttributes[] ): Promise<{ collection: CollectionAttributes; items: FullItem[] }> { - const lastItemCuration = await ItemCuration.findLastCreatedByCollectionIdAndStatus( - dbCollection.id, - CurationStatus.APPROVED + const collectionItemCurations = await ItemCuration.findByCollectionId( + dbCollection.id + ) + const collection = + collectionItemCurations.length > 0 + ? Bridge.mergeTPCollection(dbCollection, collectionItemCurations[0]) + : dbCollection + + const items = await Bridge.consolidateTPItems( + dbItems, + collectionItemCurations ) - const collection = lastItemCuration - ? Bridge.mergeTPCollection(dbCollection, lastItemCuration) - : dbCollection - - const items = await Bridge.consolidateTPItems(dbItems) return { collection, items } } @@ -223,7 +226,7 @@ export class ItemService { const catalystItems = await peerAPI.fetchWearables([urn]) if (catalystItems.length > 0) { - item = Bridge.mergeTPItem(dbItem, catalystItems[0]) + item = Bridge.mergeTPItem(dbItem, collection, catalystItems[0]) } } diff --git a/src/Item/utils.ts b/src/Item/utils.ts index 83c1d935..07c041c0 100644 --- a/src/Item/utils.ts +++ b/src/Item/utils.ts @@ -114,7 +114,7 @@ export async function getMergedItem(id: string): Promise { throw new UnpublishedItemError(id) } - fullItem = Bridge.mergeTPItem(dbItem, wearable) + fullItem = Bridge.mergeTPItem(dbItem, dbCollection, wearable) } else { const { collection: remoteCollection, diff --git a/src/ethereum/api/Bridge.ts b/src/ethereum/api/Bridge.ts index 090a6c12..2700ae0c 100644 --- a/src/ethereum/api/Bridge.ts +++ b/src/ethereum/api/Bridge.ts @@ -1,6 +1,10 @@ import { constants } from 'ethers' import { utils } from 'decentraland-commons' -import { CollectionAttributes, Collection } from '../../Collection' +import { + CollectionAttributes, + Collection, + ThirdPartyCollectionAttributes, +} from '../../Collection' import { ItemAttributes, Item, FullItem } from '../../Item' import { fromUnixTimestamp } from '../../utils/parse' import { buildTPItemURN, decodeThirdPartyItemURN } from '../../Item/utils' @@ -55,7 +59,8 @@ export class Bridge { * @param dbItems - Database TP items */ static async consolidateTPItems( - dbItems: ItemAttributes[] + dbItems: ItemAttributes[], + itemCurations: ItemCurationAttributes[] ): Promise { const dbTPItemIds = dbItems.map((item) => item.collection_id!) const dbTPCollections = await Collection.findByIds(dbTPItemIds) @@ -90,16 +95,21 @@ export class Bridge { for (const urn in itemsByURN) { const item = itemsByURN[urn] + const itemCuration = itemCurations.find((ic) => ic.item_id === item.id) let fullItem: FullItem const catalystItem = tpCatalystItems.find( (catalystItem) => catalystItem.id === urn ) - if (catalystItem) { - fullItem = Bridge.mergeTPItem(item, catalystItem) + const collection = dbTPCollectionsIndex[item.collection_id!] + if (itemCuration || catalystItem) { + fullItem = Bridge.mergeTPItem( + item, + collection as ThirdPartyCollectionAttributes, + catalystItem + ) } else { - const collection = dbTPCollectionsIndex[item.collection_id!] fullItem = Bridge.toFullItem(item, collection) } @@ -115,10 +125,20 @@ export class Bridge { * @param dbItem - Database TP item * @param catalystItem - Catalyst item */ - static mergeTPItem(dbItem: ItemAttributes, catalystItem: Wearable): FullItem { + static mergeTPItem( + dbItem: ItemAttributes, + dbCollection: ThirdPartyCollectionAttributes, + catalystItem?: Wearable + ): FullItem { const data = dbItem.data const category = data.category - const urn: string = catalystItem.id + const urn: string = catalystItem + ? catalystItem.id + : buildTPItemURN( + dbCollection.third_party_id, + dbCollection.urn_suffix, + dbItem.urn_suffix! + ) return { ...Bridge.toFullItem(dbItem), @@ -134,7 +154,7 @@ export class Bridge { in_catalyst: true, is_published: true, // For now, items are always approved. Rejecting (or disabling) items will be done at the record level, for all collections that apply. - is_approved: true, + is_approved: !!catalystItem, // TODO: This will be resolved when we tackle #394 content_hash: '', data: {