Migrate saved object to typescript and deangularisation #51562
Conversation
💔 Build Failed
|
|
Jenkins, test this |
💔 Build Failed
|
kertal
force-pushed
the
kertal-pr-2019-11-22-np-saved-object
branch
from
48be33a to
e24bc6b
Compare
💔 Build Failed |
💚 Build Succeeded |
💔 Build Failed |
💚 Build Succeeded |
💚 Build Succeeded |
kertal
changed the title
lukeelmers
left a comment
lukeelmers
left a comment
There was a problem hiding this comment.
Overall code LGTM, with the caveat that I don't have a ton of expertise on this area of Kibana. Might be worth getting another opinion before merging.
There's still plenty that will need to be done here to prep for NP migration, but I'm assuming you are intentionally keeping the scope of this PR to TypeScript, so I mainly focused on those areas.
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
flash1293
left a comment
flash1293
left a comment
There was a problem hiding this comment.
LGTM besides some small nits
| } | ||
|
|
||
| export async function buildGlobalAngularServices() { | ||
| const injector = await chromeLegacy.dangerouslyGetActiveInjector(); |
| public static fieldOrder = ['title', 'description']; | ||
| public static searchSource = true; | ||
|
|
||
| public id: string; |
There was a problem hiding this comment.
Those can be removed now because they are already defined in the base type
There was a problem hiding this comment.
I adapted this change, since it makes sense, but had to revert, because for some reason inheritance doesn't work in this case (Functional tests were failing). could be because we don't inherit directly but generate the Class to inherit from. A pattern we should and will change.
| return savedObject; | ||
| } catch (e) { | ||
| // eslint-disable-next-line no-console | ||
| console.error(e); |
There was a problem hiding this comment.
console log for debugging?
There was a problem hiding this comment.
life saving during development, but now redundant! thx! removed
|
@kertal How do I test this PR please? Thanks! |
|
@bhavyarm loading and saving saved searches, visualization, dashboard + timelion app. Also trying to edit these saved objects in management |
💔 Build FailedHistory
To update your PR or re-run it, just comment with: |
7 participants
Summary
Part of #46435
This PR migrates
ui/saved_objectscode to typescript, deangularizes it, converts to usage of native Promises and async/await, makes use of new platform services. Furthermore it extracts functions of the migratedsaved_object.js, nowsaved_object.ts, that make the move to a new platform migrated service easier, the code more modular.There's now an alternative to
New:
It's now used in Discover, and will make the use of the 'global' Angular redundant (#49483).
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support- [ ] Documentation was added for features that require explanation or tutorials- [ ] Unit or functional tests were updated or added to match the most common scenarios- [ ] This was checked for keyboard-only and screenreader accessibilityFor maintainers
- [ ] This was checked for breaking API changes and was labeled appropriately