-
Notifications
You must be signed in to change notification settings - Fork 97
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: extend type CommentProps for new status marker #1856
Conversation
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.
LGTM
export type CommentProps = { | ||
sys: CommentSysProps | ||
body: string | ||
status?: CommentStatus |
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.
issue: I think status
is always available and defaulting to active
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.
yes! for get, status
is always there, just or PUT/POST you do not need to provide it.
see: https://github.com/contentful/comments-api/blob/master/src/api/types.ts
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.
Then we would need to update these types for those cases:
contentful-management.js/lib/entities/comment.ts
Lines 52 to 55 in c80524c
export type CreateCommentProps = Omit<CommentProps, 'sys'> | |
export type UpdateCommentProps = Omit<CommentProps, 'sys'> & { | |
sys: Pick<CommentSysProps, 'version'> | |
} |
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.
@Lelith comments that were created before this property existed will return active
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.
ok, will change it to non optional :)
Do we have to mark this release then as breaking / major update? |
closing for now, will update in a different story |
Summary
The comments api got extended and delivers now a status field. This field is meant to represent the comment-thread status. Comments can now be marked as resolved, in order to help organize conversations in comments. The currently available states are 'active' and 'resolved'