Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Return item curation when updating/publishing #420

Merged
merged 21 commits into from
Mar 9, 2022
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/Collection/Collection.router.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1531,6 +1531,11 @@ describe('Collection router', () => {
created_at: item.created_at.toISOString(),
updated_at: item.updated_at.toISOString(),
})),
itemCurations: Array(3).fill({
...itemCurationMock,
created_at: itemCurationMock.created_at.toISOString(),
updated_at: itemCurationMock.updated_at.toISOString(),
}),
},
ok: true,
})
Expand Down
13 changes: 9 additions & 4 deletions src/Collection/Collection.router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,17 @@ import { collectionAPI } from '../ethereum/api/collection'
import { OwnableModel } from '../Ownable/Ownable.types'
import { MAX_FORUM_ITEMS } from '../Item/utils'
import { UnpublishedItemError } from '../Item/Item.errors'
import { FullItem, Item } from '../Item'
import { Item } from '../Item'
import { isCommitteeMember } from '../Committee'
import { buildCollectionForumPost, createPost } from '../Forum'
import { sendDataToWarehouse } from '../warehouse'
import { Collection } from './Collection.model'
import { CollectionService } from './Collection.service'
import { CollectionAttributes, FullCollection } from './Collection.types'
import {
CollectionAttributes,
FullCollection,
PublishCollectionResponse,
} from './Collection.types'
import { upsertCollectionSchema, saveTOSSchema } from './Collection.schema'
import { hasPublicAccess } from './access'
import { hasTPCollectionURN, isTPCollection, toFullCollection } from './utils'
Expand Down Expand Up @@ -245,13 +249,13 @@ export class CollectionRouter extends Router {

publishCollection = async (
req: AuthRequest
): Promise<{ collection: FullCollection; items: FullItem[] }> => {
): Promise<PublishCollectionResponse<FullCollection>> => {
const id = server.extractFromReq(req, 'id')

try {
const dbCollection = await this.service.getDBCollection(id)

let result: { collection: CollectionAttributes; items: FullItem[] }
let result: PublishCollectionResponse<CollectionAttributes>

if (isTPCollection(dbCollection)) {
const itemIds = server.extractFromReq<string[]>(req, 'itemIds')
Expand Down Expand Up @@ -282,6 +286,7 @@ export class CollectionRouter extends Router {
return {
collection: toFullCollection(result.collection),
items: result.items,
itemCurations: result.itemCurations,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding this here because we need to keep track of the itemCurations in the FE as well, so now it's part of the response.

}
} catch (error) {
if (error instanceof InvalidRequestError) {
Expand Down
11 changes: 5 additions & 6 deletions src/Collection/Collection.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { Bridge } from '../ethereum/api/Bridge'
import { isPublished } from '../utils/eth'
import { InvalidRequestError } from '../utils/errors'
import { Ownable } from '../Ownable'
import { FullItem, Item, ItemAttributes } from '../Item'
import { Item, ItemAttributes } from '../Item'
import { UnpublishedItemError } from '../Item/Item.errors'
import { ItemCuration, ItemCurationAttributes } from '../Curation/ItemCuration'
import { SlotUsageCheque, SlotUsageChequeAttributes } from '../SlotUsageCheque'
Expand All @@ -21,6 +21,7 @@ import { decodeTPCollectionURN, isTPCollection, toDBCollection } from './utils'
import {
CollectionAttributes,
FullCollection,
PublishCollectionResponse,
ThirdPartyCollectionAttributes,
} from './Collection.types'
import { Collection } from './Collection.model'
Expand Down Expand Up @@ -79,7 +80,7 @@ export class CollectionService {
public async publishDCLCollection(
dbCollection: CollectionAttributes,
dbItems: ItemAttributes[]
) {
): Promise<PublishCollectionResponse<CollectionAttributes>> {
const remoteCollection = await collectionAPI.fetchCollection(
dbCollection!.contract_address!
)
Expand Down Expand Up @@ -142,10 +143,7 @@ export class CollectionService {
dbItems: ItemAttributes[],
signedMessage: string,
signature: string
): Promise<{
collection: CollectionAttributes
items: FullItem[]
}> {
): Promise<PublishCollectionResponse<CollectionAttributes>> {
// For DCL collections, once a published collection item changes, the PUSH CHANGES button appears
// That will fire a /collections/${collectionId}/curation which will create a new CollectionCuration
// Subsequent changes will not show the push changes button, as it's already under_review
Expand Down Expand Up @@ -233,6 +231,7 @@ export class CollectionService {
return {
collection: Bridge.mergeTPCollection(dbCollection, lastItemCuration),
items: await Bridge.consolidateTPItems(dbItems, itemCurations),
itemCurations,
}
}

Expand Down
9 changes: 9 additions & 0 deletions src/Collection/Collection.types.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import { ItemCurationAttributes } from '../Curation/ItemCuration'
import { FullItem } from '../Item'

export type CollectionAttributes = {
id: string // uuid
/**
Expand Down Expand Up @@ -33,3 +36,9 @@ export type FullCollection = Omit<
> & {
urn: string | null
}

export type PublishCollectionResponse<T> = {
collection: T
items: FullItem[]
itemCurations?: ItemCurationAttributes[]
}
28 changes: 14 additions & 14 deletions src/Curation/Curation.router.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -449,8 +449,12 @@ describe('when handling a request', () => {
.spyOn(service, 'getLatestById')
.mockResolvedValueOnce({ id: 'curationId' } as any)

jest
.spyOn(service, 'updateStatusAndReturnById')
.mockResolvedValueOnce(expectedCuration)

updateSpy = jest
.spyOn(CollectionCuration, 'update')
.spyOn(service, 'updateStatusAndReturnById')
.mockResolvedValueOnce(expectedCuration)
})

Expand All @@ -464,12 +468,8 @@ describe('when handling a request', () => {
await router.updateCollectionCuration(req)

expect(updateSpy).toHaveBeenCalledWith(
{
id: 'curationId',
status: CurationStatus.REJECTED,
updated_at: expect.any(String),
},
{ id: 'curationId' }
'curationId',
CurationStatus.REJECTED
)
})
})
Expand All @@ -485,6 +485,10 @@ describe('when handling a request', () => {
id: 'uuid-123123-123123',
} as ItemCurationAttributes

jest
.spyOn(service, 'updateStatusAndReturnById')
.mockResolvedValueOnce(expectedCuration)

jest
.spyOn(service, 'getLatestById')
.mockResolvedValueOnce({ id: 'curationId' } as any)
Expand All @@ -494,7 +498,7 @@ describe('when handling a request', () => {
.mockResolvedValueOnce({ rowCount: 1 })

itemUpdateSpy = jest
.spyOn(ItemCuration, 'update')
.spyOn(service, 'updateStatusAndReturnById')
.mockResolvedValueOnce(expectedCuration)
})

Expand All @@ -508,12 +512,8 @@ describe('when handling a request', () => {
await router.updateItemCuration(req)

expect(itemUpdateSpy).toHaveBeenCalledWith(
{
id: 'curationId',
status: CurationStatus.REJECTED,
updated_at: expect.any(String),
},
{ id: 'curationId' }
'curationId',
CurationStatus.REJECTED
)
})

Expand Down
12 changes: 3 additions & 9 deletions src/Curation/Curation.router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -341,13 +341,9 @@ export class CurationRouter extends Router {
)
}

return curationService.getModel().update(
{
...curation,
status: curationJSON.status,
updated_at: this.getISODate(),
},
{ id: curation.id }
return await curationService.updateStatusAndReturnById(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, we need the updated row, so I created this method to update the status and updated_at fields and return the affected row

curation.id,
curationJSON.status
)
}

Expand Down Expand Up @@ -389,8 +385,6 @@ export class CurationRouter extends Router {
return curationService.getModel().create(attributes)
}

private getISODate = () => new Date().toISOString()

private validateAccessToCuration = async (
service: CurationService<any>,
id: string,
Expand Down
11 changes: 10 additions & 1 deletion src/Curation/Curation.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { getMergedCollection } from '../Collection/utils'
import { hasAccess as hasItemAccess } from '../Item/access'
import { getMergedItem } from '../Item/utils'
import { CollectionCuration } from './CollectionCuration'
import { CurationType } from './Curation.types'
import { CurationStatus, CurationType } from './Curation.types'
import { ItemCuration } from './ItemCuration'

// TODO: This class SHOULD NOT make database queries. It's useful but it breakes the convention we have where only model know about queries
Expand Down Expand Up @@ -69,6 +69,15 @@ export class CurationService<
return result[0]
}

async updateStatusAndReturnById(id: string, status: CurationStatus) {
const result = await this.getModel().query(SQL`
UPDATE ${raw(this.getTableName())}
SET status = ${status}, updated_at = ${new Date()}
WHERE id = ${id}
RETURNING *`)
return result[0]
}

async hasAccess(id: string, ethAddress: string) {
switch (this.type) {
case CurationType.COLLECTION: {
Expand Down
6 changes: 5 additions & 1 deletion src/Curation/Curation.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@ export const patchCurationSchema = Object.freeze({
properties: {
status: {
type: 'string',
enum: [CurationStatus.APPROVED, CurationStatus.REJECTED],
enum: [
CurationStatus.PENDING,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding PENDING so it can be updated to that status.

CurationStatus.APPROVED,
CurationStatus.REJECTED,
],
},
},
additionalProperties: false,
Expand Down
10 changes: 6 additions & 4 deletions src/Item/Item.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,12 @@ export class Item extends Model<ItemAttributes> {

static findByThirdPartyIds(thirdPartyIds: string[]) {
return this.query<ItemAttributes>(SQL`
SELECT *
FROM ${raw(this.tableName)} i
JOIN ${raw(Collection.tableName)} c ON c.id = i.collection_id
WHERE c.third_party_id = ANY(${thirdPartyIds})`)
SELECT item.*
FROM ${raw(this.tableName)} item
JOIN ${raw(
Collection.tableName
)} collection ON collection.id = item.collection_id
WHERE collection.third_party_id = ANY(${thirdPartyIds})`)
}

static findOrderedByCollectionId(
Expand Down
2 changes: 1 addition & 1 deletion src/Item/Item.router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ export class ItemRouter extends Router {
}

const [dbItems, remoteItems, dbTPItems, itemCurations] = await Promise.all([
Item.find<ItemAttributes>({ eth_address }),
Item.find<ItemAttributes>({ eth_address, urn_suffix: null }),
collectionAPI.fetchItemsByAuthorizedUser(eth_address),
this.itemService.getTPItemsByManager(eth_address),
ItemCuration.find<ItemCurationAttributes>(),
Expand Down