Commit 229e126c authored by Stan Hu's avatar Stan Hu

Fix resolve WIP clearing merge request area

When we split the merge request widget into cached and non-cached
serializers in
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/15045, some merge
request endpoints (e.g. `/remove_wip`, `/cancel_auto_merge`) weren't
adapted to ensure that all the data was returned. As a result, the
frontend would set its merge request state to the partial data it did
have, but then it would clear out data that it didn't have (e.g. CI
pipeline).

When a user clicked on the `Resolve WIP Status` or `Cancel automatic
merge` button, this would cause certain elements (e.g. CI pipeline
status, merge area) to disappear until the next refresh occurred.

We worked around the problem in the MWPS refresh case on the frontend in
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/26232 by forcing a
refresh, but that didn't fix the "Resolve Work in Progress" case. This
fix combines both the cached and non-cached data to ensure the response
data contains all the information necessary for the frontend.

Closes https://gitlab.com/gitlab-org/gitlab/-/issues/214750
parent d549c1e2
...@@ -340,11 +340,13 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo ...@@ -340,11 +340,13 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
end end
def serialize_widget(merge_request) def serialize_widget(merge_request)
serializer.represent(merge_request, serializer: 'widget') cached_data = serializer.represent(merge_request, serializer: 'poll_cached_widget')
widget_data = serializer.represent(merge_request, serializer: 'poll_widget')
cached_data.merge!(widget_data)
end end
def serializer def serializer
MergeRequestSerializer.new(current_user: current_user, project: merge_request.project) @serializer ||= MergeRequestSerializer.new(current_user: current_user, project: merge_request.project)
end end
def define_edit_vars def define_edit_vars
......
...@@ -15,6 +15,10 @@ class MergeRequestSerializer < BaseSerializer ...@@ -15,6 +15,10 @@ class MergeRequestSerializer < BaseSerializer
MergeRequestBasicEntity MergeRequestBasicEntity
when 'noteable' when 'noteable'
MergeRequestNoteableEntity MergeRequestNoteableEntity
when 'poll_cached_widget'
MergeRequestPollCachedWidgetEntity
when 'poll_widget'
MergeRequestPollWidgetEntity
else else
# fallback to widget for old poll requests without `serializer` set # fallback to widget for old poll requests without `serializer` set
MergeRequestWidgetEntity MergeRequestWidgetEntity
......
---
title: Fix resolve WIP clearing merge request area
merge_request: 29757
author:
type: fixed
...@@ -1245,7 +1245,7 @@ describe Projects::MergeRequestsController do ...@@ -1245,7 +1245,7 @@ describe Projects::MergeRequestsController do
end end
it 'renders MergeRequest as JSON' do it 'renders MergeRequest as JSON' do
expect(json_response.keys).to include('id', 'iid') expect(json_response.keys).to include('id', 'iid', 'title', 'has_ci', 'merge_status', 'can_be_merged', 'current_user')
end end
end end
...@@ -1279,7 +1279,7 @@ describe Projects::MergeRequestsController do ...@@ -1279,7 +1279,7 @@ describe Projects::MergeRequestsController do
it 'renders MergeRequest as JSON' do it 'renders MergeRequest as JSON' do
subject subject
expect(json_response.keys).to include('id', 'iid') expect(json_response.keys).to include('id', 'iid', 'title', 'has_ci', 'merge_status', 'can_be_merged', 'current_user')
end end
end end
......
# frozen_string_literal: true
require 'spec_helper'
describe 'Merge request > User resolves Work in Progress', :js do
let(:project) { create(:project, :public, :repository) }
let(:user) { project.creator }
let(:merge_request) do
create(:merge_request_with_diffs, source_project: project,
author: user,
title: 'WIP: Bug NS-04',
merge_params: { force_remove_source_branch: '1' })
end
let(:pipeline) do
create(:ci_pipeline, project: project,
sha: merge_request.diff_head_sha,
ref: merge_request.source_branch,
head_pipeline_of: merge_request)
end
before do
project.add_maintainer(user)
end
context 'when there is active pipeline for merge request' do
before do
create(:ci_build, pipeline: pipeline)
sign_in(user)
visit project_merge_request_path(project, merge_request)
wait_for_requests
end
it 'retains merge request data after clicking Resolve WIP status' do
expect(page.find('.ci-widget-content')).to have_content("Pipeline ##{pipeline.id}")
expect(page).to have_content "This is a Work in Progress"
click_button('Resolve WIP status')
wait_for_requests
# If we don't disable the wait here, the test will wait until the
# merge request widget refreshes, which masks missing elements
# that should already be present.
expect(page.find('.ci-widget-content', wait: 0)).to have_content("Pipeline ##{pipeline.id}")
expect(page).not_to have_content('This is a Work in Progress')
end
end
end
...@@ -69,6 +69,22 @@ describe MergeRequestSerializer do ...@@ -69,6 +69,22 @@ describe MergeRequestSerializer do
end end
end end
context 'poll cached widget merge request serialization' do
let(:serializer) { 'poll_cached_widget' }
it 'matches basic merge request json schema' do
expect(json_entity).to match_schema('entities/merge_request_poll_cached_widget')
end
end
context 'poll widget merge request serialization' do
let(:serializer) { 'poll_widget' }
it 'matches basic merge request json schema' do
expect(json_entity).to match_schema('entities/merge_request_poll_widget')
end
end
context 'no serializer' do context 'no serializer' do
let(:serializer) { nil } let(:serializer) { nil }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment