-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[Cases] Renaming comment type #156086
[Cases] Renaming comment type #156086
Conversation
Pinging @elastic/response-ops (Team:ResponseOps) |
Pinging @elastic/response-ops-cases (Feature:Cases) |
@@ -233,7 +233,7 @@ export const CommentResponseTypePersistableStateRt = rt.intersection([ | |||
}), | |||
]); | |||
|
|||
export const AllCommentsResponseRT = rt.array(CommentResponseRt); | |||
export const AllCommentsResponseRT = rt.array(CommentRt); |
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.
I think this should be CommentsRt
and the CommentsRt
should be CommentsFindResponseRt
. We can do it on another PR as we have the same issue for Cases
.
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.
Also, this seems like a duplicate of AllCommentsResponseRt
. They differ in the last letter (capital T
).
@@ -185,7 +185,7 @@ export const CommentRequestRt = rt.union([ | |||
PersistableStateAttachmentRt, | |||
]); | |||
|
|||
export const CommentResponseRt = rt.intersection([ | |||
export const CommentRt = rt.intersection([ |
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.
Should we use the new terminology and rename it to AttachmentRt
? I am not sure 🙂.
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.
Great idea, we'll do those renames on a separate PR.
@@ -142,14 +142,14 @@ export const transformComments = ( | |||
|
|||
export const flattenCommentSavedObjects = ( | |||
savedObjects: Array<SavedObject<CommentAttributes>> |
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.
We can create the type type CommentSavedObjectTransformed = SavedObject<AttachmentTransformedAttributes>
and use it here.
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.
These utilities are used by the client, I'll create the type you recommended and use it within the service layer 👍
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Page load bundle
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
This PR:
CommentResponseRt
toCommentRt
CommentUI
for the UI based on the snake to camel case version ofComment
AttachmentTransformedAttributes
type that should be used when returningCommentAttributes
from the service layer