Skip to content
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.

Sign up for GitHub

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

[ML] Display link to create data view from error cases in data frame analytics results pages #143596

Merged
merged 5 commits into from
Oct 20, 2022
Merged

[ML] Display link to create data view from error cases in data frame analytics results pages #143596

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
098aa10
add link to dataView creation when no dataView
alvarezmelissa87 Oct 18, 2022
7e8031b
update translations
alvarezmelissa87 Oct 19, 2022
c849b80
add util function for destIndex
alvarezmelissa87 Oct 19, 2022
e5f0676
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine Oct 19, 2022
a891812
Merge branch 'main' into ml-dfa-data-view-link
kibanamachine Oct 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion ...es/analytics_exploration/components/exploration_page_wrapper/exploration_page_wrapper.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import { LoadingPanel } from '../loading_panel';
import { FeatureImportanceSummaryPanelProps } from '../total_feature_importance_summary/feature_importance_summary';
import { useExplorationUrlState } from '../../hooks/use_exploration_url_state';
import { ExplorationQueryBarProps } from '../exploration_query_bar/exploration_query_bar';
import { IndexPatternPrompt } from '../index_pattern_prompt';

function getFilters(resultsField: string) {
return {
Expand Down Expand Up @@ -114,6 +115,9 @@ export const ExplorationPageWrapper: FC<Props> = ({
};

const resultsField = jobConfig?.dest.results_field ?? '';
const destIndex =
(Array.isArray(jobConfig?.dest.index) ? jobConfig?.dest.index[0] : jobConfig?.dest.index) ?? '';

const scatterplotFieldOptions = useScatterplotFieldOptions(
indexPattern,
jobConfig?.analyzed_fields?.includes,
Expand All @@ -131,7 +135,12 @@ export const ExplorationPageWrapper: FC<Props> = ({
color="danger"
iconType="cross"
>
<p>{indexPatternErrorMessage}</p>
<p>
{indexPatternErrorMessage}
{needsDestIndexPattern ? (
<IndexPatternPrompt destIndex={destIndex} color="text" />
) : null}
</p>
</EuiCallOut>
</EuiPanel>
);
Expand Down
15 changes: 8 additions & 7 deletions ...tics/pages/analytics_exploration/components/index_pattern_prompt/index_pattern_prompt.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@ import { EuiLink, EuiText } from '@elastic/eui';
import { useMlKibana } from '../../../../../contexts/kibana';

interface Props {
destIndex: string;
color?: string;
destIndex?: string;
}

export const IndexPatternPrompt: FC<Props> = ({ destIndex }) => {
export const IndexPatternPrompt: FC<Props> = ({ destIndex, color }) => {
const {
services: {
http: { basePath },
Expand All @@ -30,20 +31,20 @@ export const IndexPatternPrompt: FC<Props> = ({ destIndex }) => {

return (
<>
<EuiText size="xs" color="warning">
<EuiText size="xs" color={color ?? 'warning'}>
<FormattedMessage
id="xpack.ml.dataframe.analytics.dataViewPromptMessage"
defaultMessage="No data view exists for index {destIndex}. "
defaultMessage="No data view exists for index{destIndex}. "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there supposed to be a space in between for index{destIndex}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming it's like this for when destIndex is undefined. What about when destIndex is defined? It would read "No data view exists for indexmy-index-name. "

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Updated in 7e8031b

values={{
destIndex,
destIndex: destIndex ?? '',
}}
/>
{canCreateDataView === true ? (
<FormattedMessage
id="xpack.ml.dataframe.analytics.dataViewPromptLink"
defaultMessage="{linkToDataViewManagement} for {destIndex}."
defaultMessage="{linkToDataViewManagement}{destIndex}."
values={{
destIndex,
destIndex: destIndex ? ` for ${destIndex}` : '',
linkToDataViewManagement: (
<EuiLink
href={`${basePath.get()}/app/management/kibana/dataViews/create`}
Expand Down
10 changes: 9 additions & 1 deletion ...lytics/pages/analytics_exploration/components/outlier_exploration/outlier_exploration.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { getFeatureCount } from './common';
import { useOutlierData } from './use_outlier_data';
import { useExplorationUrlState } from '../../hooks/use_exploration_url_state';
import { ExplorationQueryBarProps } from '../exploration_query_bar/exploration_query_bar';
import { IndexPatternPrompt } from '../index_pattern_prompt';

export type TableItem = Record<string, any>;

Expand Down Expand Up @@ -90,6 +91,8 @@ export const OutlierExploration: FC<ExplorationProps> = React.memo(({ jobId }) =
jobConfig?.analyzed_fields?.excludes,
resultsField
);
const destIndex =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic is used in several places in DFA like useResultsViewConfig or deleteAnalyticsAndDestIndex... Might be out of scope of this PR but it's better to have a utility function (e.g. getDestinationIndex) so future updates to the logic can be more consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added util function in c849b80

(Array.isArray(jobConfig?.dest.index) ? jobConfig?.dest.index[0] : jobConfig?.dest.index) ?? '';

if (indexPatternErrorMessage !== undefined) {
return (
Expand All @@ -101,7 +104,12 @@ export const OutlierExploration: FC<ExplorationProps> = React.memo(({ jobId }) =
color="danger"
iconType="cross"
>
<p>{indexPatternErrorMessage}</p>
<p>
{indexPatternErrorMessage}
{needsDestIndexPattern ? (
<IndexPatternPrompt destIndex={destIndex} color="text" />
) : null}
</p>
</EuiCallOut>
</EuiPanel>
);
Expand Down
53 changes: 45 additions & 8 deletions ..._frame_analytics/pages/analytics_management/components/action_clone/clone_action_name.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
* 2.0.
*/

import { EuiToolTip } from '@elastic/eui';
import { EuiToolTip, EuiLink, EuiText } from '@elastic/eui';
import React, { FC } from 'react';
import { cloneDeep, isEqual } from 'lodash';
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n-react';
import { toMountPoint, wrapWithTheme } from '@kbn/kibana-react-plugin/public';
import { DeepReadonly } from '../../../../../../../common/types/common';
import { DataFrameAnalyticsConfig, isOutlierAnalysis } from '../../../../common';
import { isClassificationAnalysis, isRegressionAnalysis } from '../../../../common/analytics';
Expand Down Expand Up @@ -401,9 +403,15 @@ export const useNavigateToWizardWithClonedJob = () => {
services: {
notifications: { toasts },
data: { dataViews },
http: { basePath },
application: { capabilities },
theme,
},
} = useMlKibana();
const theme$ = theme.theme$;
const navigateToPath = useNavigateToPath();
const canCreateDataView =
capabilities.savedObjectsManagement.edit === true || capabilities.indexPatterns.save === true;

return async (item: Pick<DataFrameAnalyticsListRow, 'config' | 'stats'>) => {
const sourceIndex = Array.isArray(item.config.source.index)
Expand All @@ -416,13 +424,42 @@ export const useNavigateToWizardWithClonedJob = () => {
if (dv !== undefined) {
sourceIndexId = dv.id;
} else {
toasts.addDanger(
i18n.translate('xpack.ml.dataframe.analyticsList.noSourceDataViewForClone', {
defaultMessage:
'Unable to clone the analytics job. No data view exists for index {dataView}.',
values: { dataView: sourceIndex },
})
);
toasts.addDanger({
title: toMountPoint(
wrapWithTheme(
<>
<FormattedMessage
id="xpack.ml.dataframe.analyticsList.noSourceDataViewForClone"
defaultMessage="Unable to clone the analytics job. No data view exists for index {dataView}."
values={{ dataView: sourceIndex }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super duper nit: The variable name dataView in this message is a bit confusing. Suggestion to rename variable to something like No data view exists for index {sourceIndex}.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in 7e8031b

/>
{canCreateDataView ? (
<EuiText size="xs" color="text">
<FormattedMessage
id="xpack.ml.dataframe.analytics.cloneAction.dataViewPromptLink"
defaultMessage="{linkToDataViewManagement}"
values={{
linkToDataViewManagement: (
<EuiLink
href={`${basePath.get()}/app/management/kibana/dataViews/create`}
target="_blank"
>
<FormattedMessage
id="xpack.ml.dataframe.analytics.cloneAction.dataViewPromptLinkText"
defaultMessage="Create a data view for {dataView}"
values={{ dataView: sourceIndex }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super duper nit: Similarly, the variable name dataView in this message is a bit confusing as the data view has not been created yet. Suggestion to rename variable to something like Create a data view for {sourceIndex}.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in 7e8031b

/>
</EuiLink>
),
}}
/>
</EuiText>
) : null}
</>,
theme$
)
),
});
}
} catch (e) {
const error = extractErrorMessage(e);
Expand Down