-
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
[home] Include ability to publish kibana saved objects from add data tutorial #19559
Changes from 3 commits
b0f6537
a63831a
6decd9d
6eaeb6a
c3cd774
23928e0
bc739f5
822627c
7f9c845
c73f6e9
63db6f9
fc2064d
560b4a9
9ff43d3
e5000e3
7bc56e1
232a760
12e29b3
ef0be0e
6175825
cb3f8de
8ea193d
e037c91
e0cb81b
66faeb3
a4ad403
c0e25ad
7cd8938
7e108c0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,137 @@ | ||
/* | ||
* Licensed to Elasticsearch B.V. under one or more contributor | ||
* license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright | ||
* ownership. Elasticsearch B.V. licenses this file to you under | ||
* the Apache License, Version 2.0 (the "License"); you may | ||
* not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, | ||
* software distributed under the License is distributed on an | ||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
* KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
|
||
import React, { Fragment } from 'react'; | ||
import PropTypes from 'prop-types'; | ||
|
||
import { | ||
EuiSteps, | ||
EuiFlexGroup, | ||
EuiFlexItem, | ||
EuiText, | ||
EuiButton, | ||
EuiSpacer, | ||
EuiCallOut, | ||
} from '@elastic/eui'; | ||
|
||
const INCOMPLETE = 'incomplete'; | ||
const COMPLETE = 'complete'; | ||
|
||
export class SavedObjectsInstaller extends React.Component { | ||
constructor(props) { | ||
super(props); | ||
|
||
this.state = { | ||
isInstalling: false, | ||
installStatus: INCOMPLETE, | ||
}; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you don't need the constructor for anything else, you can set the shorthand: export class SavedObjectsInstaller extends React.Component {
state = {
isInstalling: false,
installStatus: INCOMPLETE,
};
} |
||
|
||
installSavedObjects = async () => { | ||
this.setState({ | ||
isInstalling: true, | ||
}); | ||
|
||
const resp = await this.props.bulkCreate(this.props.savedObjects, { overwrite: true }); | ||
const errors = resp.savedObjects.filter(savedObjectCreateResult => { | ||
return savedObjectCreateResult.hasOwnProperty('error'); | ||
}); | ||
|
||
let statusMsg = `${this.props.savedObjects.length} saved objects successfully added`; | ||
if (errors.length > 0) { | ||
statusMsg = `Unable to load kibana saved objects, Error: ${errors[0]}`; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer |
||
|
||
this.setState({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need to use the isMounted check as this component may have been unmounted due to the async call:
|
||
isInstalling: false, | ||
installStatusMsg: statusMsg, | ||
installStatus: errors.length === 0 ? COMPLETE : INCOMPLETE, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of having two different checks for error ( This way they won't somehow come out of sync in a future change. |
||
}); | ||
} | ||
|
||
renderInstallMessage() { | ||
if (!this.state.installStatusMsg) { | ||
return; | ||
} | ||
|
||
return ( | ||
<EuiCallOut | ||
title={this.state.installStatusMsg} | ||
color={this.state.installStatus === COMPLETE ? 'success' : 'warning'} | ||
/> | ||
); | ||
} | ||
|
||
renderInstallStep() { | ||
const installStep = ( | ||
<Fragment> | ||
<EuiFlexGroup justifyContent="spaceBetween" alignItems="center"> | ||
<EuiFlexItem> | ||
<EuiText> | ||
<p> | ||
Click button to add Saved Searches, Visualizations, and Dashboards for module | ||
</p> | ||
</EuiText> | ||
</EuiFlexItem> | ||
|
||
<EuiFlexItem | ||
grow={false} | ||
> | ||
<EuiButton | ||
onClick={this.installSavedObjects} | ||
isLoading={this.state.isInstalling} | ||
> | ||
Add | ||
</EuiButton> | ||
</EuiFlexItem> | ||
</EuiFlexGroup> | ||
|
||
<EuiSpacer size="s" /> | ||
|
||
{this.renderInstallMessage()} | ||
</Fragment> | ||
); | ||
|
||
return { | ||
title: 'Load Kibana objects', | ||
status: this.state.installStatus, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If status only has two modes, I think it would be better to make it a boolean. |
||
children: installStep, | ||
key: 'installStep' | ||
}; | ||
} | ||
|
||
render() { | ||
return ( | ||
<EuiSteps | ||
steps={[this.renderInstallStep()]} | ||
/> | ||
); | ||
} | ||
} | ||
|
||
const savedObjectShape = PropTypes.shape({ | ||
id: PropTypes.string.isRequired, | ||
type: PropTypes.string.isRequired, | ||
attributes: PropTypes.object.isRequired, | ||
}); | ||
|
||
SavedObjectsInstaller.propTypes = { | ||
bulkCreate: PropTypes.func.isRequired, | ||
savedObjects: PropTypes.arrayOf(savedObjectShape).isRequired, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ import chrome from 'ui/chrome'; | |
import routes from 'ui/routes'; | ||
import template from './home_ng_wrapper.html'; | ||
import { FeatureCatalogueRegistryProvider } from 'ui/registry/feature_catalogue'; | ||
import { SavedObjectsClientProvider } from 'ui/saved_objects'; | ||
import { uiModules } from 'ui/modules'; | ||
import { | ||
HomeApp | ||
|
@@ -37,6 +38,7 @@ function getRoute() { | |
return { | ||
template, | ||
controller($scope, config, indexPatterns, Private) { | ||
const savedObjectsClient = Private(SavedObjectsClientProvider); | ||
$scope.addBasePath = chrome.addBasePath; | ||
$scope.directories = Private(FeatureCatalogueRegistryProvider).inTitleOrder; | ||
$scope.recentlyAccessed = recentlyAccessed.get().map(item => { | ||
|
@@ -49,6 +51,7 @@ function getRoute() { | |
const getter = indexPatterns.getIds; | ||
getter.clearCache(); | ||
}; | ||
$scope.bulkCreate = savedObjectsClient.bulkCreate; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we made a mistake by coupling rest clients to Angular in the past. Instead they should have been framework-agnostic static functions. If we keep coupling new code (in this case React) to Angular we make it harder to switch away from Angular, and increase technical debt. It's easier to use an existing client but I don't think the alternative is much more difficult. You don't have to re-implement the entire savedObjectsClient - just create a starting point with the endpoints you need. Afaict this is all you need: export function bulkCreate(savedObjects, { overwrite }) {
return kfetch({
pathname: `/api/saved_objects/_bulk_create`,
method: 'POST',
query: {
overwrite
},
body: JSON.stringify(savedObjects)
});
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @epixa Do we already have a strategy for consuming Angular services from outside Angular - and how to migrate them? I've already spent a fair amount of time de-angularizing some services and it would be beneficial if everyone were aligned. If we don't have anything I'm open for doing a write-up of the approach I've used, and how I see us moving forward. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do not think this is a problem and is exactly how we are interacting with saved objects in other react components. From react's perspective, it is just an injected function. Eventually, the savedObjectClient will be decoupled from angular but all saved object interaction should go through the savedObjectClient. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Maybe I don't know enough about the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The savedObjectClient converts the results into savedObject classes and camel cases the object keys. But more importantly, it puts all calls to the saved object API in a single place so if the API ever changes, only the savedObjectClient has to be updated instead of hunting down lots of segregated fetches. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Totally agree. IMHO it is okay temporarily to have two "clients": the old Angular client, and a new framework agnostic service. Over time the Angular client could even call the framework agnostic client, and eventually the Angular client would be dropped. That's just my view, but interested to hear what other ideas are floating around (ping @epixa ;) ) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Btw. I think it's fine to leave it as is. Just my opinion on how to do it going forward. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not in this PR, but I'd rather just remove the angular dependency from the saved object client we have today. It shouldn't be terribly difficult, we'd just need to update any references to the saved object client from angular code to invoke $scope.$apply() after the call. |
||
} | ||
}; | ||
} | ||
|
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 consistency
installStatus
should be a boolean likeisInstalling
(and namedisComplete
).