From b1504ceb434c0fd2543db2f27b46c25898e72f6d Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Tue, 3 Jan 2012 20:09:44 +0000 Subject: [PATCH] Adds previous/next links to issue (#2850). git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@8488 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/issues_controller.rb | 19 +++++- app/helpers/queries_helper.rb | 12 ++++ app/helpers/sort_helper.rb | 3 +- app/views/issues/show.html.erb | 9 ++- lib/redmine/core_ext/active_record.rb | 53 +++++++++++++++++ public/stylesheets/application.css | 1 + test/functional/issues_controller_test.rb | 71 +++++++++++++++++++++++ test/unit/query_test.rb | 7 +++ 8 files changed, 172 insertions(+), 3 deletions(-) create mode 100644 lib/redmine/core_ext/active_record.rb diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index 353a3b977..7b3684c85 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -120,7 +120,10 @@ class IssuesController < ApplicationController @priorities = IssuePriority.active @time_entry = TimeEntry.new(:issue => @issue, :project => @issue.project) respond_to do |format| - format.html { render :template => 'issues/show' } + format.html { + retrieve_previous_and_next_issue_ids + render :template => 'issues/show' + } format.api 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") } @@ -277,6 +280,20 @@ private render_404 end + def retrieve_previous_and_next_issue_ids + retrieve_query_from_session + if @query + sort_init(@query.sort_criteria.empty? ? [['id', 'desc']] : @query.sort_criteria) + sort_update(@query.sortable_columns, 'issues_index_sort') + limit = 500 + issue_ids = @query.issue_ids(:order => sort_clause, :limit => (limit + 1)) + if (idx = issue_ids.index(@issue.id.to_s)) && idx < limit + @prev_issue_id = issue_ids[idx - 1].to_i if idx > 0 + @next_issue_id = issue_ids[idx + 1].to_i if idx < (issue_ids.size - 1) + end + end + end + # Used by #edit and #update to set some common instance variables # from the params # TODO: Refactor, not everything in here is needed by #edit diff --git a/app/helpers/queries_helper.rb b/app/helpers/queries_helper.rb index c2d72fb59..afb560c61 100644 --- a/app/helpers/queries_helper.rb +++ b/app/helpers/queries_helper.rb @@ -92,6 +92,18 @@ module QueriesHelper end end + def retrieve_query_from_session + if session[:query] + @query = Query.new(:name => "_", :filters => session[:query][:filters], :group_by => session[:query][:group_by], :column_names => session[:query][:column_names]) + if session[:query].has_key?(:project_id) + @query.project_id = session[:query][:project_id] + else + @query.project = @project + end + @query + end + end + def build_query_from_params if params[:fields] || params[:f] @query.filters = {} diff --git a/app/helpers/sort_helper.rb b/app/helpers/sort_helper.rb index 9f8ac6470..9fda5982b 100644 --- a/app/helpers/sort_helper.rb +++ b/app/helpers/sort_helper.rb @@ -160,7 +160,8 @@ module SortHelper # sort_clause. # - criteria can be either an array or a hash of allowed keys # - def sort_update(criteria) + def sort_update(criteria, sort_name=nil) + sort_name ||= self.sort_name @sort_criteria = SortCriteria.new @sort_criteria.available_criteria = criteria @sort_criteria.from_param(params[:sort] || session[sort_name]) diff --git a/app/views/issues/show.html.erb b/app/views/issues/show.html.erb index c3e23a776..d23458a1f 100644 --- a/app/views/issues/show.html.erb +++ b/app/views/issues/show.html.erb @@ -3,7 +3,14 @@

<%= issue_heading(@issue) %>

- <%= avatar(@issue.author, :size => "50") %> + <% if @prev_issue_id || @next_issue_id %> +
+ <%= link_to_if @prev_issue_id, '« Previous', issue_path(@prev_issue_id), :title => "##{@prev_issue_id}" %> | + <%= link_to_if @next_issue_id, 'Next »', issue_path(@next_issue_id), :title => "##{@next_issue_id}" %> +
+ <% end %> + + <%= avatar(@issue.author, :size => "50") %>
<%= render_issue_subject_with_tree(@issue) %> diff --git a/lib/redmine/core_ext/active_record.rb b/lib/redmine/core_ext/active_record.rb new file mode 100644 index 000000000..ba57b639a --- /dev/null +++ b/lib/redmine/core_ext/active_record.rb @@ -0,0 +1,53 @@ +# Redmine - project management software +# Copyright (C) 2006-2011 Jean-Philippe Lang +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +module ActiveRecord + class Base + def self.find_ids(options={}) + find_ids_with_associations(options) + end + end + + module Associations + module ClassMethods + def find_ids_with_associations(options = {}) + catch :invalid_query do + join_dependency = ActiveRecord::Associations::ClassMethods::JoinDependency.new(self, merge_includes(scope(:find, :include), options[:include]), options[:joins]) + return connection.select_values(construct_ids_finder_sql_with_included_associations(options, join_dependency)) + end + [] + end + + def construct_ids_finder_sql_with_included_associations(options, join_dependency) + scope = scope(:find) + sql = "SELECT #{table_name}.id FROM #{(scope && scope[:from]) || options[:from] || quoted_table_name} " + sql << join_dependency.join_associations.collect{|join| join.association_join }.join + + add_joins!(sql, options[:joins], scope) + add_conditions!(sql, options[:conditions], scope) + add_limited_ids_condition!(sql, options, join_dependency) if !using_limitable_reflections?(join_dependency.reflections) && ((scope && scope[:limit]) || options[:limit]) + + add_group!(sql, options[:group], options[:having], scope) + add_order!(sql, options[:order], scope) + add_limit!(sql, options, scope) if using_limitable_reflections?(join_dependency.reflections) + add_lock!(sql, options, scope) + + return sanitize_sql(sql) + end + end + end +end diff --git a/public/stylesheets/application.css b/public/stylesheets/application.css index 6baac680e..876623794 100644 --- a/public/stylesheets/application.css +++ b/public/stylesheets/application.css @@ -292,6 +292,7 @@ div.issue div.subject p {margin: 0; margin-bottom: 0.1em; font-size: 90%; color: div.issue div.subject>div>p { margin-top: 0.5em; } div.issue div.subject h3 {margin: 0; margin-bottom: 0.1em;} div.issue span.private { position:relative; bottom: 2px; text-transform: uppercase; background: #d22; color: #fff; font-weight:bold; padding: 0px 2px 0px 2px; font-size: 60%; margin-right: 2px; border-radius: 2px; -moz-border-radius: 2px;} +div.issue .next-prev-links {color:#999;} #issue_tree table.issues, #relations table.issues { border: 0; } #issue_tree td.checkbox, #relations td.checkbox {display:none;} diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index fb32a9db4..e76de084d 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -920,6 +920,77 @@ class IssuesControllerTest < ActionController::TestCase :descendant => {:tag => 'a', :attributes => {:href => '/issues/1'}} end + def test_show_should_not_display_prev_next_links_without_query_in_session + get :show, :id => 1 + assert_response :success + assert_nil assigns(:prev_issue_id) + assert_nil assigns(:next_issue_id) + end + + def test_show_should_display_prev_next_links_with_query_in_session + @request.session[:query] = {:filters => {'status_id' => {:values => [''], :operator => 'o'}}, :project_id => nil} + @request.session['issues_index_sort'] = 'id' + + with_settings :display_subprojects_issues => '0' do + get :show, :id => 3 + end + + assert_response :success + # Previous and next issues for all projects + assert_equal 2, assigns(:prev_issue_id) + assert_equal 5, assigns(:next_issue_id) + + assert_tag 'a', :attributes => {:href => '/issues/2'}, :content => /Previous/ + assert_tag 'a', :attributes => {:href => '/issues/5'}, :content => /Next/ + end + + def test_show_should_display_prev_next_links_with_project_query_in_session + @request.session[:query] = {:filters => {'status_id' => {:values => [''], :operator => 'o'}}, :project_id => 1} + @request.session['issues_index_sort'] = 'id' + + with_settings :display_subprojects_issues => '0' do + get :show, :id => 3 + end + + assert_response :success + # Previous and next issues inside project + assert_equal 2, assigns(:prev_issue_id) + assert_equal 7, assigns(:next_issue_id) + + assert_tag 'a', :attributes => {:href => '/issues/2'}, :content => /Previous/ + assert_tag 'a', :attributes => {:href => '/issues/7'}, :content => /Next/ + end + + def test_show_should_not_display_prev_link_for_first_issue + @request.session[:query] = {:filters => {'status_id' => {:values => [''], :operator => 'o'}}, :project_id => 1} + @request.session['issues_index_sort'] = 'id' + + with_settings :display_subprojects_issues => '0' do + get :show, :id => 1 + end + + assert_response :success + assert_nil assigns(:prev_issue_id) + assert_equal 2, assigns(:next_issue_id) + + assert_no_tag 'a', :content => /Previous/ + assert_tag 'a', :attributes => {:href => '/issues/2'}, :content => /Next/ + end + + def test_show_should_not_display_prev_next_links_for_issue_not_in_query_results + @request.session[:query] = {:filters => {'status_id' => {:values => [''], :operator => 'c'}}, :project_id => 1} + @request.session['issues_index_sort'] = 'id' + + get :show, :id => 1 + + assert_response :success + assert_nil assigns(:prev_issue_id) + assert_nil assigns(:next_issue_id) + + assert_no_tag 'a', :content => /Previous/ + assert_no_tag 'a', :content => /Next/ + end + def test_show_atom get :show, :id => 2, :format => 'atom' assert_response :success diff --git a/test/unit/query_test.rb b/test/unit/query_test.rb index 37a8face9..6fc0bd2e8 100644 --- a/test/unit/query_test.rb +++ b/test/unit/query_test.rb @@ -612,6 +612,13 @@ class QueryTest < ActiveSupport::TestCase assert_equal %w(Fixnum), count_by_group.values.collect {|k| k.class.name}.uniq end + def test_issue_ids + q = Query.new(:name => '_') + order = "issues.subject, issues.id" + issues = q.issues(:order => order) + assert_equal issues.map(&:id).map(&:to_s), q.issue_ids(:order => order) + end + def test_label_for q = Query.new assert_equal 'Assignee', q.label_for('assigned_to_id')