Commit a96507bf authored by Lin Jen-Shin's avatar Lin Jen-Shin

Introduce ServiceResponse to wrap around response

See https://gitlab.com/gitlab-org/gitlab-ce/issues/60730
parent 98f898d3
......@@ -100,14 +100,14 @@ class Projects::BranchesController < Projects::ApplicationController
respond_to do |format|
format.html do
flash_type = result[:status] == :error ? :alert : :notice
flash[flash_type] = result[:message]
flash_type = result.error? ? :alert : :notice
flash[flash_type] = result.message
redirect_to project_branches_path(@project), status: :see_other
end
format.js { head result[:return_code] }
format.json { render json: { message: result[:message] }, status: result[:return_code] }
format.js { head result.http_status }
format.json { render json: { message: result.message }, status: result.http_status }
end
end
......
......@@ -6,27 +6,25 @@ class DeleteBranchService < BaseService
branch = repository.find_branch(branch_name)
unless current_user.can?(:push_code, project)
return error('You dont have push access to repo', 405)
return ServiceResponse.error(
message: 'You dont have push access to repo',
http_status: 405)
end
unless branch
return error('No such branch', 404)
return ServiceResponse.error(
message: 'No such branch',
http_status: 404)
end
if repository.rm_branch(current_user, branch_name)
success('Branch was deleted')
ServiceResponse.success(message: 'Branch was deleted')
else
error('Failed to remove branch')
ServiceResponse.error(
message: 'Failed to remove branch',
http_status: 400)
end
rescue Gitlab::Git::PreReceiveError => ex
error(ex.message)
end
def error(message, return_code = 400)
super(message).merge(return_code: return_code)
end
def success(message)
super().merge(message: message)
ServiceResponse.error(message: ex.message, http_status: 400)
end
end
# frozen_string_literal: true
class ServiceResponse
def self.success(message: nil)
new(status: :success, message: message)
end
def self.error(message:, http_status: nil)
new(status: :error, message: message, http_status: http_status)
end
attr_reader :status, :message, :http_status
def initialize(status:, message: nil, http_status: nil)
self.status = status
self.message = message
self.http_status = http_status
end
def success?
status == :success
end
def error?
status == :error
end
private
attr_writer :status, :message, :http_status
end
......@@ -162,8 +162,8 @@ module API
result = DeleteBranchService.new(user_project, current_user)
.execute(params[:branch])
if result[:status] != :success
render_api_error!(result[:message], result[:return_code])
if result.error?
render_api_error!(result.message, result.http_status)
end
end
end
......
......@@ -19,7 +19,7 @@ describe DeleteBranchService do
result = service.execute('feature')
expect(result[:status]).to eq :success
expect(result.status).to eq :success
expect(branch_exists?('feature')).to be false
end
end
......@@ -30,8 +30,8 @@ describe DeleteBranchService do
result = service.execute('feature')
expect(result[:status]).to eq :error
expect(result[:message]).to eq 'You dont have push access to repo'
expect(result.status).to eq :error
expect(result.message).to eq 'You dont have push access to repo'
expect(branch_exists?('feature')).to be true
end
end
......
# frozen_string_literal: true
require 'fast_spec_helper'
ActiveSupport::Dependencies.autoload_paths << 'app/services'
describe ServiceResponse do
describe '.success' do
it 'creates a successful response without a message' do
expect(described_class.success).to be_success
end
it 'creates a successful response with a message' do
response = described_class.success(message: 'Good orange')
expect(response).to be_success
expect(response.message).to eq('Good orange')
end
end
describe '.error' do
it 'creates a failed response without HTTP status' do
response = described_class.error(message: 'Bad apple')
expect(response).to be_error
expect(response.message).to eq('Bad apple')
end
it 'creates a failed response with HTTP status' do
response = described_class.error(message: 'Bad apple', http_status: 400)
expect(response).to be_error
expect(response.message).to eq('Bad apple')
expect(response.http_status).to eq(400)
end
end
describe '#success?' do
it 'returns true for a successful response' do
expect(described_class.success.success?).to eq(true)
end
it 'returns false for a failed response' do
expect(described_class.error(message: 'Bad apple').success?).to eq(false)
end
end
describe '#error?' do
it 'returns false for a successful response' do
expect(described_class.success.error?).to eq(false)
end
it 'returns true for a failed response' do
expect(described_class.error(message: 'Bad apple').error?).to eq(true)
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