From 6bb1ea8ae8a1f7495ceb1ca656a056dbf11d3dac Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Thu, 21 Jan 2016 04:39:56 +0000 Subject: [PATCH] Use regular edit/update actions and named routes for JournalsController. git-svn-id: http://svn.redmine.org/redmine/trunk@15074 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/journals_controller.rb | 31 +++++++++++---------- app/helpers/issues_helper.rb | 3 +- app/helpers/journals_helper.rb | 8 +++--- app/views/journals/_notes_form.html.erb | 3 +- config/routes.rb | 7 +++-- lib/redmine.rb | 4 +-- test/functional/journals_controller_test.rb | 4 +-- test/integration/routing/journals_test.rb | 6 ++-- test/unit/mailer_test.rb | 6 ++-- 9 files changed, 38 insertions(+), 34 deletions(-) diff --git a/app/controllers/journals_controller.rb b/app/controllers/journals_controller.rb index bae6ca2bc..382a84eb0 100644 --- a/app/controllers/journals_controller.rb +++ b/app/controllers/journals_controller.rb @@ -16,10 +16,10 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. class JournalsController < ApplicationController - before_filter :find_journal, :only => [:edit, :diff] + before_filter :find_journal, :only => [:edit, :update, :diff] before_filter :find_issue, :only => [:new] before_filter :find_optional_project, :only => [:index] - before_filter :authorize, :only => [:new, :edit, :diff] + before_filter :authorize, :only => [:new, :edit, :update, :diff] accept_rss_auth :index menu_item :issues @@ -82,19 +82,20 @@ class JournalsController < ApplicationController def edit (render_403; return false) unless @journal.editable_by?(User.current) - if request.post? - @journal.update_attributes(:notes => params[:notes]) if params[:notes] - @journal.destroy if @journal.details.empty? && @journal.notes.blank? - call_hook(:controller_journals_edit_post, { :journal => @journal, :params => params}) - respond_to do |format| - format.html { redirect_to issue_path(@journal.journalized) } - format.js { render :action => 'update' } - end - else - respond_to do |format| - # TODO: implement non-JS journal update - format.js - end + respond_to do |format| + # TODO: implement non-JS journal update + format.js + end + end + + def update + (render_403; return false) unless @journal.editable_by?(User.current) + @journal.update_attributes(:notes => params[:notes]) if params[:notes] + @journal.destroy if @journal.details.empty? && @journal.notes.blank? + call_hook(:controller_journals_edit_post, { :journal => @journal, :params => params}) + respond_to do |format| + format.html { redirect_to issue_path(@journal.journalized) } + format.js end end diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 4353e5033..5fdc6bf8c 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -456,8 +456,7 @@ module IssuesHelper s = l(:text_journal_changed_no_detail, :label => label) unless no_html diff_link = link_to 'diff', - {:controller => 'journals', :action => 'diff', :id => detail.journal_id, - :detail_id => detail.id, :only_path => options[:only_path]}, + diff_journal_url(detail.journal_id, :detail_id => detail.id, :only_path => options[:only_path]), :title => l(:label_view_diff) s << " (#{ diff_link })" end diff --git a/app/helpers/journals_helper.rb b/app/helpers/journals_helper.rb index 0b57e1acb..2a0a7d351 100644 --- a/app/helpers/journals_helper.rb +++ b/app/helpers/journals_helper.rb @@ -31,23 +31,23 @@ module JournalsHelper links = [] if !journal.notes.blank? links << link_to('', - {:controller => 'journals', :action => 'new', :id => issue, :journal_id => journal}, + quoted_issue_path(issue, :journal_id => journal), :remote => true, :method => 'post', :title => l(:button_quote), :class => 'icon-only icon-comment' ) if options[:reply_links] links << link_to('', - {:controller => 'journals', :action => 'edit', :id => journal}, + edit_journal_path(journal), :remote => true, :method => 'get', :title => l(:button_edit), :class => 'icon-only icon-edit' ) if editable links << link_to('', - {:controller => 'journals', :action => 'edit', :id => journal, :notes => ""}, + journal_path(journal, :notes => ""), :remote => true, - :method => :post, :data => {:confirm => l(:text_are_you_sure)}, + :method => 'put', :data => {:confirm => l(:text_are_you_sure)}, :title => l(:button_delete), :class => 'icon-only icon-del' ) if editable diff --git a/app/views/journals/_notes_form.html.erb b/app/views/journals/_notes_form.html.erb index 63b87883a..41650c5d3 100644 --- a/app/views/journals/_notes_form.html.erb +++ b/app/views/journals/_notes_form.html.erb @@ -1,5 +1,6 @@ -<%= form_tag({:controller => 'journals', :action => 'edit', :id => @journal}, +<%= form_tag(journal_path(@journal), :remote => true, + :method => 'put', :id => "journal-#{@journal.id}-form") do %> <%= label_tag "notes", l(:description_notes), :class => "hidden-for-sighted" %> <%= text_area_tag :notes, @journal.notes, diff --git a/config/routes.rb b/config/routes.rb index 47451453f..3b469d7e9 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -49,8 +49,11 @@ Rails.application.routes.draw do match '/issues/changes', :to => 'journals#index', :as => 'issue_changes', :via => :get match '/issues/:id/quoted', :to => 'journals#new', :id => /\d+/, :via => :post, :as => 'quoted_issue' - match '/journals/diff/:id', :to => 'journals#diff', :id => /\d+/, :via => :get - match '/journals/edit/:id', :to => 'journals#edit', :id => /\d+/, :via => [:get, :post] + resources :journals, :only => [:edit, :update] do + member do + get 'diff' + end + end get '/projects/:project_id/issues/gantt', :to => 'gantts#show', :as => 'project_gantt' get '/issues/gantt', :to => 'gantts#show' diff --git a/lib/redmine.rb b/lib/redmine.rb index a7e29c891..d797a017c 100644 --- a/lib/redmine.rb +++ b/lib/redmine.rb @@ -105,8 +105,8 @@ Redmine::AccessControl.map do |map| map.permission :set_issues_private, {} map.permission :set_own_issues_private, {}, :require => :loggedin map.permission :add_issue_notes, {:issues => [:edit, :update], :journals => [:new], :attachments => :upload} - map.permission :edit_issue_notes, {:journals => :edit}, :require => :loggedin - map.permission :edit_own_issue_notes, {:journals => :edit}, :require => :loggedin + map.permission :edit_issue_notes, {:journals => [:edit, :update]}, :require => :loggedin + map.permission :edit_own_issue_notes, {:journals => [:edit, :update]}, :require => :loggedin map.permission :view_private_notes, {}, :read => true, :require => :member map.permission :set_notes_private, {}, :require => :member map.permission :delete_issues, {:issues => :destroy}, :require => :member diff --git a/test/functional/journals_controller_test.rb b/test/functional/journals_controller_test.rb index c82d2e588..cc1446c7a 100644 --- a/test/functional/journals_controller_test.rb +++ b/test/functional/journals_controller_test.rb @@ -199,7 +199,7 @@ class JournalsControllerTest < ActionController::TestCase def test_update_xhr @request.session[:user_id] = 1 - xhr :post, :edit, :id => 2, :notes => 'Updated notes' + xhr :post, :update, :id => 2, :notes => 'Updated notes' assert_response :success assert_template 'update' assert_equal 'text/javascript', response.content_type @@ -210,7 +210,7 @@ class JournalsControllerTest < ActionController::TestCase def test_update_xhr_with_empty_notes_should_delete_the_journal @request.session[:user_id] = 1 assert_difference 'Journal.count', -1 do - xhr :post, :edit, :id => 2, :notes => '' + xhr :post, :update, :id => 2, :notes => '' assert_response :success assert_template 'update' assert_equal 'text/javascript', response.content_type diff --git a/test/integration/routing/journals_test.rb b/test/integration/routing/journals_test.rb index c0bcf0ea3..33e161e35 100644 --- a/test/integration/routing/journals_test.rb +++ b/test/integration/routing/journals_test.rb @@ -21,9 +21,9 @@ class RoutingJournalsTest < Redmine::RoutingTest def test_journals should_route 'POST /issues/1/quoted' => 'journals#new', :id => '1' should_route 'GET /issues/changes' => 'journals#index' - should_route 'GET /journals/diff/1' => 'journals#diff', :id => '1' + should_route 'GET /journals/1/diff' => 'journals#diff', :id => '1' - should_route 'GET /journals/edit/1' => 'journals#edit', :id => '1' - should_route 'POST /journals/edit/1' => 'journals#edit', :id => '1' + should_route 'GET /journals/1/edit' => 'journals#edit', :id => '1' + should_route 'PUT /journals/1' => 'journals#update', :id => '1' end end diff --git a/test/unit/mailer_test.rb b/test/unit/mailer_test.rb index f3742c2b5..8de5bfe56 100644 --- a/test/unit/mailer_test.rb +++ b/test/unit/mailer_test.rb @@ -68,7 +68,7 @@ class MailerTest < ActiveSupport::TestCase # should be https://mydomain.foo/journals/diff/3?detail_id=4 # but the Rails 4.2 DOM assertion doesn't handle the ? in the # attribute value - 'https://mydomain.foo/journals/diff/3', + 'https://mydomain.foo/journals/3/diff', 'View differences', :text => 'diff' # link to an attachment @@ -109,7 +109,7 @@ class MailerTest < ActiveSupport::TestCase # should be http://mydomain.foo/rdm/journals/diff/3?detail_id=4 # but the Rails 4.2 DOM assertion doesn't handle the ? in the # attribute value - 'http://mydomain.foo/rdm/journals/diff/3', + 'http://mydomain.foo/rdm/journals/3/diff', 'View differences', :text => 'diff' # link to an attachment @@ -179,7 +179,7 @@ class MailerTest < ActiveSupport::TestCase # should be http://mydomain.foo/rdm/journals/diff/3?detail_id=4 # but the Rails 4.2 DOM assertion doesn't handle the ? in the # attribute value - 'http://mydomain.foo/rdm/journals/diff/3', + 'http://mydomain.foo/rdm/journals/3/diff', 'View differences', :text => 'diff' # link to an attachment -- 2.39.5