From: Jean-Philippe Lang Date: Fri, 3 Dec 2010 13:52:07 +0000 (+0000) Subject: Converts IssuesController to use the new API template system and makes xml/json respo... X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=735a83c596156e32f8cc686207eecddc029d7629;p=redminele%2Fredmine.git 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 --- diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index 02d97643..79b831ca 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 00000000..a63b07a0 --- /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 ba5697c9..00000000 --- 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 00000000..224026d1 --- /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 bd3448cc..00000000 --- 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 b1add8be..c168bd73 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 1c1f3f8b..d7bc785c 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 52e6b9ca..195fba02 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|