Merge branch 'issue_24815' into 'master'
fix ERR_CONTENT_LENGTH_MISMATCH on task checkboxes See merge request !8567
This commit is contained in:
commit
dfd15596a4
|
@ -1,3 +1,4 @@
|
||||||
|
/* global Flash */
|
||||||
require('vendor/task_list');
|
require('vendor/task_list');
|
||||||
|
|
||||||
class TaskList {
|
class TaskList {
|
||||||
|
@ -6,6 +7,16 @@ class TaskList {
|
||||||
this.dataType = options.dataType;
|
this.dataType = options.dataType;
|
||||||
this.fieldName = options.fieldName;
|
this.fieldName = options.fieldName;
|
||||||
this.onSuccess = options.onSuccess || (() => {});
|
this.onSuccess = options.onSuccess || (() => {});
|
||||||
|
this.onError = function showFlash(response) {
|
||||||
|
let errorMessages = '';
|
||||||
|
|
||||||
|
if (response.responseJSON) {
|
||||||
|
errorMessages = response.responseJSON.errors.join(' ');
|
||||||
|
}
|
||||||
|
|
||||||
|
return new Flash(errorMessages || 'Update failed', 'alert');
|
||||||
|
};
|
||||||
|
|
||||||
this.init();
|
this.init();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -32,6 +43,7 @@ class TaskList {
|
||||||
url: $target.data('update-url') || $('form.js-issuable-update').attr('action'),
|
url: $target.data('update-url') || $('form.js-issuable-update').attr('action'),
|
||||||
data: patchData,
|
data: patchData,
|
||||||
success: this.onSuccess,
|
success: this.onSuccess,
|
||||||
|
error: this.onError,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -26,6 +26,23 @@ module IssuableActions
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
|
def render_conflict_response
|
||||||
|
respond_to do |format|
|
||||||
|
format.html do
|
||||||
|
@conflict = true
|
||||||
|
render :edit
|
||||||
|
end
|
||||||
|
|
||||||
|
format.json do
|
||||||
|
render json: {
|
||||||
|
errors: [
|
||||||
|
"Someone edited this #{issuable.human_class_name} at the same time you did. Please refresh your browser and make sure your changes will not unintentionally remove theirs."
|
||||||
|
]
|
||||||
|
}, status: 409
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
def labels
|
def labels
|
||||||
@labels ||= LabelsFinder.new(current_user, project_id: @project.id).execute
|
@labels ||= LabelsFinder.new(current_user, project_id: @project.id).execute
|
||||||
end
|
end
|
||||||
|
|
|
@ -134,8 +134,7 @@ class Projects::IssuesController < Projects::ApplicationController
|
||||||
end
|
end
|
||||||
|
|
||||||
rescue ActiveRecord::StaleObjectError
|
rescue ActiveRecord::StaleObjectError
|
||||||
@conflict = true
|
render_conflict_response
|
||||||
render :edit
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def referenced_merge_requests
|
def referenced_merge_requests
|
||||||
|
|
|
@ -296,22 +296,21 @@ class Projects::MergeRequestsController < Projects::ApplicationController
|
||||||
def update
|
def update
|
||||||
@merge_request = MergeRequests::UpdateService.new(project, current_user, merge_request_params).execute(@merge_request)
|
@merge_request = MergeRequests::UpdateService.new(project, current_user, merge_request_params).execute(@merge_request)
|
||||||
|
|
||||||
if @merge_request.valid?
|
respond_to do |format|
|
||||||
respond_to do |format|
|
format.html do
|
||||||
format.html do
|
if @merge_request.valid?
|
||||||
redirect_to([@merge_request.target_project.namespace.becomes(Namespace),
|
redirect_to([@merge_request.target_project.namespace.becomes(Namespace), @merge_request.target_project, @merge_request])
|
||||||
@merge_request.target_project, @merge_request])
|
else
|
||||||
end
|
render :edit
|
||||||
format.json do
|
|
||||||
render json: @merge_request.to_json(include: { milestone: {}, assignee: { methods: :avatar_url }, labels: { methods: :text_color } }, methods: [:task_status, :task_status_short])
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
else
|
|
||||||
render "edit"
|
format.json do
|
||||||
|
render json: @merge_request.to_json(include: { milestone: {}, assignee: { methods: :avatar_url }, labels: { methods: :text_color } }, methods: [:task_status, :task_status_short])
|
||||||
|
end
|
||||||
end
|
end
|
||||||
rescue ActiveRecord::StaleObjectError
|
rescue ActiveRecord::StaleObjectError
|
||||||
@conflict = true
|
render_conflict_response
|
||||||
render :edit
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def remove_wip
|
def remove_wip
|
||||||
|
|
|
@ -0,0 +1,4 @@
|
||||||
|
---
|
||||||
|
title: Fix issuable stale object error handler for js when updating tasklists
|
||||||
|
merge_request:
|
||||||
|
author:
|
|
@ -125,14 +125,16 @@ describe Projects::IssuesController do
|
||||||
end
|
end
|
||||||
|
|
||||||
describe 'PUT #update' do
|
describe 'PUT #update' do
|
||||||
|
before do
|
||||||
|
sign_in(user)
|
||||||
|
project.team << [user, :developer]
|
||||||
|
end
|
||||||
|
|
||||||
|
it_behaves_like 'update invalid issuable', Issue
|
||||||
|
|
||||||
context 'when moving issue to another private project' do
|
context 'when moving issue to another private project' do
|
||||||
let(:another_project) { create(:empty_project, :private) }
|
let(:another_project) { create(:empty_project, :private) }
|
||||||
|
|
||||||
before do
|
|
||||||
sign_in(user)
|
|
||||||
project.team << [user, :developer]
|
|
||||||
end
|
|
||||||
|
|
||||||
context 'when user has access to move issue' do
|
context 'when user has access to move issue' do
|
||||||
before { another_project.team << [user, :reporter] }
|
before { another_project.team << [user, :reporter] }
|
||||||
|
|
||||||
|
|
|
@ -255,6 +255,8 @@ describe Projects::MergeRequestsController do
|
||||||
|
|
||||||
expect { merge_request.reload.target_branch }.not_to change { merge_request.target_branch }
|
expect { merge_request.reload.target_branch }.not_to change { merge_request.target_branch }
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it_behaves_like 'update invalid issuable', MergeRequest
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -0,0 +1,57 @@
|
||||||
|
shared_examples 'update invalid issuable' do |klass|
|
||||||
|
let(:params) do
|
||||||
|
{
|
||||||
|
namespace_id: project.namespace.path,
|
||||||
|
project_id: project.path,
|
||||||
|
id: issuable.iid
|
||||||
|
}
|
||||||
|
end
|
||||||
|
|
||||||
|
let(:issuable) do
|
||||||
|
klass == Issue ? issue : merge_request
|
||||||
|
end
|
||||||
|
|
||||||
|
before do
|
||||||
|
if klass == Issue
|
||||||
|
params.merge!(issue: { title: "any" })
|
||||||
|
else
|
||||||
|
params.merge!(merge_request: { title: "any" })
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when updating causes conflicts' do
|
||||||
|
before do
|
||||||
|
allow_any_instance_of(issuable.class).to receive(:save).
|
||||||
|
and_raise(ActiveRecord::StaleObjectError.new(issuable, :save))
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'renders edit when format is html' do
|
||||||
|
put :update, params
|
||||||
|
|
||||||
|
expect(response).to render_template(:edit)
|
||||||
|
expect(assigns[:conflict]).to be_truthy
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'renders json error message when format is json' do
|
||||||
|
params.merge!(format: "json")
|
||||||
|
|
||||||
|
put :update, params
|
||||||
|
|
||||||
|
expect(response.status).to eq(409)
|
||||||
|
expect(JSON.parse(response.body)).to have_key('errors')
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when updating an invalid issuable' do
|
||||||
|
before do
|
||||||
|
key = klass == Issue ? :issue : :merge_request
|
||||||
|
params[key][:title] = ""
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'renders edit when merge request is invalid' do
|
||||||
|
put :update, params
|
||||||
|
|
||||||
|
expect(response).to render_template(:edit)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
Loading…
Reference in New Issue