Skip to content

Commit

Permalink
Fixed that 200 API responses have a body containing one space (#11388).
Browse files Browse the repository at this point in the history
git-svn-id: svn://rubyforge.org/var/svn/redmine/trunk@9975 e93f8b46-1217-0410-a6f0-8f06a7374b81
  • Loading branch information
jplang committed Jul 14, 2012
1 parent 557b8bc commit 0fab00f
Show file tree
Hide file tree
Showing 19 changed files with 48 additions and 19 deletions.
6 changes: 6 additions & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,12 @@ def query_statement_invalid(exception)
render_error "An error occurred while executing the query and has been logged. Please report this error to your Redmine administrator."
end

# Renders a 200 response for successfull updates or deletions via the API
def render_api_ok
# head :ok would return a response body with one space
render :text => '', :status => :ok, :layout => nil
end

# Renders API response on validation failure
def render_validation_errors(objects)
if objects.is_a?(Array)
Expand Down
8 changes: 4 additions & 4 deletions app/controllers/groups_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def update
if @group.save
flash[:notice] = l(:notice_successful_update)
format.html { redirect_to(groups_path) }
format.api { head :ok }
format.api { render_api_ok }
else
format.html { render :action => "edit" }
format.api { render_validation_errors(@group) }
Expand All @@ -85,7 +85,7 @@ def destroy

respond_to do |format|
format.html { redirect_to(groups_url) }
format.api { head :ok }
format.api { render_api_ok }
end
end

Expand All @@ -100,7 +100,7 @@ def add_users
users.each {|user| page.visual_effect(:highlight, "user-#{user.id}") }
}
}
format.api { head :ok }
format.api { render_api_ok }
end
end

Expand All @@ -109,7 +109,7 @@ def remove_user
respond_to do |format|
format.html { redirect_to :controller => 'groups', :action => 'edit', :id => @group, :tab => 'users' }
format.js { render(:update) {|page| page.replace_html "tab-content-users", :partial => 'groups/users'} }
format.api { head :ok }
format.api { render_api_ok }
end
end

Expand Down
4 changes: 2 additions & 2 deletions app/controllers/issue_categories_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ def update
flash[:notice] = l(:notice_successful_update)
redirect_to :controller => 'projects', :action => 'settings', :tab => 'categories', :id => @project
}
format.api { head :ok }
format.api { render_api_ok }
end
else
respond_to do |format|
Expand All @@ -118,7 +118,7 @@ def destroy
@category.destroy(reassign_to)
respond_to do |format|
format.html { redirect_to :controller => 'projects', :action => 'settings', :id => @project, :tab => 'categories' }
format.api { head :ok }
format.api { render_api_ok }
end
return
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/issue_relations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ def destroy
respond_to do |format|
format.html { redirect_to issue_path } # TODO : does this really work since @issue is always nil? What is it useful to?
format.js { render(:update) {|page| page.remove "relation-#{@relation.id}"} }
format.api { head :ok }
format.api { render_api_ok }
end
end

Expand Down
4 changes: 2 additions & 2 deletions app/controllers/issues_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ def update

respond_to do |format|
format.html { redirect_back_or_default({:action => 'show', :id => @issue}) }
format.api { head :ok }
format.api { render_api_ok }
end
else
respond_to do |format|
Expand Down Expand Up @@ -307,7 +307,7 @@ def destroy
end
respond_to do |format|
format.html { redirect_back_or_default(:action => 'index', :project_id => @project) }
format.api { head :ok }
format.api { render_api_ok }
end
end

