From b925325ddbd5fb594f20221dd724f7822ed4c3d3 Mon Sep 17 00:00:00 2001 From: Eric Davis Date: Fri, 27 Aug 2010 14:05:54 +0000 Subject: [PATCH] Refactor: extract ProjectsController#activity to a new Activities controller. git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@4047 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/activities_controller.rb | 59 +++++++++++++ app/controllers/projects_controller.rb | 50 +---------- .../index.html.erb} | 0 app/views/boards/index.rhtml | 4 +- app/views/projects/index.rhtml | 2 +- app/views/projects/show.rhtml | 2 +- app/views/users/show.rhtml | 6 +- app/views/welcome/index.rhtml | 2 +- app/views/wiki/special_date_index.rhtml | 4 +- config/routes.rb | 2 +- lib/redmine.rb | 4 +- test/functional/activities_controller_test.rb | 87 +++++++++++++++++++ test/functional/projects_controller_test.rb | 81 ----------------- test/integration/routing_test.rb | 8 +- 14 files changed, 166 insertions(+), 145 deletions(-) create mode 100644 app/controllers/activities_controller.rb rename app/views/{projects/activity.rhtml => activities/index.html.erb} (100%) create mode 100644 test/functional/activities_controller_test.rb diff --git a/app/controllers/activities_controller.rb b/app/controllers/activities_controller.rb new file mode 100644 index 000000000..ae6a67369 --- /dev/null +++ b/app/controllers/activities_controller.rb @@ -0,0 +1,59 @@ +class ActivitiesController < ApplicationController + menu_item :activity + before_filter :find_optional_project + accept_key_auth :index + + def index + @days = Setting.activity_days_default.to_i + + if params[:from] + begin; @date_to = params[:from].to_date + 1; rescue; end + end + + @date_to ||= Date.today + 1 + @date_from = @date_to - @days + @with_subprojects = params[:with_subprojects].nil? ? Setting.display_subprojects_issues? : (params[:with_subprojects] == '1') + @author = (params[:user_id].blank? ? nil : User.active.find(params[:user_id])) + + @activity = Redmine::Activity::Fetcher.new(User.current, :project => @project, + :with_subprojects => @with_subprojects, + :author => @author) + @activity.scope_select {|t| !params["show_#{t}"].nil?} + @activity.scope = (@author.nil? ? :default : :all) if @activity.scope.empty? + + events = @activity.events(@date_from, @date_to) + + if events.empty? || stale?(:etag => [events.first, User.current]) + respond_to do |format| + format.html { + @events_by_day = events.group_by(&:event_date) + render :layout => false if request.xhr? + } + format.atom { + title = l(:label_activity) + if @author + title = @author.name + elsif @activity.scope.size == 1 + title = l("label_#{@activity.scope.first.singularize}_plural") + end + render_feed(events, :title => "#{@project || Setting.app_title}: #{title}") + } + end + end + + rescue ActiveRecord::RecordNotFound + render_404 + end + + private + + # TODO: refactor, duplicated in projects_controller + def find_optional_project + return true unless params[:id] + @project = Project.find(params[:id]) + authorize + rescue ActiveRecord::RecordNotFound + render_404 + end + +end diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 44071d214..6bb766386 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -17,17 +17,15 @@ class ProjectsController < ApplicationController menu_item :overview - menu_item :activity, :only => :activity menu_item :roadmap, :only => :roadmap menu_item :files, :only => [:list_files, :add_file] menu_item :settings, :only => :settings - before_filter :find_project, :except => [ :index, :list, :add, :copy, :activity ] - before_filter :find_optional_project, :only => :activity - before_filter :authorize, :except => [ :index, :list, :add, :copy, :archive, :unarchive, :destroy, :activity ] + before_filter :find_project, :except => [ :index, :list, :add, :copy ] + before_filter :authorize, :except => [ :index, :list, :add, :copy, :archive, :unarchive, :destroy] before_filter :authorize_global, :only => :add before_filter :require_admin, :only => [ :copy, :archive, :unarchive, :destroy ] - accept_key_auth :activity, :index + accept_key_auth :index after_filter :only => [:add, :edit, :archive, :unarchive, :destroy] do |controller| if controller.request.post? @@ -313,48 +311,6 @@ class ProjectsController < ApplicationController @versions.reject! {|version| !project_ids.include?(version.project_id) && @issues_by_version[version].blank?} end - def activity - @days = Setting.activity_days_default.to_i - - if params[:from] - begin; @date_to = params[:from].to_date + 1; rescue; end - end - - @date_to ||= Date.today + 1 - @date_from = @date_to - @days - @with_subprojects = params[:with_subprojects].nil? ? Setting.display_subprojects_issues? : (params[:with_subprojects] == '1') - @author = (params[:user_id].blank? ? nil : User.active.find(params[:user_id])) - - @activity = Redmine::Activity::Fetcher.new(User.current, :project => @project, - :with_subprojects => @with_subprojects, - :author => @author) - @activity.scope_select {|t| !params["show_#{t}"].nil?} - @activity.scope = (@author.nil? ? :default : :all) if @activity.scope.empty? - - events = @activity.events(@date_from, @date_to) - - if events.empty? || stale?(:etag => [events.first, User.current]) - respond_to do |format| - format.html { - @events_by_day = events.group_by(&:event_date) - render :layout => false if request.xhr? - } - format.atom { - title = l(:label_activity) - if @author - title = @author.name - elsif @activity.scope.size == 1 - title = l("label_#{@activity.scope.first.singularize}_plural") - end - render_feed(events, :title => "#{@project || Setting.app_title}: #{title}") - } - end - end - - rescue ActiveRecord::RecordNotFound - render_404 - end - private def find_optional_project return true unless params[:id] diff --git a/app/views/projects/activity.rhtml b/app/views/activities/index.html.erb similarity index 100% rename from app/views/projects/activity.rhtml rename to app/views/activities/index.html.erb diff --git a/app/views/boards/index.rhtml b/app/views/boards/index.rhtml index 7cc6a0e2f..6310f942e 100644 --- a/app/views/boards/index.rhtml +++ b/app/views/boards/index.rhtml @@ -30,11 +30,11 @@ <% other_formats_links do |f| %> - <%= f.link_to 'Atom', :url => {:controller => 'projects', :action => 'activity', :id => @project, :show_messages => 1, :key => User.current.rss_key} %> + <%= f.link_to 'Atom', :url => {:controller => 'activities', :action => 'index', :id => @project, :show_messages => 1, :key => User.current.rss_key} %> <% end %> <% content_for :header_tags do %> - <%= auto_discovery_link_tag(:atom, {:controller => 'projects', :action => 'activity', :id => @project, :format => 'atom', :show_messages => 1, :key => User.current.rss_key}) %> + <%= auto_discovery_link_tag(:atom, {:controller => 'activities', :action => 'index', :id => @project, :format => 'atom', :show_messages => 1, :key => User.current.rss_key}) %> <% end %> <% html_title l(:label_board_plural) %> diff --git a/app/views/projects/index.rhtml b/app/views/projects/index.rhtml index b4952e905..a2ba1c389 100644 --- a/app/views/projects/index.rhtml +++ b/app/views/projects/index.rhtml @@ -6,7 +6,7 @@ <%= link_to(l(:label_project_new), {:controller => 'projects', :action => 'add'}, :class => 'icon icon-add') + ' |' if User.current.allowed_to?(:add_project, nil, :global => true) %> <%= link_to(l(:label_issue_view_all), { :controller => 'issues' }) + ' |' if User.current.allowed_to?(:view_issues, nil, :global => true) %> <%= link_to(l(:label_overall_spent_time), { :controller => 'time_entries' }) + ' |' if User.current.allowed_to?(:view_time_entries, nil, :global => true) %> - <%= link_to l(:label_overall_activity), { :controller => 'projects', :action => 'activity' }%> + <%= link_to l(:label_overall_activity), { :controller => 'activities', :action => 'index' }%>

