From 735a83c596156e32f8cc686207eecddc029d7629 Mon Sep 17 00:00:00 2001
From: Jean-Philippe Lang <jp_lang@yahoo.fr>
Date: Fri, 3 Dec 2010 13:52:07 +0000
Subject: [PATCH] Converts IssuesController to use the new API template system
and makes xml/json responses consistent (#6136).
git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@4458 e93f8b46-1217-0410-a6f0-8f06a7374b81
---
app/controllers/issues_controller.rb | 27 +++-----
app/views/issues/index.apit | 32 ++++++++++
app/views/issues/index.xml.builder | 33 ----------
app/views/issues/show.apit | 61 ++++++++++++++++++
app/views/issues/show.xml.builder | 62 -------------------
lib/redmine/views/builders/structure.rb | 2 +
test/integration/api_test/issues_test.rb | 6 +-
.../lib/redmine/views/builders/json_test.rb | 9 +++
8 files changed, 116 insertions(+), 116 deletions(-)
create mode 100644 app/views/issues/index.apit
delete mode 100644 app/views/issues/index.xml.builder
create mode 100644 app/views/issues/show.apit
delete mode 100644 app/views/issues/show.xml.builder
diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
index 02d97643a..79b831ca6 100644
--- a/app/controllers/issues_controller.rb
+++ b/app/controllers/issues_controller.rb
@@ -84,8 +84,7 @@ class IssuesController < ApplicationController
respond_to do |format|
format.html { render :template => 'issues/index.rhtml', :layout => !request.xhr? }
- format.xml { render :layout => false }
- format.json { render :text => @issues.to_json, :layout => false }
+ format.api { render :template => 'issues/index.apit' }
format.atom { render_feed(@issues, :title => "#{@project || Setting.app_title}: #{l(:label_issue_plural)}") }
format.csv { send_data(issues_to_csv(@issues, @project), :type => 'text/csv; header=present', :filename => 'export.csv') }
format.pdf { send_data(issues_to_pdf(@issues, @project, @query), :type => 'application/pdf', :filename => 'export.pdf') }
@@ -110,8 +109,7 @@ class IssuesController < ApplicationController
@time_entry = TimeEntry.new
respond_to do |format|
format.html { render :template => 'issues/show.rhtml' }
- format.xml { render :layout => false }
- format.json { render :text => @issue.to_json, :layout => false }
+ format.api { render :template => 'issues/show.apit' }
format.atom { render :template => 'journals/index', :layout => false, :content_type => 'application/atom+xml' }
format.pdf { send_data(issue_to_pdf(@issue), :type => 'application/pdf', :filename => "#{@project.identifier}-#{@issue.id}.pdf") }
end
@@ -138,15 +136,13 @@ class IssuesController < ApplicationController
redirect_to(params[:continue] ? { :action => 'new', :project_id => @project, :issue => {:tracker_id => @issue.tracker, :parent_issue_id => @issue.parent_issue_id}.reject {|k,v| v.nil?} } :
{ :action => 'show', :id => @issue })
}
- format.xml { render :action => 'show', :status => :created, :location => url_for(:controller => 'issues', :action => 'show', :id => @issue) }
- format.json { render :text => @issue.to_json, :status => :created, :location => url_for(:controller => 'issues', :action => 'show'), :layout => false }
+ format.api { render :template => 'issues/show.apit', :status => :created, :location => issue_url(@issue) }
end
return
else
respond_to do |format|
format.html { render :action => 'new' }
- format.xml { render(:xml => @issue.errors, :status => :unprocessable_entity); return }
- format.json { render :text => object_errors_to_json(@issue), :status => :unprocessable_entity, :layout => false }
+ format.api { render_validation_errors(@issue) }
end
end
end
@@ -171,8 +167,7 @@ class IssuesController < ApplicationController
respond_to do |format|
format.html { redirect_back_or_default({:action => 'show', :id => @issue}) }
- format.xml { head :ok }
- format.json { head :ok }
+ format.api { head :ok }
end
else
render_attachment_warning_if_needed(@issue)
@@ -181,8 +176,7 @@ class IssuesController < ApplicationController
respond_to do |format|
format.html { render :action => 'edit' }
- format.xml { render :xml => @issue.errors, :status => :unprocessable_entity }
- format.json { render :text => object_errors_to_json(@issue), :status => :unprocessable_entity, :layout => false }
+ format.api { render_validation_errors(@issue) }
end
end
end
@@ -232,17 +226,14 @@ class IssuesController < ApplicationController
TimeEntry.update_all("issue_id = #{reassign_to.id}", ['issue_id IN (?)', @issues])
end
else
- unless params[:format] == 'xml' || params[:format] == 'json'
- # display the destroy form if it's a user request
- return
- end
+ # display the destroy form if it's a user request
+ return unless api_request?
end
end
@issues.each(&:destroy)
respond_to do |format|
format.html { redirect_back_or_default(:action => 'index', :project_id => @project) }
- format.xml { head :ok }
- format.json { head :ok }
+ format.api { head :ok }
end
end
diff --git a/app/views/issues/index.apit b/app/views/issues/index.apit
new file mode 100644
index 000000000..a63b07a09
--- /dev/null
+++ b/app/views/issues/index.apit
@@ -0,0 +1,32 @@
+api.array :issues do
+ @issues.each do |issue|
+ api.issue do
+ api.id issue.id
+ api.project(:id => issue.project_id, :name => issue.project.name) unless issue.project.nil?
+ api.tracker(:id => issue.tracker_id, :name => issue.tracker.name) unless issue.tracker.nil?
+ api.status(:id => issue.status_id, :name => issue.status.name) unless issue.status.nil?
+ api.priority(:id => issue.priority_id, :name => issue.priority.name) unless issue.priority.nil?
+ api.author(:id => issue.author_id, :name => issue.author.name) unless issue.author.nil?
+ api.assigned_to(:id => issue.assigned_to_id, :name => issue.assigned_to.name) unless issue.assigned_to.nil?
+ api.category(:id => issue.category_id, :name => issue.category.name) unless issue.category.nil?
+ api.fixed_version(:id => issue.fixed_version_id, :name => issue.fixed_version.name) unless issue.fixed_version.nil?
+ api.parent(:id => issue.parent_id) unless issue.parent.nil?
+
+ api.subject issue.subject
+ api.description issue.description
+ api.start_date issue.start_date
+ api.due_date issue.due_date
+ api.done_ratio issue.done_ratio
+ api.estimated_hours issue.estimated_hours
+
+ api.array :custom_fields do
+ issue.custom_field_values.each do |custom_value|
+ api.custom_field custom_value.value, :id => custom_value.custom_field_id, :name => custom_value.custom_field.name
+ end
+ end
+
+ api.created_on issue.created_on
+ api.updated_on issue.updated_on
+ end
+ end
+end
diff --git a/app/views/issues/index.xml.builder b/app/views/issues/index.xml.builder
deleted file mode 100644
index ba5697c9b..000000000
--- a/app/views/issues/index.xml.builder
+++ /dev/null
@@ -1,33 +0,0 @@
-xml.instruct!
-xml.issues :type => 'array' do
- @issues.each do |issue|
- xml.issue do
- xml.id issue.id
- xml.project(:id => issue.project_id, :name => issue.project.name) unless issue.project.nil?
- xml.tracker(:id => issue.tracker_id, :name => issue.tracker.name) unless issue.tracker.nil?
- xml.status(:id => issue.status_id, :name => issue.status.name) unless issue.status.nil?
- xml.priority(:id => issue.priority_id, :name => issue.priority.name) unless issue.priority.nil?
- xml.author(:id => issue.author_id, :name => issue.author.name) unless issue.author.nil?
- xml.assigned_to(:id => issue.assigned_to_id, :name => issue.assigned_to.name) unless issue.assigned_to.nil?
- xml.category(:id => issue.category_id, :name => issue.category.name) unless issue.category.nil?
- xml.fixed_version(:id => issue.fixed_version_id, :name => issue.fixed_version.name) unless issue.fixed_version.nil?
- xml.parent(:id => issue.parent_id) unless issue.parent.nil?
-
- xml.subject issue.subject
- xml.description issue.description
- xml.start_date issue.start_date
- xml.due_date issue.due_date
- xml.done_ratio issue.done_ratio
- xml.estimated_hours issue.estimated_hours
-
- xml.custom_fields do
- issue.custom_field_values.each do |custom_value|
- xml.custom_field custom_value.value, :id => custom_value.custom_field_id, :name => custom_value.custom_field.name
- end
- end
-
- xml.created_on issue.created_on
- xml.updated_on issue.updated_on
- end
- end
-end
diff --git a/app/views/issues/show.apit b/app/views/issues/show.apit
new file mode 100644
index 000000000..224026d1f
--- /dev/null
+++ b/app/views/issues/show.apit
@@ -0,0 +1,61 @@
+api.issue do
+ api.id @issue.id
+ api.project(:id => @issue.project_id, :name => @issue.project.name) unless @issue.project.nil?
+ api.tracker(:id => @issue.tracker_id, :name => @issue.tracker.name) unless @issue.tracker.nil?
+ api.status(:id => @issue.status_id, :name => @issue.status.name) unless @issue.status.nil?
+ api.priority(:id => @issue.priority_id, :name => @issue.priority.name) unless @issue.priority.nil?
+ api.author(:id => @issue.author_id, :name => @issue.author.name) unless @issue.author.nil?
+ api.assigned_to(:id => @issue.assigned_to_id, :name => @issue.assigned_to.name) unless @issue.assigned_to.nil?
+ api.category(:id => @issue.category_id, :name => @issue.category.name) unless @issue.category.nil?
+ api.fixed_version(:id => @issue.fixed_version_id, :name => @issue.fixed_version.name) unless @issue.fixed_version.nil?
+ api.parent(:id => @issue.parent_id) unless @issue.parent.nil?
+
+ api.subject @issue.subject
+ api.description @issue.description
+ api.start_date @issue.start_date
+ api.due_date @issue.due_date
+ api.done_ratio @issue.done_ratio
+ api.estimated_hours @issue.estimated_hours
+ if User.current.allowed_to?(:view_time_entries, @project)
+ api.spent_hours @issue.spent_hours
+ end
+
+ api.array :custom_fields do
+ @issue.custom_field_values.each do |custom_value|
+ api.custom_field custom_value.value, :id => custom_value.custom_field_id, :name => custom_value.custom_field.name
+ end
+ end unless @issue.custom_field_values.empty?
+
+ api.created_on @issue.created_on
+ api.updated_on @issue.updated_on
+
+ api.array :relations do
+ @issue.relations.select {|r| r.other_issue(@issue).visible? }.each do |relation|
+ api.relation(:id => relation.id, :issue_id => relation.other_issue(@issue).id, :relation_type => relation.relation_type_for(@issue), :delay => relation.delay)
+ end
+ end
+
+ api.array :changesets do
+ @issue.changesets.each do |changeset|
+ api.changeset :revision => changeset.revision do
+ api.user(:id => changeset.user_id, :name => changeset.user.name) unless changeset.user.nil?
+ api.comments changeset.comments
+ api.committed_on changeset.committed_on
+ end
+ end
+ end if User.current.allowed_to?(:view_changesets, @project) && @issue.changesets.any?
+
+ api.array :journals do
+ @issue.journals.each do |journal|
+ api.journal :id => journal.id do
+ api.user(:id => journal.user_id, :name => journal.user.name) unless journal.user.nil?
+ api.notes journal.notes
+ api.array :details do
+ journal.details.each do |detail|
+ api.detail :property => detail.property, :name => detail.prop_key, :old => detail.old_value, :new => detail.value
+ end
+ end
+ end
+ end
+ end unless @issue.journals.empty?
+end
diff --git a/app/views/issues/show.xml.builder b/app/views/issues/show.xml.builder
deleted file mode 100644
index bd3448cc8..000000000
--- a/app/views/issues/show.xml.builder
+++ /dev/null
@@ -1,62 +0,0 @@
-xml.instruct!
-xml.issue do
- xml.id @issue.id
- xml.project(:id => @issue.project_id, :name => @issue.project.name) unless @issue.project.nil?
- xml.tracker(:id => @issue.tracker_id, :name => @issue.tracker.name) unless @issue.tracker.nil?
- xml.status(:id => @issue.status_id, :name => @issue.status.name) unless @issue.status.nil?
- xml.priority(:id => @issue.priority_id, :name => @issue.priority.name) unless @issue.priority.nil?
- xml.author(:id => @issue.author_id, :name => @issue.author.name) unless @issue.author.nil?
- xml.assigned_to(:id => @issue.assigned_to_id, :name => @issue.assigned_to.name) unless @issue.assigned_to.nil?
- xml.category(:id => @issue.category_id, :name => @issue.category.name) unless @issue.category.nil?
- xml.fixed_version(:id => @issue.fixed_version_id, :name => @issue.fixed_version.name) unless @issue.fixed_version.nil?
- xml.parent(:id => @issue.parent_id) unless @issue.parent.nil?
-
- xml.subject @issue.subject
- xml.description @issue.description
- xml.start_date @issue.start_date
- xml.due_date @issue.due_date
- xml.done_ratio @issue.done_ratio
- xml.estimated_hours @issue.estimated_hours
- if User.current.allowed_to?(:view_time_entries, @project)
- xml.spent_hours @issue.spent_hours
- end
-
- xml.custom_fields do
- @issue.custom_field_values.each do |custom_value|
- xml.custom_field custom_value.value, :id => custom_value.custom_field_id, :name => custom_value.custom_field.name
- end
- end unless @issue.custom_field_values.empty?
-
- xml.created_on @issue.created_on
- xml.updated_on @issue.updated_on
-
- xml.relations do
- @issue.relations.select {|r| r.other_issue(@issue).visible? }.each do |relation|
- xml.relation(:id => relation.id, :issue_id => relation.other_issue(@issue).id, :relation_type => relation.relation_type_for(@issue), :delay => relation.delay)
- end
- end
-
- xml.changesets do
- @issue.changesets.each do |changeset|
- xml.changeset :revision => changeset.revision do
- xml.user(:id => changeset.user_id, :name => changeset.user.name) unless changeset.user.nil?
- xml.comments changeset.comments
- xml.committed_on changeset.committed_on
- end
- end
- end if User.current.allowed_to?(:view_changesets, @project) && @issue.changesets.any?
-
- xml.journals do
- @issue.journals.each do |journal|
- xml.journal :id => journal.id do
- xml.user(:id => journal.user_id, :name => journal.user.name) unless journal.user.nil?
- xml.notes journal.notes
- xml.details do
- journal.details.each do |detail|
- xml.detail :property => detail.property, :name => detail.prop_key, :old => detail.old_value, :new => detail.value
- end
- end
- end
- end
- end unless @issue.journals.empty?
-end
diff --git a/lib/redmine/views/builders/structure.rb b/lib/redmine/views/builders/structure.rb
index b1add8be8..c168bd73a 100644
--- a/lib/redmine/views/builders/structure.rb
+++ b/lib/redmine/views/builders/structure.rb
@@ -37,6 +37,8 @@ module Redmine
if args.first.is_a?(Hash)
if @struct.last.is_a?(Array)
@struct.last << args.first
+ else
+ @struct.last[sym] = args.first
end
else
if @struct.last.is_a?(Array)
diff --git a/test/integration/api_test/issues_test.rb b/test/integration/api_test/issues_test.rb
index 1c1f3f8b7..d7bc785c0 100644
--- a/test/integration/api_test/issues_test.rb
+++ b/test/integration/api_test/issues_test.rb
@@ -74,7 +74,7 @@ class ApiTest::IssuesTest < ActionController::IntegrationTest
get '/issues.json?status_id=5'
json = ActiveSupport::JSON.decode(response.body)
- status_ids_used = json.collect {|j| j['status_id'] }
+ status_ids_used = json['issues'].collect {|j| j['status']['id'] }
assert_equal 3, status_ids_used.length
assert status_ids_used.all? {|id| id == 5 }
end
@@ -160,7 +160,7 @@ class ApiTest::IssuesTest < ActionController::IntegrationTest
end
json = ActiveSupport::JSON.decode(response.body)
- assert_equal "can't be blank", json.first['subject']
+ assert json['errors'].include?(['subject', "can't be blank"])
end
end
@@ -300,7 +300,7 @@ class ApiTest::IssuesTest < ActionController::IntegrationTest
put '/issues/6.json', @parameters, @headers
json = ActiveSupport::JSON.decode(response.body)
- assert_equal "can't be blank", json.first['subject']
+ assert json['errors'].include?(['subject', "can't be blank"])
end
end
diff --git a/test/unit/lib/redmine/views/builders/json_test.rb b/test/unit/lib/redmine/views/builders/json_test.rb
index 52e6b9ca6..195fba020 100644
--- a/test/unit/lib/redmine/views/builders/json_test.rb
+++ b/test/unit/lib/redmine/views/builders/json_test.rb
@@ -28,6 +28,15 @@ class Redmine::Views::Builders::JsonTest < HelperTestCase
end
end
+ def test_hash_hash
+ assert_json_output({'person' => {'name' => 'Ryan', 'birth' => {'city' => 'London', 'country' => 'UK'}}}) do |b|
+ b.person do
+ b.name 'Ryan'
+ b.birth :city => 'London', :country => 'UK'
+ end
+ end
+ end
+
def test_array
assert_json_output({'books' => [{'title' => 'Book 1', 'author' => 'B. Smith'}, {'title' => 'Book 2', 'author' => 'G. Cooper'}]}) do |b|
b.array :books do |b|
--
GitLab