[ML] Fix deletion of models that are not used by pipelines

x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/action_delete/use_delete_action.tsx

@@ -89,9 +89,9 @@ export const useDeleteAction = (canDeleteDataFrameAnalytics: boolean) => {
       );
     }
   };
-  const checkUserIndexPermission = () => {
+  const checkUserIndexPermission = async () => {
     try {
-      const userCanDelete = canDeleteIndex(indexName, toastNotificationService);
+      const userCanDelete = await canDeleteIndex(indexName, toastNotificationService);
       if (userCanDelete) {
         setUserCanDeleteIndex(true);
       }

x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/models_management/delete_models_modal.tsx

@@ -30,7 +30,11 @@ export const DeleteModelsModal: FC<DeleteModelsModalProps> = ({ models, onClose
     .map((model) => model.model_id);
 
   return (
-    <EuiModal onClose={onClose.bind(null, false)} initialFocus="[name=cancelModelDeletion]">
+    <EuiModal
+      onClose={onClose.bind(null, false)}
+      initialFocus="[name=cancelModelDeletion]"
+      data-test-subj="mlModelsDeleteModal"
+    >
       <EuiModalHeader>
         <EuiModalHeaderTitle>
           <FormattedMessage
@@ -72,7 +76,12 @@ export const DeleteModelsModal: FC<DeleteModelsModalProps> = ({ models, onClose
           />
         </EuiButtonEmpty>
 
-        <EuiButton onClick={onClose.bind(null, true)} fill color="danger">
+        <EuiButton
+          onClick={onClose.bind(null, true)}
+          fill
+          color="danger"
+          data-test-subj="mlModelsDeleteModalConfirmButton"
+        >
           <FormattedMessage
             id="xpack.ml.trainedModels.modelsList.deleteModal.deleteButtonLabel"
             defaultMessage="Delete"

x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/models_management/models_list.tsx

@@ -50,6 +50,7 @@ import {
 import { ML_PAGES } from '../../../../../../../common/constants/locator';
 import { DataFrameAnalysisConfigType } from '../../../../../../../common/types/data_frame_analytics';
 import { timeFormatter } from '../../../../../../../common/util/date_utils';
+import { isPopulatedObject } from '../../../../../../../common';
 import { ListingPageUrlState } from '../../../../../../../common/types/common';
 import { usePageUrlState } from '../../../../../util/url_state';
 import { BUILT_IN_MODEL_TAG } from '../../../../../../../common/constants/data_frame_analytics';
@@ -342,6 +343,7 @@ export const ModelsList: FC = () => {
       description: i18n.translate('xpack.ml.trainedModels.modelsList.deleteModelActionLabel', {
         defaultMessage: 'Delete model',
       }),
+      'data-test-subj': 'mlModelsTableRowDeleteAction',
       icon: 'trash',
       type: 'icon',
       color: 'danger',
@@ -353,7 +355,7 @@ export const ModelsList: FC = () => {
       enabled: (item) => {
         // TODO check for permissions to delete ingest pipelines.
         // ATM undefined means pipelines fetch failed server-side.
-        return !item.pipelines;
+        return !isPopulatedObject(item.pipelines);
       },
     },
   ];
@@ -389,6 +391,7 @@ export const ModelsList: FC = () => {
           iconType={itemIdToExpandedRowMap[item.model_id] ? 'arrowUp' : 'arrowDown'}
         />
       ),
+      'data-test-subj': 'mlModelsTableRowDetailsToggle',
     },
     {
       field: ModelsTableToConfigMapping.id,
@@ -397,6 +400,7 @@ export const ModelsList: FC = () => {
       }),
       sortable: true,
       truncateText: true,
+      'data-test-subj': 'mlModelsTableColumnId',
     },
     {
       field: ModelsTableToConfigMapping.description,
@@ -406,6 +410,7 @@ export const ModelsList: FC = () => {
       }),
       sortable: false,
       truncateText: true,
+      'data-test-subj': 'mlModelsTableColumnDescription',
     },
     {
       field: ModelsTableToConfigMapping.type,
@@ -418,11 +423,14 @@ export const ModelsList: FC = () => {
         <EuiFlexGroup gutterSize={'xs'} wrap>
           {types.map((type) => (
             <EuiFlexItem key={type} grow={false}>
-              <EuiBadge color="hollow">{type}</EuiBadge>
+              <EuiBadge color="hollow" data-test-subj="mlModelType">
+                {type}
+              </EuiBadge>
             </EuiFlexItem>
           ))}
         </EuiFlexGroup>
       ),
+      'data-test-subj': 'mlModelsTableColumnType',
     },
     {
       field: ModelsTableToConfigMapping.createdAt,
@@ -432,12 +440,14 @@ export const ModelsList: FC = () => {
       dataType: 'date',
       render: timeFormatter,
       sortable: true,
+      'data-test-subj': 'mlModelsTableColumnCreatedAt',
     },
     {
       name: i18n.translate('xpack.ml.trainedModels.modelsList.actionsHeader', {
         defaultMessage: 'Actions',
       }),
       actions,
+      'data-test-subj': 'mlModelsTableColumnActions',
     },
   ];
 
@@ -492,8 +502,7 @@ export const ModelsList: FC = () => {
               defaultMessage: 'Select a model',
             });
           }
-
-          if (Array.isArray(item.pipelines) && item.pipelines.length > 0) {
+          if (isPopulatedObject(item.pipelines)) {
             return i18n.translate('xpack.ml.trainedModels.modelsList.disableSelectableMessage', {
               defaultMessage: 'Model has associated pipelines',
             });
@@ -507,7 +516,7 @@ export const ModelsList: FC = () => {
 
           return '';
         },
-        selectable: (item) => !item.pipelines && !isBuiltInModel(item),
+        selectable: (item) => !isPopulatedObject(item.pipelines) && !isBuiltInModel(item),
         onSelectionChange: (selectedItems) => {
           setSelectedModels(selectedItems);
         },
@@ -574,6 +583,7 @@ export const ModelsList: FC = () => {
           pagination={pagination}
           onTableChange={onTableChange}
           sorting={sorting}
+          data-test-subj={isLoading ? 'mlModelsTable loading' : 'mlModelsTable loaded'}
         />
       </div>
       {modelsToDelete.length > 0 && (

x-pack/test/functional/apps/ml/data_frame_analytics/trained_models.ts

@@ -12,7 +12,7 @@ export default function ({ getService }: FtrProviderContext) {
 
   describe('trained models', function () {
     before(async () => {
-      await ml.trainedModels.createdTestTrainedModels('classification', 15);
+      await ml.trainedModels.createdTestTrainedModels('classification', 15, true);
       await ml.trainedModels.createdTestTrainedModels('regression', 15);
       await ml.securityUI.loginAsMlPowerUser();
       await ml.navigation.navigateToTrainedModels();
@@ -22,10 +22,114 @@ export default function ({ getService }: FtrProviderContext) {
       await ml.api.cleanMlIndices();
     });
 
+    // 'Created at' will be different on each run,
+    // so we will just assert that the value is in the expected timestamp format.
+    const builtInModelData = {
+      modelId: 'lang_ident_model_1',
+      description: 'Model used for identifying language from arbitrary input text.',
+      modelTypes: ['classification', 'built-in'],
+    };
+
+    const modelWithPipelineData = {
+      modelId: 'dfa_classification_model_n_0',
+      description: '',
+      modelTypes: ['classification'],
+    };
+
+    const modelWithoutPipelineData = {
+      modelId: 'dfa_regression_model_n_0',
+      description: '',
+      modelTypes: ['regression'],
+    };
+
     it('renders trained models list', async () => {
-      await ml.trainedModels.assertRowsNumberPerPage(10);
+      await ml.testExecution.logTestStep(
+        'should display the stats bar with the total number of models'
+      );
       // +1 because of the built-in model
       await ml.trainedModels.assertStats(31);
+
+      await ml.testExecution.logTestStep('should display the table');
+      await ml.trainedModels.assertTableExists();
+      await ml.trainedModels.assertRowsNumberPerPage(10);
+    });
+
+    it('displays the built-in model and no actions are enabled', async () => {
+      await ml.testExecution.logTestStep('should display the model in the table');
+      await ml.trainedModelsTable.filterWithSearchString(builtInModelData.modelId, 1);
+
+      await ml.testExecution.logTestStep('displays expected row values for the model in the table');
+      await ml.trainedModelsTable.assertModelsRowFields(builtInModelData.modelId, {
+        id: builtInModelData.modelId,
+        description: builtInModelData.description,
+        modelTypes: builtInModelData.modelTypes,
+      });
+
+      await ml.testExecution.logTestStep(
+        'should not show collapsed actions menu for the model in the table'
+      );
+      await ml.trainedModelsTable.assertModelCollapsedActionsButtonExists(
+        builtInModelData.modelId,
+        false
+      );
+
+      await ml.testExecution.logTestStep(
+        'should not show delete action for the model in the table'
+      );
+      await ml.trainedModelsTable.assertModelDeleteActionButtonExists(
+        builtInModelData.modelId,
+        false
+      );
+    });
+
+    it('displays a model with an ingest pipeline and delete action is disabled', async () => {
+      await ml.testExecution.logTestStep('should display the model in the table');
+      await ml.trainedModelsTable.filterWithSearchString(modelWithPipelineData.modelId, 1);
+
+      await ml.testExecution.logTestStep('displays expected row values for the model in the table');
+      await ml.trainedModelsTable.assertModelsRowFields(modelWithPipelineData.modelId, {
+        id: modelWithPipelineData.modelId,
+        description: modelWithPipelineData.description,
+        modelTypes: modelWithPipelineData.modelTypes,
+      });
+
+      await ml.testExecution.logTestStep(
+        'should show disabled delete action for the model in the table'
+      );
+
+      await ml.trainedModelsTable.assertModelDeleteActionButtonEnabled(
+        modelWithPipelineData.modelId,
+        false
+      );
+    });
+
+    it('displays a model without an ingest pipeline and model can be deleted', async () => {
+      await ml.testExecution.logTestStep('should display the model in the table');
+      await ml.trainedModelsTable.filterWithSearchString(modelWithoutPipelineData.modelId, 1);
+
+      await ml.testExecution.logTestStep('displays expected row values for the model in the table');
+      await ml.trainedModelsTable.assertModelsRowFields(modelWithoutPipelineData.modelId, {
+        id: modelWithoutPipelineData.modelId,
+        description: modelWithoutPipelineData.description,
+        modelTypes: modelWithoutPipelineData.modelTypes,
+      });
+
+      await ml.testExecution.logTestStep(
+        'should show enabled delete action for the model in the table'
+      );
+
+      await ml.trainedModelsTable.assertModelDeleteActionButtonEnabled(
+        modelWithoutPipelineData.modelId,
+        true
+      );
+
+      await ml.testExecution.logTestStep('should show the delete modal');
+      await ml.trainedModelsTable.clickDeleteAction(modelWithoutPipelineData.modelId);
+      await ml.trainedModelsTable.assertDeleteModalExists();
+
+      await ml.testExecution.logTestStep('should delete the model');
+      await ml.trainedModelsTable.confirmDeleteModel();
+      await ml.trainedModelsTable.assertRowNotExists(modelWithoutPipelineData.modelId);
     });
   });
 }

x-pack/test/functional/services/ml/api.ts

@@ -981,7 +981,7 @@ export function MachineLearningAPIProvider({ getService }: FtrProviderContext) {
         .expect(200)
         .then((res: any) => res.body);
 
-      log.debug('> Trained model crated');
+      log.debug('> Trained model created');
       return model;
     },
 

x-pack/test/functional/services/ml/index.ts

@@ -51,6 +51,7 @@ import { SwimLaneProvider } from './swim_lane';
 import { MachineLearningDashboardJobSelectionTableProvider } from './dashboard_job_selection_table';
 import { MachineLearningDashboardEmbeddablesProvider } from './dashboard_embeddables';
 import { TrainedModelsProvider } from './trained_models';
+import { TrainedModelsTableProvider } from './trained_models_table';
 import { MachineLearningJobAnnotationsProvider } from './job_annotations_table';
 
 export function MachineLearningProvider(context: FtrProviderContext) {
@@ -121,6 +122,7 @@ export function MachineLearningProvider(context: FtrProviderContext) {
   const alerting = MachineLearningAlertingProvider(context, commonUI);
   const swimLane = SwimLaneProvider(context);
   const trainedModels = TrainedModelsProvider(context, api, commonUI);
+  const trainedModelsTable = TrainedModelsTableProvider(context);
 
   return {
     anomaliesTable,
@@ -168,5 +170,6 @@ export function MachineLearningProvider(context: FtrProviderContext) {
     testExecution,
     testResources,
     trainedModels,
+    trainedModelsTable,
   };
 }

x-pack/test/functional/services/ml/trained_models.ts

@@ -18,15 +18,26 @@ export function TrainedModelsProvider(
   mlCommonUI: MlCommonUI
 ) {
   const testSubjects = getService('testSubjects');
+  const retry = getService('retry');
 
   return {
-    async createdTestTrainedModels(modelType: ModelType, count: number = 10) {
-      await mlApi.createdTestTrainedModels(modelType, count);
+    async createdTestTrainedModels(
+      modelType: ModelType,
+      count: number = 10,
+      withIngestPipelines = false
+    ) {
+      await mlApi.createdTestTrainedModels(modelType, count, withIngestPipelines);
     },
 
     async assertStats(expectedTotalCount: number) {
-      const actualStats = await testSubjects.getVisibleText('mlInferenceModelsStatsBar');
-      expect(actualStats).to.eql(`Total trained models: ${expectedTotalCount}`);
+      await retry.tryForTime(5 * 1000, async () => {
+        const actualStats = await testSubjects.getVisibleText('mlInferenceModelsStatsBar');
+        expect(actualStats).to.eql(`Total trained models: ${expectedTotalCount}`);
+      });
+    },
+
+    async assertTableExists() {
+      await testSubjects.existOrFail('~mlModelsTable');
     },
 
     async assertRowsNumberPerPage(rowsNumber: 10 | 25 | 100) {