Expand Down
4 changes: 2 additions & 2 deletions app/controllers/members_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ def update
}
format.api {
if saved
head :ok
render_api_ok
else
render_validation_errors(@member)
end
Expand All @@ -128,7 +128,7 @@ def destroy
}
format.api {
if @member.destroyed?
head :ok
render_api_ok
else
head :unprocessable_entity
end
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/projects_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ def update
flash[:notice] = l(:notice_successful_update)
redirect_to :action => 'settings', :id => @project
}
format.api { head :ok }
format.api { render_api_ok }
end
else
respond_to do |format|
Expand Down Expand Up @@ -241,7 +241,7 @@ def destroy
@project_to_destroy.destroy
respond_to do |format|
format.html { redirect_to :controller => 'admin', :action => 'projects' }
format.api { head :ok }
format.api { render_api_ok }
end
end
# hide project in layout
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/timelog_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ def update
flash[:notice] = l(:notice_successful_update)
redirect_back_or_default :action => 'index', :project_id => @time_entry.project
}
format.api { head :ok }
format.api { render_api_ok }
end
else
respond_to do |format|
Expand Down Expand Up @@ -223,7 +223,7 @@ def destroy
}
format.api {
if destroyed
head :ok
render_api_ok
else
render_validation_errors(@time_entries)
end
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ def update
flash[:notice] = l(:notice_successful_update)
redirect_to_referer_or edit_user_path(@user)
}
format.api { head :ok }
format.api { render_api_ok }
end
else
@auth_sources = AuthSource.find(:all)
Expand All @@ -175,7 +175,7 @@ def destroy
@user.destroy
respond_to do |format|
format.html { redirect_to_referer_or(users_url) }
format.api { head :ok }
format.api { render_api_ok }
end
end

Expand Down
4 changes: 2 additions & 2 deletions app/controllers/versions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ def update
flash[:notice] = l(:notice_successful_update)
redirect_back_or_default :controller => 'projects', :action => 'settings', :tab => 'versions', :id => @project
}
format.api { head :ok }
format.api { render_api_ok }
end
else
respond_to do |format|
Expand All @@ -167,7 +167,7 @@ def destroy
@version.destroy
respond_to do |format|
format.html { redirect_back_or_default :controller => 'projects', :action => 'settings', :tab => 'versions', :id => @project }
format.api { head :ok }
format.api { render_api_ok }
end
else
respond_to do |format|
Expand Down
4 changes: 4 additions & 0 deletions test/integration/api_test/groups_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ def setup
should "update the group" do
put '/groups/10.xml', {:group => {:name => 'New name', :user_ids => [2, 3]}}, credentials('admin')
assert_response :ok
assert_equal '', @response.body

group = Group.find(10)
assert_equal 'New name', group.name
Expand Down Expand Up @@ -177,6 +178,7 @@ def setup
assert_difference 'Group.count', -1 do
delete '/groups/10.xml', {}, credentials('admin')
assert_response :ok
assert_equal '', @response.body
end
end
end
Expand All @@ -188,6 +190,7 @@ def setup
assert_difference 'Group.find(10).users.count' do
post '/groups/10/users.xml', {:user_id => 5}, credentials('admin')
assert_response :ok
assert_equal '', @response.body
end
assert_include User.find(5), Group.find(10).users
end
Expand All @@ -200,6 +203,7 @@ def setup
assert_difference 'Group.find(10).users.count', -1 do
delete '/groups/10/users/8.xml', {}, credentials('admin')
assert_response :ok
assert_equal '', @response.body
end
assert_not_include User.find(8), Group.find(10).users
end
Expand Down
3 changes: 3 additions & 0 deletions test/integration/api_test/issue_categories_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ def setup
put '/issue_categories/2.xml', {:issue_category => {:name => 'API Update'}}, credentials('jsmith')
end
assert_response :ok
assert_equal '', @response.body
assert_equal 'API Update', IssueCategory.find(2).name
end
end
Expand All @@ -104,6 +105,7 @@ def setup
delete '/issue_categories/1.xml', {}, credentials('jsmith')
end
assert_response :ok
assert_equal '', @response.body
assert_nil IssueCategory.find_by_id(1)
end

Expand All @@ -117,6 +119,7 @@ def setup
end
end
assert_response :ok
assert_equal '', @response.body
assert_nil IssueCategory.find_by_id(1)
end
end
Expand Down
1 change: 1 addition & 0 deletions test/integration/api_test/issue_relations_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ def setup
end

