-
Notifications
You must be signed in to change notification settings - Fork 17
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
Conversation
@@ -282,6 +291,7 @@ export class CollectionRouter extends Router { | |||
return { | |||
collection: toFullCollection(result.collection), | |||
items: result.items, | |||
itemCurations: result.itemCurations, |
There was a problem hiding this comment.
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.
updated_at: this.getISODate(), | ||
}, | ||
{ id: curation.id } | ||
return await curationService.updateStatusAndReturnById( |
There was a problem hiding this comment.
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
@@ -14,7 +14,11 @@ export const patchCurationSchema = Object.freeze({ | |||
properties: { | |||
status: { | |||
type: 'string', | |||
enum: [CurationStatus.APPROVED, CurationStatus.REJECTED], | |||
enum: [ | |||
CurationStatus.PENDING, |
There was a problem hiding this comment.
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.
Pull Request Test Coverage Report for Build 1958254663
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small comment
src/Collection/Collection.router.ts
Outdated
): Promise<{ | ||
collection: FullCollection | ||
items: FullItem[] | ||
itemCurations?: ItemCurationAttributes[] | ||
}> => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might make sense to type this with a type
of its own? cause is being used here, on the result definition, and on the service
it could be used in publishDCLCollection
too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, I added PublishCollectionResponse
with a generic for the type of collection to return.
publishCollection
returns a FullCollection
but this.service.publishTPCollection
returns CollectionAttributes
for the collections
attribute so I had to add a generic to the type to support both (mainly because they are not compatible)
Please notice that the PR is created against
feat/tp-publish
because it includes many of the changes included in that PR.