<%=l(:label_project_plural)%>

diff --git a/app/views/projects/show.rhtml b/app/views/projects/show.rhtml index 62ea27848..0ad9a11da 100644 --- a/app/views/projects/show.rhtml +++ b/app/views/projects/show.rhtml @@ -74,7 +74,7 @@ <% end %> <% content_for :header_tags do %> -<%= auto_discovery_link_tag(:atom, {:action => 'activity', :id => @project, :format => 'atom', :key => User.current.rss_key}) %> +<%= auto_discovery_link_tag(:atom, {:controller => 'activities', :action => 'index', :id => @project, :format => 'atom', :key => User.current.rss_key}) %> <% end %> <% html_title(l(:label_overview)) -%> diff --git a/app/views/users/show.rhtml b/app/views/users/show.rhtml index df5aec825..a2f90226c 100644 --- a/app/views/users/show.rhtml +++ b/app/views/users/show.rhtml @@ -35,7 +35,7 @@
<% unless @events_by_day.empty? %> -

<%= link_to l(:label_activity), :controller => 'projects', :action => 'activity', :id => nil, :user_id => @user, :from => @events_by_day.keys.first %>

+

<%= link_to l(:label_activity), :controller => 'activities', :action => 'index', :id => nil, :user_id => @user, :from => @events_by_day.keys.first %>

