diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 4d8daed28d9..0ca8d83c64a 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -7aa06a578d76bdc294ee8e9acb4f063e7d9f1d5f +1f796d76e13237a957abccde4a07fb5b2d177241 diff --git a/app/models/draft_note.rb b/app/models/draft_note.rb index b2848bf2cff..6f5c7436f06 100644 --- a/app/models/draft_note.rb +++ b/app/models/draft_note.rb @@ -5,7 +5,7 @@ class DraftNote < ApplicationRecord include Sortable include ShaAttribute - PUBLISH_ATTRS = %i[noteable_id noteable_type type note internal].freeze + PUBLISH_ATTRS = %i[noteable type note internal].freeze DIFF_ATTRS = %i[position original_position change_position commit_id].freeze sha_attribute :commit_id diff --git a/app/services/notes/build_service.rb b/app/services/notes/build_service.rb index 91993700e25..96df14aa574 100644 --- a/app/services/notes/build_service.rb +++ b/app/services/notes/build_service.rb @@ -20,7 +20,11 @@ module Notes discussion = discussion.convert_to_discussion! if discussion.can_convert_to_discussion? - params.merge!(discussion.reply_attributes) + reply_attributes = discussion.reply_attributes + # NOTE: Avoid overriding noteable if it already exists so that we don't have to reload noteable. + reply_attributes = reply_attributes.except(:noteable_id, :noteable_type) if params[:noteable] + + params.merge!(reply_attributes) end # The `confidential` param for notes is deprecated with 15.3 diff --git a/qa/qa/specs/features/api/5_package/container_registry/saas/container_registry_spec.rb b/qa/qa/specs/features/api/5_package/container_registry/saas/container_registry_spec.rb index 8ce462ad59e..83704521d0f 100644 --- a/qa/qa/specs/features/api/5_package/container_registry/saas/container_registry_spec.rb +++ b/qa/qa/specs/features/api/5_package/container_registry/saas/container_registry_spec.rb @@ -59,7 +59,7 @@ module QA image: #{ci_test_image} stage: test script: - - 'id=$(curl --header "PRIVATE-TOKEN: #{masked_token}" "https://${CI_SERVER_HOST}/api/v4/projects/#{project.id}/registry/repositories" | jq ".[0].id")' + - 'id=$(curl --header "PRIVATE-TOKEN: #{masked_token}" "https://${CI_SERVER_HOST}/api/v4/projects/#{project.id}/registry/repositories" | tac | jq ".[0].id")' - echo $id - 'tag_count=$(curl --header "PRIVATE-TOKEN: #{masked_token}" "https://${CI_SERVER_HOST}/api/v4/projects/#{project.id}/registry/repositories/$id/tags" | jq ". | length")' - if [ $tag_count -ne 1 ]; then exit 1; fi; diff --git a/spec/services/draft_notes/publish_service_spec.rb b/spec/services/draft_notes/publish_service_spec.rb index ba4c579d391..3822752b90b 100644 --- a/spec/services/draft_notes/publish_service_spec.rb +++ b/spec/services/draft_notes/publish_service_spec.rb @@ -224,19 +224,17 @@ RSpec.describe DraftNotes::PublishService, feature_category: :code_review_workfl end end - context 'with many draft notes', :use_sql_query_cache do + context 'with many draft notes', :use_sql_query_cache, :request_store do let(:merge_request) { create(:merge_request) } it 'reduce N+1 queries' do - create(:draft_note_on_text_diff, merge_request: merge_request, author: user, note: 'note 1') - create(:draft_note_on_text_diff, merge_request: merge_request, author: user, note: 'note 2') - create(:draft_note_on_text_diff, merge_request: merge_request, author: user, note: 'note 3') - create(:draft_note_on_text_diff, merge_request: merge_request, author: user, note: 'note 4') - create(:draft_note_on_text_diff, merge_request: merge_request, author: user, note: 'note 5') + 5.times do + create(:draft_note_on_discussion, merge_request: merge_request, author: user, note: 'some note') + end recorder = ActiveRecord::QueryRecorder.new(skip_cached: false) { publish } - expect(recorder.count).not_to be > 186 + expect(recorder.count).not_to be > 105 end end @@ -303,6 +301,7 @@ RSpec.describe DraftNotes::PublishService, feature_category: :code_review_workfl refresh = MergeRequests::RefreshService.new(project: project, current_user: user) refresh.execute(oldrev, newrev, merge_request.source_branch_ref) + merge_request.reload expect { publish(draft: draft) }.to change { Suggestion.count }.by(1) .and change { DiffNote.count }.from(0).to(1)