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

8 files changed, 116 insertions, 116 deletions

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|