-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Maps] convert SavedGisMap to TS #72286
Conversation
Pinging @elastic/kibana-gis (Team:Geo) |
.map((reference) => { | ||
return reference.id; | ||
}); | ||
savedObject.indexPatternIds = _.uniq(indexPatternIds); |
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.
savedObject.indexPatternIds
was added when MapEmbeddable was first introduced in https://github.com/elastic/kibana/pull/31473/files#diff-1bb10565f2889677ad2ded8210390726R50. At the time MapEmbeddableFactory used savedObject.indexPatternIds
to obtain index patterns used by the map
#35542 changed the way MapEmbeddableFactory obtained indexPatternIds to pulling them from store, acb78a6#diff-e347d1b056afd109215cc604d63d4aefL47. However, that PR failed to clean up setting indexPatternIds on savedMap.
Since savedMap.indexPatternIds is no longer used, it is safe to remove this code
💛 Build succeeded, but was flaky
Test FailuresKibana Pipeline / kibana-xpack-agent / X-Pack Alerting API Integration Tests.x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/alerts·ts.alerting api integration security and spaces enabled Alerts alerts space_1_all at space1 should schedule task, run alert and schedule actions when appropriateStandard Out
Stack Trace
Build metricsasync chunks size
To update your PR or re-run it, just comment with: |
|
||
getFullPath() { | ||
return getExistingMapPath(this.id); | ||
this.getFullPath = () => { |
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.
Why is this function getting created in the constructor?
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.
typescripting. Something about functional method versus prototypemethod.
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.
Sounds related to the fact that it's now an arrow function. Does it need to be? It doesn't look like this PR changes any calls to it
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.
for typescripting purposes, yes. I had to copy what SavedDashboard was doing to get TS to be happy.
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! thanks for the conversion
- code review
* [Maps] convert SavedGisMap to TS * i18n translate new map title
* master: [Observability] Remove app logos (elastic#72259) Fix float percentiles line chart (elastic#71902) update chromedriver to 84 (elastic#72228) [esArchiver] actually re-delete the .kibana index if we lose recreate race (elastic#72354) [Maps] convert SavedGisMap to TS (elastic#72286) [DOCS] Removes occurrences of X-Pack Security and Reporting (elastic#72302) use WORKSPACE env var for stack_functional_integration tests, fix navigate path (elastic#71908) [Monitoring] Fix issue with ES node detail status (elastic#72298) [SIEM] Updates consumer in export_rule archive (elastic#72324) [kbn/dev-utils] add RunWithCommands utility (elastic#72311) [Security Solution][Endpoint][Exceptions] Only write manifest to policy when there are changes (elastic#72000) skip flaky suite (elastic#72339) [ML] Fix annotations pagination & change labels from letters to numbers (elastic#72204) [Lens] Fix switching with layers (elastic#71982) [Maps] 7.9 documenation updates (elastic#71893) docs: ✏️ add "Explore underlying data" user docs (elastic#70807) [Security Solution][Exceptions] - Remove initial add exception item button in builder (elastic#72215) Fix indentation level in code exploration doc (elastic#72274) register graph usage (elastic#72041) [Monitoring] Added a case for Alerting if security/ssl is disabled (elastic#71846)
Breaking this out into a really small PR to isolate since some logic in
injectReferences
is removed. After a lot of github sleuthing, this logic is safe to remove. See inline comment for details.