assert_response :ok
assert_equal '', @response.body
assert_nil IssueRelation.find_by_id(2)
end
end
Expand Down
1 change: 1 addition & 0 deletions test/integration/api_test/issues_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,7 @@ def test_update_issue_with_uploaded_file
{:issue => {:notes => 'Attachment added', :uploads => [{:token => token, :filename => 'test.txt', :content_type => 'text/plain'}]}},
credentials('jsmith')
assert_response :ok
assert_equal '', @response.body
end

issue = Issue.find(1)
Expand Down
2 changes: 2 additions & 0 deletions test/integration/api_test/memberships_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ def setup
put '/memberships/2.xml', {:membership => {:user_id => 3, :role_ids => [1,2]}}, credentials('jsmith')

assert_response :ok
assert_equal '', @response.body
end
member = Member.find(2)
assert_equal [1,2], member.role_ids.sort
Expand All @@ -179,6 +180,7 @@ def setup
delete '/memberships/2.xml', {}, credentials('jsmith')

assert_response :ok
assert_equal '', @response.body
end
assert_nil Member.find_by_id(2)
end
Expand Down
4 changes: 4 additions & 0 deletions test/integration/api_test/projects_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ def setup
put '/projects/2.xml', @parameters, credentials('jsmith')
end
assert_response :ok
assert_equal '', @response.body
assert_equal 'application/xml', @response.content_type
project = Project.find(2)
assert_equal 'API update', project.name
Expand All @@ -238,6 +239,7 @@ def setup
put '/projects/2.xml', @parameters, credentials('admin')
end
assert_response :ok
assert_equal '', @response.body
project = Project.find(2)
assert_equal ['issue_tracking', 'news', 'time_tracking'], project.enabled_module_names.sort
end
Expand All @@ -249,6 +251,7 @@ def setup
put '/projects/2.xml', @parameters, credentials('admin')
end
assert_response :ok
assert_equal '', @response.body
project = Project.find(2)
assert_equal [1, 3], project.trackers.map(&:id).sort
end
Expand Down Expand Up @@ -286,6 +289,7 @@ def setup
delete '/projects/2.xml', {}, credentials('admin')
end
assert_response :ok
assert_equal '', @response.body
assert_nil Project.find_by_id(2)
end
end
Expand Down
2 changes: 2 additions & 0 deletions test/integration/api_test/time_entries_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ def setup
put '/time_entries/2.xml', {:time_entry => {:comments => 'API Update'}}, credentials('jsmith')
end
assert_response :ok
assert_equal '', @response.body
assert_equal 'API Update', TimeEntry.find(2).comments
end
end
Expand All @@ -157,6 +158,7 @@ def setup
delete '/time_entries/2.xml', {}, credentials('jsmith')
end
assert_response :ok
assert_equal '', @response.body
assert_nil TimeEntry.find_by_id(2)
end
end
Expand Down
4 changes: 4 additions & 0 deletions test/integration/api_test/users_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ def setup
assert !user.admin?

assert_response :ok
assert_equal '', @response.body
end
end

Expand All @@ -263,6 +264,7 @@ def setup
assert !user.admin?

assert_response :ok
assert_equal '', @response.body
end
end
end
Expand Down Expand Up @@ -322,6 +324,7 @@ def setup
end

assert_response :ok
assert_equal '', @response.body
end
end

Expand All @@ -337,6 +340,7 @@ def setup
end

assert_response :ok
assert_equal '', @response.body
end
end
end
Expand Down
2 changes: 2 additions & 0 deletions test/integration/api_test/versions_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ def setup
put '/versions/2.xml', {:version => {:name => 'API update'}}, credentials('jsmith')

assert_response :ok
assert_equal '', @response.body
assert_equal 'API update', Version.find(2).name
end
end
Expand All @@ -131,6 +132,7 @@ def setup
end

assert_response :ok
assert_equal '', @response.body
assert_nil Version.find_by_id(3)
end
end
Expand Down

0 comments on commit 0fab00f

Please sign in to comment.