diff --git a/app/controllers/admin/topics_controller.rb b/app/controllers/admin/topics_controller.rb index c3b1c6793adb65618efcb21097664477b635f3e4..e97ead12f717964e9bc44de519fe3cf098d99a2c 100644 --- a/app/controllers/admin/topics_controller.rb +++ b/app/controllers/admin/topics_controller.rb @@ -49,11 +49,8 @@ def merge source_topic = Projects::Topic.find(merge_params[:source_topic_id]) target_topic = Projects::Topic.find(merge_params[:target_topic_id]) - begin - ::Topics::MergeService.new(source_topic, target_topic).execute - rescue ArgumentError => e - return render status: :bad_request, json: { type: :alert, message: e.message } - end + response = ::Topics::MergeService.new(source_topic, target_topic).execute + return render status: :bad_request, json: { type: :alert, message: response.message } if response.error? message = _('Topic %{source_topic} was successfully merged into topic %{target_topic}.') flash[:toast] = message % { source_topic: source_topic.name, target_topic: target_topic.name } diff --git a/app/services/topics/merge_service.rb b/app/services/topics/merge_service.rb index 0d256579fe04d7e2396c890e74cf155eed90b857..58f3d5305b48efbcbc911fff7cd1af356488b453 100644 --- a/app/services/topics/merge_service.rb +++ b/app/services/topics/merge_service.rb @@ -17,14 +17,21 @@ def execute refresh_target_topic_counters delete_source_topic end + + ServiceResponse.success + rescue ArgumentError => e + ServiceResponse.error(message: e.message) + rescue StandardError => e + Gitlab::ErrorTracking.track_exception(e, source_topic_id: source_topic.id, target_topic_id: target_topic.id) + ServiceResponse.error(message: _('Topics could not be merged!')) end private def validate_parameters! - raise ArgumentError, 'The source topic is not a topic.' unless source_topic.is_a?(Projects::Topic) - raise ArgumentError, 'The target topic is not a topic.' unless target_topic.is_a?(Projects::Topic) - raise ArgumentError, 'The source topic and the target topic are identical.' if source_topic == target_topic + raise ArgumentError, _('The source topic is not a topic.') unless source_topic.is_a?(Projects::Topic) + raise ArgumentError, _('The target topic is not a topic.') unless target_topic.is_a?(Projects::Topic) + raise ArgumentError, _('The source topic and the target topic are identical.') if source_topic == target_topic end # rubocop: disable CodeReuse/ActiveRecord diff --git a/doc/api/topics.md b/doc/api/topics.md index ee88a43ff1c412eb13488b070862e08025c40f13..38d99244bb6ef04d28170a01a2f057f633c71c54 100644 --- a/doc/api/topics.md +++ b/doc/api/topics.md @@ -217,7 +217,7 @@ curl --request PUT \ > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/80725) in GitLab 14.9. -You must be an administrator to delete a project. +You must be an administrator to delete a project topic. When you delete a project topic, you also delete the topic assignment for projects. ```plaintext @@ -237,3 +237,43 @@ curl --request DELETE \ --header "PRIVATE-TOKEN: " \ "https://gitlab.example.com/api/v4/topics/1" ``` + +## Merge topics + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/95501) in GitLab 15.4. + +You must be an administrator to merge a source topic into a target topic. +When you merge topics, you delete the source topic and move all assigned projects to the target topic. + +```plaintext +POST /topics/merge +``` + +Supported attributes: + +| Attribute | Type | Required | Description | +| ----------------- | ------- | ---------------------- | -------------------------- | +| `source_topic_id` | integer | **{check-circle}** Yes | ID of source project topic | +| `target_topic_id` | integer | **{check-circle}** Yes | ID of target project topic | + +Example request: + +```shell +curl --request POST \ + --data "source_topic_id=2&target_topic_id=1" \ + --header "PRIVATE-TOKEN: " \ + "https://gitlab.example.com/api/v4/topics/merge" +``` + +Example response: + +```json +{ + "id": 1, + "name": "topic1", + "title": "Topic 1", + "description": null, + "total_projects_count": 0, + "avatar_url": null +} +``` diff --git a/lib/api/topics.rb b/lib/api/topics.rb index a08b4c6c1070e7eb16816749a6ab9f8a6ba53040..38cfdc44021adcf9abf9b551fd775c06a0c510d7 100644 --- a/lib/api/topics.rb +++ b/lib/api/topics.rb @@ -94,5 +94,25 @@ class Topics < ::API::Base destroy_conditionally!(topic) end + + desc 'Merge topics' do + detail 'This feature was introduced in GitLab 15.4.' + success Entities::Projects::Topic + end + params do + requires :source_topic_id, type: Integer, desc: 'ID of source project topic' + requires :target_topic_id, type: Integer, desc: 'ID of target project topic' + end + post 'topics/merge' do + authenticated_as_admin! + + source_topic = ::Projects::Topic.find(params[:source_topic_id]) + target_topic = ::Projects::Topic.find(params[:target_topic_id]) + + response = ::Topics::MergeService.new(source_topic, target_topic).execute + render_api_error!(response.message, :bad_request) if response.error? + + present target_topic, with: Entities::Projects::Topic + end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index bcfc87fa07845f680c203cad5c704ec9dac0e86c..9d33cbedb36c731507fb0d99edf15c1b028b7496 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -39553,6 +39553,12 @@ msgstr "" msgid "The source project of this merge request has been removed." msgstr "" +msgid "The source topic and the target topic are identical." +msgstr "" + +msgid "The source topic is not a topic." +msgstr "" + msgid "The specified tab is invalid, please select another" msgstr "" @@ -39565,6 +39571,9 @@ msgstr "" msgid "The tag name can't be changed for an existing release." msgstr "" +msgid "The target topic is not a topic." +msgstr "" + msgid "The time period in seconds that the maximum requests per project limit applies to." msgstr "" @@ -41270,6 +41279,9 @@ msgstr "" msgid "Topics" msgstr "" +msgid "Topics could not be merged!" +msgstr "" + msgid "Total" msgstr "" diff --git a/spec/controllers/admin/topics_controller_spec.rb b/spec/controllers/admin/topics_controller_spec.rb index 87093e0263bbbfafe870f93d9884e3b38e32e6e1..111fdcc3be6c470679686a19cf3a409dfbc74d5c 100644 --- a/spec/controllers/admin/topics_controller_spec.rb +++ b/spec/controllers/admin/topics_controller_spec.rb @@ -194,7 +194,7 @@ end it 'renders a 400 error for identical topic ids' do - post :merge, params: { source_topic_id: topic, target_topic_id: topic.id } + post :merge, params: { source_topic_id: topic.id, target_topic_id: topic.id } expect(response).to have_gitlab_http_status(:bad_request) expect { topic.reload }.not_to raise_error diff --git a/spec/requests/api/topics_spec.rb b/spec/requests/api/topics_spec.rb index 72221e3fb6a8156e71e7e278169fd7850645efaa..1ad6f876fabdfdc6e81d7bd5ea1a79da5ffec7fd 100644 --- a/spec/requests/api/topics_spec.rb +++ b/spec/requests/api/topics_spec.rb @@ -317,4 +317,66 @@ end end end + + describe 'POST /topics/merge', :aggregate_failures do + context 'as administrator' do + let_it_be(:api_url) { api('/topics/merge', admin) } + + it 'merge topics' do + post api_url, params: { source_topic_id: topic_3.id, target_topic_id: topic_2.id } + + expect(response).to have_gitlab_http_status(:created) + expect { topic_2.reload }.not_to raise_error + expect { topic_3.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect(json_response['id']).to eq(topic_2.id) + expect(json_response['total_projects_count']).to eq(topic_2.total_projects_count) + end + + it 'returns 404 for non existing source topic id' do + post api_url, params: { source_topic_id: non_existing_record_id, target_topic_id: topic_2.id } + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'returns 404 for non existing target topic id' do + post api_url, params: { source_topic_id: topic_3.id, target_topic_id: non_existing_record_id } + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'returns 400 for identical topic ids' do + post api_url, params: { source_topic_id: topic_2.id, target_topic_id: topic_2.id } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eql('The source topic and the target topic are identical.') + end + + it 'returns 400 if merge failed' do + allow_next_found_instance_of(Projects::Topic) do |topic| + allow(topic).to receive(:destroy!).and_raise(ActiveRecord::RecordNotDestroyed) + end + + post api_url, params: { source_topic_id: topic_3.id, target_topic_id: topic_2.id } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eql('Topics could not be merged!') + end + end + + context 'as normal user' do + it 'returns 403 Forbidden' do + post api('/topics/merge', user), params: { source_topic_id: topic_3.id, target_topic_id: topic_2.id } + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'as anonymous' do + it 'returns 401 Unauthorized' do + post api('/topics/merge'), params: { source_topic_id: topic_3.id, target_topic_id: topic_2.id } + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + end end diff --git a/spec/services/topics/merge_service_spec.rb b/spec/services/topics/merge_service_spec.rb index 971917eb8e9bd4a5ab2bb543eaf519196718697b..eef31817aa8d42b556fa7e55da1616ac3211ed7c 100644 --- a/spec/services/topics/merge_service_spec.rb +++ b/spec/services/topics/merge_service_spec.rb @@ -30,7 +30,9 @@ it 'reverts previous changes' do allow(source_topic.reload).to receive(:destroy!).and_raise(ActiveRecord::RecordNotDestroyed) - expect { subject }.to raise_error(ActiveRecord::RecordNotDestroyed) + response = subject + expect(response).to be_error + expect(response.message).to eq('Topics could not be merged!') expect(source_topic.projects).to contain_exactly(project_1, project_2, project_4) expect(target_topic.projects).to contain_exactly(project_3, project_4) @@ -50,9 +52,9 @@ with_them do it 'raises correct error' do - expect { subject }.to raise_error(ArgumentError) do |error| - expect(error.message).to eq(expected_message) - end + response = subject + expect(response).to be_error + expect(response.message).to eq(expected_message) end end end