diff --git a/app/graphql/mutations/ml/models/delete.rb b/app/graphql/mutations/ml/models/delete.rb
new file mode 100644
index 0000000000000000000000000000000000000000..6791eddab295659dff7010cdfd070dcfd894bc44
--- /dev/null
+++ b/app/graphql/mutations/ml/models/delete.rb
@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+module Mutations
+ module Ml
+ module Models
+ class Delete < Base
+ graphql_name 'MlModelDelete'
+
+ include FindsProject
+
+ argument :id, ::Types::GlobalIDType[::Ml::Model],
+ required: true,
+ description: 'Global ID of the model to be deleted.'
+
+ def resolve(**args)
+ project = authorized_find!(args[:project_path])
+
+ model = ::Ml::Model.by_project_id_and_id(project.id, args[:id].model_id)
+
+ return { errors: ['Model not found'] } unless model
+
+ result = ::Ml::DestroyModelService.new(model, current_user).execute
+
+ {
+ model: result.payload[:model],
+ errors: result.errors
+ }
+ end
+ end
+ end
+ end
+end
diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb
index fd8233d5ebf01daf929548614653fa10d3d732d3..96601c035a036c176d40f1416d673759a1c1a292 100644
--- a/app/graphql/types/mutation_type.rb
+++ b/app/graphql/types/mutation_type.rb
@@ -210,6 +210,7 @@ class MutationType < BaseObject
mount_mutation Mutations::Admin::AbuseReportLabels::Create, alpha: { milestone: '16.4' }
mount_mutation Mutations::Ml::Models::Create, alpha: { milestone: '16.8' }
mount_mutation Mutations::Ml::Models::Destroy, alpha: { milestone: '16.10' }
+ mount_mutation Mutations::Ml::Models::Delete, alpha: { milestone: '17.0' }
mount_mutation Mutations::Ml::ModelVersions::Delete, alpha: { milestone: '17.0' }
mount_mutation Mutations::BranchRules::Delete, alpha: { milestone: '16.9' }
end
diff --git a/app/services/ml/destroy_model_service.rb b/app/services/ml/destroy_model_service.rb
index 29df70b3c37d64e5c1e918de27b9af3646e7cc3d..f2a33d58cd88282838ba6dd885ff0fd802e56c34 100644
--- a/app/services/ml/destroy_model_service.rb
+++ b/app/services/ml/destroy_model_service.rb
@@ -8,14 +8,14 @@ def initialize(model, user)
end
def execute
- return error unless @model.destroy
-
package_deletion_result = ::Packages::MarkPackagesForDestructionService.new(
packages: @model.all_packages,
current_user: @user
).execute
- return packages_not_deleted if package_deletion_result.error?
+ return packages_not_deleted(package_deletion_result.message) if package_deletion_result.error?
+
+ return error unless @model.destroy
success
end
@@ -23,15 +23,19 @@ def execute
private
def success
- ServiceResponse.success(message: _('Model was successfully deleted'))
+ ServiceResponse.success(payload: payload)
end
def error
- ServiceResponse.error(message: _('Failed to delete model'))
+ ServiceResponse.error(message: @model.errors.full_messages, payload: payload)
+ end
+
+ def packages_not_deleted(error_message)
+ ServiceResponse.error(message: error_message, payload: payload)
end
- def packages_not_deleted
- ServiceResponse.success(message: _('Model deleted but failed to remove associated packages'))
+ def payload
+ { model: @model }
end
end
end
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index 75d7f9b959439b11cecc4de3e7c2f2eae37c22b7..3950ee66bbd33db4c08537a4652a36b7aa90f375 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -6593,6 +6593,30 @@ Input type: `MlModelCreateInput`
| <a id="mutationmlmodelcreateerrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. |
| <a id="mutationmlmodelcreatemodel"></a>`model` | [`MlModel`](#mlmodel) | Model after mutation. |
+### `Mutation.mlModelDelete`
+
+DETAILS:
+**Introduced** in GitLab 17.0.
+**Status**: Experiment.
+
+Input type: `MlModelDeleteInput`
+
+#### Arguments
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| <a id="mutationmlmodeldeleteclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
+| <a id="mutationmlmodeldeleteid"></a>`id` | [`MlModelID!`](#mlmodelid) | Global ID of the model to be deleted. |
+| <a id="mutationmlmodeldeleteprojectpath"></a>`projectPath` | [`ID!`](#id) | Project the model to mutate is in. |
+
+#### Fields
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| <a id="mutationmlmodeldeleteclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
+| <a id="mutationmlmodeldeleteerrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. |
+| <a id="mutationmlmodeldeletemodel"></a>`model` | [`MlModel`](#mlmodel) | Model after mutation. |
+
### `Mutation.mlModelDestroy`
DETAILS:
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index c6beca4fa7c0b828f369927571718b34891bb278..221a5594cb768dc15182f6d7ad199284a9849464 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -21562,9 +21562,6 @@ msgstr ""
msgid "Failed to delete custom emoji. Please try again."
msgstr ""
-msgid "Failed to delete model"
-msgstr ""
-
msgid "Failed to deploy to"
msgstr ""
@@ -33120,9 +33117,6 @@ msgstr ""
msgid "Modal|Close"
msgstr ""
-msgid "Model deleted but failed to remove associated packages"
-msgstr ""
-
msgid "Model experiments"
msgstr ""
@@ -33132,9 +33126,6 @@ msgstr ""
msgid "Model version not found"
msgstr ""
-msgid "Model was successfully deleted"
-msgstr ""
-
msgid "ModelRegistry|Model registry"
msgstr ""
diff --git a/spec/requests/api/graphql/mutations/ml/models/delete_spec.rb b/spec/requests/api/graphql/mutations/ml/models/delete_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..b502bff52ef0f25b6eb4447a729c82d9ee138bd3
--- /dev/null
+++ b/spec/requests/api/graphql/mutations/ml/models/delete_spec.rb
@@ -0,0 +1,110 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'Deleting a model', feature_category: :mlops do
+ using RSpec::Parameterized::TableSyntax
+
+ include GraphqlHelpers
+
+ let_it_be_with_reload(:model) { create(:ml_models) }
+ let_it_be(:user) { create(:user) }
+
+ let(:project) { model.project }
+ let(:id) { model.to_global_id.to_s }
+
+ let(:query) do
+ <<~GQL
+ model {
+ id
+ }
+ errors
+ GQL
+ end
+
+ let(:params) { { project_path: project.full_path, id: id } }
+ let(:mutation) { graphql_mutation(:ml_model_delete, params, query) }
+ let(:mutation_response) { graphql_mutation_response(:ml_model_delete) }
+
+ shared_examples 'destroying the model' do
+ it 'destroys model' do
+ expect(::Ml::DestroyModelService).to receive(:new).with(model, user).and_call_original
+
+ expect { mutation_request }.to change { ::Ml::Model.count }.by(-1)
+ expect(mutation_response['model']).to eq({ "id" => id })
+ end
+
+ it_behaves_like 'returning response status', :success
+ end
+
+ shared_examples 'denying the mutation request' do
+ it 'does not delete the model' do
+ expect(::Ml::DestroyModelService).not_to receive(:new)
+
+ expect { mutation_request }.to not_change { ::Ml::Model.count }
+
+ expect(mutation_response).to be_nil
+ expect_graphql_errors_to_include("The resource that you are attempting to access does not exist or you don't " \
+ "have permission to perform this action")
+ end
+
+ it_behaves_like 'returning response status', :success
+ end
+
+ shared_examples 'model was not found' do
+ it 'does not delete the model' do
+ expect(::Ml::DestroyModelService).not_to receive(:new)
+
+ expect { mutation_request }.to not_change { ::Ml::Model.count }
+
+ expect(mutation_response["errors"]).to match_array(['Model not found'])
+ end
+
+ it_behaves_like 'returning response status', :success
+ end
+
+ describe 'post graphql mutation' do
+ subject(:mutation_request) { post_graphql_mutation(mutation, current_user: user) }
+
+ context 'with valid id' do
+ where(:user_role, :mutation_behavior) do
+ :reporter | 'destroying the model'
+ :guest | 'denying the mutation request'
+ end
+
+ with_them do
+ before do
+ project.public_send("add_#{user_role}", user)
+ end
+
+ it_behaves_like params[:mutation_behavior]
+ end
+ end
+
+ context 'with authorized user' do
+ before do
+ project.add_maintainer(user)
+ end
+
+ context 'with invalid id' do
+ let(:params) { { project_path: project.full_path, id: "gid://gitlab/Ml::Model/#{non_existing_record_id}" } }
+
+ it_behaves_like 'model was not found'
+ end
+
+ context 'when an error occurs' do
+ it 'returns the errors in the response' do
+ allow_next_found_instance_of(::Ml::Model) do |model|
+ allow(model).to receive(:destroy).and_return(nil)
+ errors = ActiveModel::Errors.new(model).tap { |e| e.add(:id, 'some error') }
+ allow(model).to receive(:errors).and_return(errors)
+ end
+
+ mutation_request
+
+ expect(mutation_response['errors']).to match_array(['Id some error'])
+ end
+ end
+ end
+ end
+end
diff --git a/spec/requests/api/graphql/mutations/ml/models/destroy_spec.rb b/spec/requests/api/graphql/mutations/ml/models/destroy_spec.rb
index bd849f30c4e320c3208829ab322151c01897d738..7cf4cbf14dc4d294c0abf719aebe459992f3f62b 100644
--- a/spec/requests/api/graphql/mutations/ml/models/destroy_spec.rb
+++ b/spec/requests/api/graphql/mutations/ml/models/destroy_spec.rb
@@ -106,11 +106,13 @@
it 'returns the errors in the response' do
allow_next_found_instance_of(::Ml::Model) do |model|
allow(model).to receive(:destroy).and_return(nil)
+ errors = ActiveModel::Errors.new(model).tap { |e| e.add(:id, 'some error') }
+ allow(model).to receive(:errors).and_return(errors)
end
mutation_request
- expect(mutation_response['errors']).to match_array(['Failed to delete model'])
+ expect(mutation_response['errors']).to match_array(['Id some error'])
end
end
end
diff --git a/spec/services/ml/destroy_model_service_spec.rb b/spec/services/ml/destroy_model_service_spec.rb
index fcbafc0a57145cb9b2d1086c70dbc4e7ef4ddfa7..0e21d9dc39c9b58ae52a95d7d8e3528648a5bcbb 100644
--- a/spec/services/ml/destroy_model_service_spec.rb
+++ b/spec/services/ml/destroy_model_service_spec.rb
@@ -36,14 +36,14 @@
before do
allow_next_instance_of(Packages::MarkPackagesForDestructionService) do |instance|
- allow(instance).to receive(:execute).and_return ServiceResponse.error(message: "")
+ allow(instance).to receive(:execute).and_return ServiceResponse.error(message: "An error")
end
end
it 'returns success with warning', :aggregate_failures do
- expect { service_result }.to change { Ml::Model.count }.by(-1).and change { Ml::ModelVersion.count }.by(-1)
- expect(service_result).to be_success
- expect(service_result.message).to eq('Model deleted but failed to remove associated packages')
+ expect { service_result }.not_to change { Ml::Model.count }
+ expect(service_result).to be_error
+ expect(service_result.message).to eq("An error")
end
end
end