<%=l(:label_reported_issues)%>: <%= Issue.count(:conditions => ["author_id=?", @user.id]) %> @@ -57,11 +57,11 @@

<% other_formats_links do |f| %> - <%= f.link_to 'Atom', :url => {:controller => 'projects', :action => 'activity', :id => nil, :user_id => @user, :key => User.current.rss_key} %> + <%= f.link_to 'Atom', :url => {:controller => 'activities', :action => 'index', :id => nil, :user_id => @user, :key => User.current.rss_key} %> <% end %> <% content_for :header_tags do %> - <%= auto_discovery_link_tag(:atom, :controller => 'projects', :action => 'activity', :user_id => @user, :format => :atom, :key => User.current.rss_key) %> + <%= auto_discovery_link_tag(:atom, :controller => 'activities', :action => 'index', :user_id => @user, :format => :atom, :key => User.current.rss_key) %> <% end %> <% end %> <%= call_hook :view_account_right_bottom, :user => @user %> diff --git a/app/views/welcome/index.rhtml b/app/views/welcome/index.rhtml index 6ac09c153..982d6da52 100644 --- a/app/views/welcome/index.rhtml +++ b/app/views/welcome/index.rhtml @@ -34,6 +34,6 @@ <% content_for :header_tags do %> <%= auto_discovery_link_tag(:atom, {:controller => 'news', :action => 'index', :key => User.current.rss_key, :format => 'atom'}, :title => "#{Setting.app_title}: #{l(:label_news_latest)}") %> -<%= auto_discovery_link_tag(:atom, {:controller => 'projects', :action => 'activity', :key => User.current.rss_key, :format => 'atom'}, +<%= auto_discovery_link_tag(:atom, {:controller => 'activities', :action => 'index', :key => User.current.rss_key, :format => 'atom'}, :title => "#{Setting.app_title}: #{l(:label_activity)}") %> <% end %> diff --git a/app/views/wiki/special_date_index.rhtml b/app/views/wiki/special_date_index.rhtml index 228737a5e..b34fb8464 100644 --- a/app/views/wiki/special_date_index.rhtml +++ b/app/views/wiki/special_date_index.rhtml @@ -23,11 +23,11 @@ <% unless @pages.empty? %> <% other_formats_links do |f| %> - <%= f.link_to 'Atom', :url => {:controller => 'projects', :action => 'activity', :id => @project, :show_wiki_edits => 1, :key => User.current.rss_key} %> + <%= f.link_to 'Atom', :url => {:controller => 'activities', :action => 'index', :id => @project, :show_wiki_edits => 1, :key => User.current.rss_key} %> <%= f.link_to('HTML', :url => {:action => 'special', :page => 'export'}) if User.current.allowed_to?(:export_wiki_pages, @project) %> <% end %> <% end %> <% content_for :header_tags do %> -<%= auto_discovery_link_tag(:atom, :controller => 'projects', :action => 'activity', :id => @project, :show_wiki_edits => 1, :format => 'atom', :key => User.current.rss_key) %> +<%= auto_discovery_link_tag(:atom, :controller => 'activities', :action => 'index', :id => @project, :show_wiki_edits => 1, :format => 'atom', :key => User.current.rss_key) %> <% end %> diff --git a/config/routes.rb b/config/routes.rb index ea5112216..1465e1ffe 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -187,7 +187,7 @@ ActionController::Routing::Routes.draw do |map| project_views.connect 'projects/:project_id/issues/:copy_from/copy', :controller => 'issues', :action => 'new' end - projects.with_options :action => 'activity', :conditions => {:method => :get} do |activity| + projects.with_options :controller => 'activities', :action => 'index', :conditions => {:method => :get} do |activity| activity.connect 'projects/:id/activity' activity.connect 'projects/:id/activity.:format' activity.connect 'activity', :id => nil diff --git a/lib/redmine.rb b/lib/redmine.rb index 69b7b7a94..7040a1f99 100644 --- a/lib/redmine.rb +++ b/lib/redmine.rb @@ -44,7 +44,7 @@ end # Permissions Redmine::AccessControl.map do |map| - map.permission :view_project, {:projects => [:show, :activity]}, :public => true + map.permission :view_project, {:projects => [:show], :activities => [:index]}, :public => true map.permission :search_project, {:search => :index}, :public => true map.permission :add_project, {:projects => :add}, :require => :loggedin map.permission :edit_project, {:projects => [:settings, :edit]}, :require => :member @@ -185,7 +185,7 @@ end Redmine::MenuManager.map :project_menu do |menu| menu.push :overview, { :controller => 'projects', :action => 'show' } - menu.push :activity, { :controller => 'projects', :action => 'activity' } + menu.push :activity, { :controller => 'activities', :action => 'index' } menu.push :roadmap, { :controller => 'projects', :action => 'roadmap' }, :if => Proc.new { |p| p.shared_versions.any? } menu.push :issues, { :controller => 'issues', :action => 'index' }, :param => :project_id, :caption => :label_issue_plural diff --git a/test/functional/activities_controller_test.rb b/test/functional/activities_controller_test.rb new file mode 100644 index 000000000..ba9c33985 --- /dev/null +++ b/test/functional/activities_controller_test.rb @@ -0,0 +1,87 @@ +require File.dirname(__FILE__) + '/../test_helper' + +class ActivitiesControllerTest < ActionController::TestCase + fixtures :all + + def test_project_index + get :index, :id => 1, :with_subprojects => 0 + assert_response :success + assert_template 'index' + assert_not_nil assigns(:events_by_day) + + assert_tag :tag => "h3", + :content => /#{2.days.ago.to_date.day}/, + :sibling => { :tag => "dl", + :child => { :tag => "dt", + :attributes => { :class => /issue-edit/ }, + :child => { :tag => "a", + :content => /(#{IssueStatus.find(2).name})/, + } + } + } + end + + def test_previous_project_index + get :index, :id => 1, :from => 3.days.ago.to_date + assert_response :success + assert_template 'index' + assert_not_nil assigns(:events_by_day) + + assert_tag :tag => "h3", + :content => /#{3.day.ago.to_date.day}/, + :sibling => { :tag => "dl", + :child => { :tag => "dt", + :attributes => { :class => /issue/ }, + :child => { :tag => "a", + :content => /#{Issue.find(1).subject}/, + } + } + } + end + + def test_global_index + get :index + assert_response :success + assert_template 'index' + assert_not_nil assigns(:events_by_day) + + assert_tag :tag => "h3", + :content => /#{5.day.ago.to_date.day}/, + :sibling => { :tag => "dl", + :child => { :tag => "dt", + :attributes => { :class => /issue/ }, + :child => { :tag => "a", + :content => /#{Issue.find(5).subject}/, + } + } + } + end + + def test_user_index + get :index, :user_id => 2 + assert_response :success + assert_template 'index' + assert_not_nil assigns(:events_by_day) + + assert_tag :tag => "h3", + :content => /#{3.day.ago.to_date.day}/, + :sibling => { :tag => "dl", + :child => { :tag => "dt", + :attributes => { :class => /issue/ }, + :child => { :tag => "a", + :content => /#{Issue.find(1).subject}/, + } + } + } + end + + def test_index_atom_feed + get :index, :format => 'atom' + assert_response :success + assert_template 'common/feed.atom.rxml' + assert_tag :tag => 'entry', :child => { + :tag => 'link', + :attributes => {:href => 'http://test.host/issues/11'}} + end + +end diff --git a/test/functional/projects_controller_test.rb b/test/functional/projects_controller_test.rb index 988496cec..f9c8ce022 100644 --- a/test/functional/projects_controller_test.rb +++ b/test/functional/projects_controller_test.rb @@ -400,87 +400,6 @@ class ProjectsControllerTest < ActionController::TestCase assert assigns(:versions).include?(Version.find(4)), "Shared version not found" assert assigns(:versions).include?(@subproject_version), "Subproject version not found" end - def test_project_activity - get :activity, :id => 1, :with_subprojects => 0 - assert_response :success - assert_template 'activity' - assert_not_nil assigns(:events_by_day) - - assert_tag :tag => "h3", - :content => /#{2.days.ago.to_date.day}/, - :sibling => { :tag => "dl", - :child => { :tag => "dt", - :attributes => { :class => /issue-edit/ }, - :child => { :tag => "a", - :content => /(#{IssueStatus.find(2).name})/, - } - } - } - end - - def test_previous_project_activity - get :activity, :id => 1, :from => 3.days.ago.to_date - assert_response :success - assert_template 'activity' - assert_not_nil assigns(:events_by_day) - - assert_tag :tag => "h3", - :content => /#{3.day.ago.to_date.day}/, - :sibling => { :tag => "dl", - :child => { :tag => "dt", - :attributes => { :class => /issue/ }, - :child => { :tag => "a", - :content => /#{Issue.find(1).subject}/, - } - } - } - end - - def test_global_activity - get :activity - assert_response :success - assert_template 'activity' - assert_not_nil assigns(:events_by_day) - - assert_tag :tag => "h3", - :content => /#{5.day.ago.to_date.day}/, - :sibling => { :tag => "dl", - :child => { :tag => "dt", - :attributes => { :class => /issue/ }, - :child => { :tag => "a", - :content => /#{Issue.find(5).subject}/, - } - } - } - end - - def test_user_activity - get :activity, :user_id => 2 - assert_response :success - assert_template 'activity' - assert_not_nil assigns(:events_by_day) - - assert_tag :tag => "h3", - :content => /#{3.day.ago.to_date.day}/, - :sibling => { :tag => "dl", - :child => { :tag => "dt", - :attributes => { :class => /issue/ }, - :child => { :tag => "a", - :content => /#{Issue.find(1).subject}/, - } - } - } - end - - def test_activity_atom_feed - get :activity, :format => 'atom' - assert_response :success - assert_template 'common/feed.atom.rxml' - assert_tag :tag => 'entry', :child => { - :tag => 'link', - :attributes => {:href => 'http://test.host/issues/11'}} - end - def test_archive @request.session[:user_id] = 1 # admin post :archive, :id => 1 diff --git a/test/integration/routing_test.rb b/test/integration/routing_test.rb index 9b5f39399..62fdaf47a 100644 --- a/test/integration/routing_test.rb +++ b/test/integration/routing_test.rb @@ -19,8 +19,8 @@ require "#{File.dirname(__FILE__)}/../test_helper" class RoutingTest < ActionController::IntegrationTest context "activities" do - should_route :get, "/activity", :controller => 'projects', :action => 'activity', :id => nil - should_route :get, "/activity.atom", :controller => 'projects', :action => 'activity', :id => nil, :format => 'atom' + should_route :get, "/activity", :controller => 'activities', :action => 'index', :id => nil + should_route :get, "/activity.atom", :controller => 'activities', :action => 'index', :id => nil, :format => 'atom' end context "attachments" do @@ -175,8 +175,8 @@ class RoutingTest < ActionController::IntegrationTest should_route :get, "/projects/33/files", :controller => 'projects', :action => 'list_files', :id => '33' should_route :get, "/projects/33/files/new", :controller => 'projects', :action => 'add_file', :id => '33' should_route :get, "/projects/33/roadmap", :controller => 'projects', :action => 'roadmap', :id => '33' - should_route :get, "/projects/33/activity", :controller => 'projects', :action => 'activity', :id => '33' - should_route :get, "/projects/33/activity.atom", :controller => 'projects', :action => 'activity', :id => '33', :format => 'atom' + should_route :get, "/projects/33/activity", :controller => 'activities', :action => 'index', :id => '33' + should_route :get, "/projects/33/activity.atom", :controller => 'activities', :action => 'index', :id => '33', :format => 'atom' should_route :post, "/projects/new", :controller => 'projects', :action => 'add' should_route :post, "/projects.xml", :controller => 'projects', :action => 'add', :format => 'xml'