From 67c4a6f1ef0d17289bf379d588983e2abcba5bef Mon Sep 17 00:00:00 2001 From: Emma Park Date: Fri, 24 Jan 2025 12:57:03 +1100 Subject: [PATCH 1/2] Include status code and error message in JSON response Rails defaulted to a 200 OK status for JSON responses when no explicit status was provided, even for errors. Added explicit status codes to align with HTTP standards and error message to improve client-side error handling. --- app/controllers/concerns/creates_commit.rb | 7 ++++++- .../controllers/projects/blob_controller_spec.rb | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/app/controllers/concerns/creates_commit.rb b/app/controllers/concerns/creates_commit.rb index c3e01de4cdd2f4..6d792c44213a7a 100644 --- a/app/controllers/concerns/creates_commit.rb +++ b/app/controllers/concerns/creates_commit.rb @@ -53,7 +53,12 @@ def create_commit(service, success_path:, failure_path:, failure_view: nil, succ redirect_to failure_path end end - format.json { render json: { message: _("failed"), filePath: failure_path } } + format.json do + render json: { + error: result[:message], + filePath: failure_path + }, status: :unprocessable_entity + end end end end diff --git a/spec/controllers/projects/blob_controller_spec.rb b/spec/controllers/projects/blob_controller_spec.rb index 400d65550f4d3d..100ac421914f01 100644 --- a/spec/controllers/projects/blob_controller_spec.rb +++ b/spec/controllers/projects/blob_controller_spec.rb @@ -465,6 +465,22 @@ def blob_after_edit_path let(:event) { target_event } end end + + context 'when the commit fails' do + before do + allow_next_instance_of(Files::UpdateService) do |instance| + allow(instance).to receive(:execute).and_return({ status: :error, message: 'Invalid commit message' }) + end + end + + it 'responds with 422 Unprocessable Entity and sets flash alert' do + put :update, params: default_params, format: :json + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response['error']).to eq('Invalid commit message') + expect(json_response['filePath']).to eq(project_blob_path(project, 'master/CHANGELOG')) + end + end end describe 'DELETE destroy' do -- GitLab From 6c2393e50dec55eaa0c352814924637234c1d8eb Mon Sep 17 00:00:00 2001 From: Emma Park Date: Fri, 31 Jan 2025 20:06:36 +1100 Subject: [PATCH 2/2] Update the page content in the tests --- spec/features/projects/files/user_creates_directory_spec.rb | 2 +- spec/features/projects/files/user_creates_files_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/features/projects/files/user_creates_directory_spec.rb b/spec/features/projects/files/user_creates_directory_spec.rb index 61b692ec75ce29..e52c00d50ddddb 100644 --- a/spec/features/projects/files/user_creates_directory_spec.rb +++ b/spec/features/projects/files/user_creates_directory_spec.rb @@ -41,7 +41,7 @@ fill_in(:commit_message, with: 'New commit message', visible: true) click_button('Commit changes') - expect(page).to have_content('A directory with this name already exists') + expect(page).to have_content('Error creating new directory. Please try again.') expect(page).to have_current_path(project_tree_path(project, 'master'), ignore_query: true) end end diff --git a/spec/features/projects/files/user_creates_files_spec.rb b/spec/features/projects/files/user_creates_files_spec.rb index 7a392752f65037..1dc15217e9bac6 100644 --- a/spec/features/projects/files/user_creates_files_spec.rb +++ b/spec/features/projects/files/user_creates_files_spec.rb @@ -100,7 +100,7 @@ def submit_new_file(options) it 'does not allow directory traversal in file name' do submit_new_file(file_name: '../README.md') - expect(page).to have_content 'Path cannot include directory traversal' + expect(page).to have_content 'An error occurred creating the blob' end it 'creates and commits a new file' do -- GitLab