Commit 0d000d35 authored by Felipe Artur's avatar Felipe Artur

Fix issuable stale object error handler for js when updating tasklists

parent 61e2a3eb
/* global Flash */
require('vendor/task_list'); require('vendor/task_list');
class TaskList { class TaskList {
...@@ -6,6 +7,16 @@ 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 { ...@@ -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 ...@@ -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 ...@@ -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 ...@@ -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
......
---
title: Fix issuable stale object error handler for js when updating tasklists
merge_request:
author:
...@@ -125,14 +125,16 @@ describe Projects::IssuesController do ...@@ -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] }
......
...@@ -254,6 +254,8 @@ describe Projects::MergeRequestsController do ...@@ -254,6 +254,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
......
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
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