-
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] Split remaining cases types #155444
[Cases] Split remaining cases types #155444
Conversation
Pinging @elastic/response-ops (Team:ResponseOps) |
Pinging @elastic/response-ops-cases (Feature:Cases) |
* 2.0. | ||
*/ | ||
|
||
export type ConnectorFieldsPersistedAttributes = Array<{ |
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.
nit: What do you think about CasePersistedConnectorFields
?
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.
How do you feel about dropping the Case
part? I think it's kind of redundant.
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.
What about ConnectorPersistedFields
? I am fine with all options 🙂. Same for CasePersistedConnector
.
value: unknown; | ||
}>; | ||
|
||
export interface ConnectorPersistedAttributes { |
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.
nit: What do you think about CasePersistedConnector
?
@@ -41,7 +41,7 @@ export const createMappings = async ( | |||
refresh, | |||
}); | |||
|
|||
return theMapping.attributes.mappings; | |||
return theMapping.attributes.mappings as ConnectorMappingsAttributes[]; |
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 that if we define the return type of the functions and cast in ConnectorMappingsService
we can avoid the casting here. Same for the all functions of the configure
folder.
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.
LGMT! Should we remove the ESCaseConnector
, ESConnectorFields
, and ESCasesConfigureAttributes
types?
… into cases-split-types-others
I was thinking the same when looking at this PR, at least wrt
As for |
Yeah I can't find references to these anymore. Although I don't remember removing them 😆 are they still there? |
💔 Build FailedFailed CI Steps
Test Failures
Metrics [docs]
History
To update your PR or re-run it, just comment with: |
It seems they are gone now! |
This PR separates the persisted SO attributes from the fields received in the HTTP API requests. This is to address step 2 in preparing our HTTP routes as versioned. Issue: #153726 This PR encompasses a few PRs here which have been reviewed individually by the cases team members: #155325 - User actions #155440 - Attachments #155444 - Configure, Connector Mappings #155277 - Cases The large number of files is because of renaming some types throughout the frontend code. There shouldn't be many functionality changes, mostly just type changes. --------- Co-authored-by: Christos Nasikas <christos.nasikas@elastic.co> Co-authored-by: Patryk Kopyciński <contact@patrykkopycinski.com>
This PR separates the http API io-ts types from the types that are used in the cases service layer to interact with the saved object client. This PR is splits up the remaining types within the service layer (configure and connector mappings).
Issue: #153726