-
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
[Core] Make type option required for SavedObjects.find #42236
Conversation
Pinging @elastic/kibana-platform |
098be95
to
10fd3e6
Compare
854ec04
to
f82aa42
Compare
f82aa42
to
0313b50
Compare
💚 Build Succeeded |
type: noteSavedObjectType, | ||
...options, | ||
}); | ||
const savedObjects = await savedObjectsClient.find(options); |
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.
@XavierM making type
required in SavedObjectsFindOptions means we either have to include the type in every getAllSavedNote()
call, or change the signature of getAllSavedNote()
's options parameter to not be SavedObjectsFindOptions
.
💚 Build Succeeded |
@elastic/siem Anyone able to review these minor changes I made to the SIEM plugin? |
💚 Build Succeeded |
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, Make sense!
* Make type option required for SavedObjects.find * getSortedObjectsForExport test for type or objects
Summary
Make the type option required for
SavedObjectsFindOptions
the interface used bySavedObjects.find()
since not supplying will throw a runtime exception https://github.com/elastic/kibana/blob/master/src/core/server/saved_objects/service/lib/repository.ts#L409.Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.This was checked for cross-browser compatibility, including a check against IE11Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n supportDocumentation was added for features that require explanation or tutorialsUnit or functional tests were updated or added to match the most common scenariosThis was checked for keyboard-only and screenreader accessibilityFor maintainers
This was checked for breaking API changes and was labeled appropriatelyThis includes a feature addition or change that requires a release note and was labeled appropriately