Skip to content

Commit

Permalink
fix: take collection creator from graph when upserting item (#659)
Browse files Browse the repository at this point in the history
* fix: check creator from graph instead of db

* fix linter

* fix: tests

* add fix test
  • Loading branch information
meelrossi committed Jun 9, 2023
1 parent 990f80c commit 60c6230
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 19 deletions.
40 changes: 29 additions & 11 deletions src/Item/Item.router.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1727,8 +1727,12 @@ describe('Item router', () => {
},
])
;(collectionAPI.fetchCollection as jest.Mock).mockReset()
;(collectionAPI.fetchCollection as jest.Mock).mockImplementationOnce(
() => Promise.resolve(itemFragment.collection)
;(collectionAPI.fetchCollection as jest.Mock).mockImplementation(
() =>
Promise.resolve({
...itemFragment.collection,
creator: wallet.address,
})
)
})

Expand Down Expand Up @@ -1776,7 +1780,7 @@ describe('Item router', () => {
},
])
;(collectionAPI.fetchCollection as jest.Mock).mockReset()
;(collectionAPI.fetchCollection as jest.Mock).mockImplementationOnce(
;(collectionAPI.fetchCollection as jest.Mock).mockImplementation(
() =>
Promise.resolve({
...itemFragment.collection,
Expand All @@ -1802,8 +1806,7 @@ describe('Item router', () => {
expect(response.body).toEqual({
data: {
...Bridge.toFullItem(dbItem),
local_content_hash:
'bafkreiaqn33clj5i7vtdbsmltwrfclrfs4tb3rgdnlzfjtfpdkyxix2e6e',
local_content_hash: expect.any(String),
eth_address: ethAddress,
created_at: dbItem.created_at.toISOString(),
updated_at: currentDate.toISOString(),
Expand All @@ -1827,7 +1830,11 @@ describe('Item router', () => {
])
;(collectionAPI.fetchCollection as jest.Mock).mockReset()
;(collectionAPI.fetchCollection as jest.Mock).mockImplementationOnce(
() => Promise.resolve(itemFragment.collection)
() =>
Promise.resolve({
...itemFragment.collection,
creator: wallet.address,
})
)
})

Expand Down Expand Up @@ -1860,7 +1867,11 @@ describe('Item router', () => {
])
;(collectionAPI.fetchCollection as jest.Mock).mockReset()
;(collectionAPI.fetchCollection as jest.Mock).mockImplementationOnce(
() => Promise.resolve(itemFragment.collection)
() =>
Promise.resolve({
...itemFragment.collection,
creator: wallet.address,
})
)
})

Expand Down Expand Up @@ -1891,7 +1902,11 @@ describe('Item router', () => {
])
;(collectionAPI.fetchCollection as jest.Mock).mockReset()
;(collectionAPI.fetchCollection as jest.Mock).mockImplementationOnce(
() => Promise.resolve(itemFragment.collection)
() =>
Promise.resolve({
...itemFragment.collection,
creator: wallet.address,
})
)
})

Expand Down Expand Up @@ -1936,7 +1951,11 @@ describe('Item router', () => {
])
;(collectionAPI.fetchCollection as jest.Mock).mockReset()
;(collectionAPI.fetchCollection as jest.Mock).mockImplementationOnce(
() => Promise.resolve(itemFragment.collection)
() =>
Promise.resolve({
...itemFragment.collection,
creator: wallet.address,
})
)
})

Expand All @@ -1954,8 +1973,7 @@ describe('Item router', () => {
expect(response.body).toEqual({
data: {
...Bridge.toFullItem(dbItem),
local_content_hash:
'bafkreiaqn33clj5i7vtdbsmltwrfclrfs4tb3rgdnlzfjtfpdkyxix2e6e',
local_content_hash: expect.any(String),
eth_address: wallet.address,
created_at: dbItem.created_at.toISOString(),
updated_at: currentDate.toISOString(),
Expand Down
65 changes: 59 additions & 6 deletions src/Item/Item.service.spec.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,25 @@
import { dbCollectionMock } from '../../spec/mocks/collections'
import { dbItemMock } from '../../spec/mocks/items'
import { dbItemMock, itemFragmentMock } from '../../spec/mocks/items'
import { Collection } from '../Collection/Collection.model'
import { Bridge } from '../ethereum/api/Bridge'
import { collectionAPI } from '../ethereum/api/collection'
import { UnauthorizedToUpsertError } from './Item.errors'
import { Item } from './Item.model'
import { ItemService } from './Item.service'
import { ItemAttributes } from './Item.types'

jest.mock('../ethereum/api/collection')
jest.mock('../Collection/Collection.model')
jest.mock('./Item.model')

describe('Item Service', () => {
let dbItem: ItemAttributes
describe('isOwnedOrManagedBy', () => {
let service: ItemService
beforeEach(() => {
service = new ItemService()
})
let service: ItemService
beforeEach(() => {
service = new ItemService()
})

describe('isOwnedOrManagedBy', () => {
describe('when the owner is the same as the one in the DB', () => {
beforeEach(() => {
dbItem = { ...dbItemMock, eth_address: '0xoriginalAddress' }
Expand Down Expand Up @@ -46,4 +50,53 @@ describe('Item Service', () => {
})
})
})

describe('upsertItem', () => {
let newCreatorAddress: string
let oldCreatorAddress: string

describe('when the collection db eth_address is not the same as the collectionApi creator', () => {
beforeEach(() => {
newCreatorAddress = '0xnewCreator'
oldCreatorAddress = '0xoldCreator'
dbItem = { ...dbItemMock, eth_address: oldCreatorAddress }
;(Item.findOne as jest.Mock).mockResolvedValueOnce(dbItem)
;(Item.hasPublishedItems as jest.Mock).mockResolvedValue(true)
;(Item.upsert as jest.Mock).mockImplementation((value) => value)
;(Collection.findByIds as jest.Mock).mockImplementation((ids) =>
[dbCollectionMock].filter((collection) => ids.includes(collection.id))
)
;(collectionAPI.fetchCollection as jest.Mock).mockReset()
;(collectionAPI.fetchCollection as jest.Mock).mockImplementation(() =>
Promise.resolve({ ...itemFragmentMock, creator: newCreatorAddress })
)
})

describe('and the creator tries to upsert an item', () => {
it('should return item information', async () => {
expect(
await service.upsertItem(
Bridge.toFullItem(dbItem),
newCreatorAddress
)
).toEqual({
...Bridge.toFullItem(dbItem),
local_content_hash: expect.any(String),
updated_at: expect.anything()
})
})
})

describe('and the previous owner tries to upsert an item', () => {
it('should throw unauthorized', async () => {
expect(
() => service.upsertItem(
Bridge.toFullItem(dbItem),
oldCreatorAddress
)
).rejects.toThrow(UnauthorizedToUpsertError)
})
})
})
})
})
5 changes: 3 additions & 2 deletions src/Item/Item.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export class ItemService {
const collectionId: string | null =
dbItem?.collection_id ?? item.collection_id
if (collectionId) {
dbCollection = await this.collectionService.getDBCollection(collectionId)
dbCollection = await this.collectionService.getCollection(collectionId)
}

// Set the item dates
Expand Down Expand Up @@ -456,7 +456,8 @@ export class ItemService {
))

const isCollectionOwner =
dbCollection && dbCollection.eth_address.toLowerCase() === eth_address
dbCollection &&
dbCollection.eth_address.toLowerCase() === eth_address.toLowerCase()

const isManager =
isDbCollectionPublished &&
Expand Down

0 comments on commit 60c6230

Please sign in to comment.