Commit 5c97a83a authored by Jean-Philippe Lang's avatar Jean-Philippe Lang

Validates sort_key and sort_order params (#2378).

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@2171 e93f8b46-1217-0410-a6f0-8f06a7374b81
parent 7776b5b6
...@@ -27,7 +27,7 @@ class AdminController < ApplicationController ...@@ -27,7 +27,7 @@ class AdminController < ApplicationController
def projects def projects
sort_init 'name', 'asc' sort_init 'name', 'asc'
sort_update sort_update %w(name is_public created_on)
@status = params[:status] ? params[:status].to_i : 1 @status = params[:status] ? params[:status].to_i : 1
c = ARCondition.new(@status == 0 ? "status <> 0" : ["status = ?", @status]) c = ARCondition.new(@status == 0 ? "status <> 0" : ["status = ?", @status])
......
...@@ -35,8 +35,10 @@ class BoardsController < ApplicationController ...@@ -35,8 +35,10 @@ class BoardsController < ApplicationController
end end
def show def show
sort_init "#{Message.table_name}.updated_on", "desc" sort_init 'updated_on', 'desc'
sort_update sort_update 'created_on' => "#{Message.table_name}.created_on",
'replies' => "#{Message.table_name}.replies_count",
'updated_on' => "#{Message.table_name}.updated_on"
@topic_count = @board.topics.count @topic_count = @board.topics.count
@topic_pages = Paginator.new self, @topic_count, per_page_option, params['page'] @topic_pages = Paginator.new self, @topic_count, per_page_option, params['page']
......
...@@ -45,9 +45,10 @@ class IssuesController < ApplicationController ...@@ -45,9 +45,10 @@ class IssuesController < ApplicationController
helper :timelog helper :timelog
def index def index
sort_init "#{Issue.table_name}.id", "desc"
sort_update
retrieve_query retrieve_query
sort_init 'id', 'desc'
sort_update({'id' => "#{Issue.table_name}.id"}.merge(@query.columns.inject({}) {|h, c| h[c.name.to_s] = c.sortable; h}))
if @query.valid? if @query.valid?
limit = per_page_option limit = per_page_option
respond_to do |format| respond_to do |format|
...@@ -78,9 +79,10 @@ class IssuesController < ApplicationController ...@@ -78,9 +79,10 @@ class IssuesController < ApplicationController
end end
def changes def changes
sort_init "#{Issue.table_name}.id", "desc"
sort_update
retrieve_query retrieve_query
sort_init 'id', 'desc'
sort_update({'id' => "#{Issue.table_name}.id"}.merge(@query.columns.inject({}) {|h, c| h[c.name.to_s] = c.sortable; h}))
if @query.valid? if @query.valid?
@journals = Journal.find :all, :include => [ :details, :user, {:issue => [:project, :author, :tracker, :status]} ], @journals = Journal.find :all, :include => [ :details, :user, {:issue => [:project, :author, :tracker, :status]} ],
:conditions => @query.statement, :conditions => @query.statement,
......
...@@ -200,8 +200,12 @@ class ProjectsController < ApplicationController ...@@ -200,8 +200,12 @@ class ProjectsController < ApplicationController
end end
def list_files def list_files
sort_init "#{Attachment.table_name}.filename", "asc" sort_init 'filename', 'asc'
sort_update sort_update 'filename' => "#{Attachment.table_name}.filename",
'created_on' => "#{Attachment.table_name}.created_on",
'size' => "#{Attachment.table_name}.filesize",
'downloads' => "#{Attachment.table_name}.downloads"
@containers = [ Project.find(@project.id, :include => :attachments, :order => sort_clause)] @containers = [ Project.find(@project.id, :include => :attachments, :order => sort_clause)]
@containers += @project.versions.find(:all, :include => :attachments, :order => sort_clause).sort.reverse @containers += @project.versions.find(:all, :include => :attachments, :order => sort_clause).sort.reverse
render :layout => !request.xhr? render :layout => !request.xhr?
......
...@@ -138,7 +138,12 @@ class TimelogController < ApplicationController ...@@ -138,7 +138,12 @@ class TimelogController < ApplicationController
def details def details
sort_init 'spent_on', 'desc' sort_init 'spent_on', 'desc'
sort_update sort_update 'spent_on' => 'spent_on',
'user' => 'user_id',
'activity' => 'activity_id',
'project' => "#{Project.table_name}.name",
'issue' => 'issue_id',
'hours' => 'hours'
cond = ARCondition.new cond = ARCondition.new
if @project.nil? if @project.nil?
......
...@@ -30,7 +30,7 @@ class UsersController < ApplicationController ...@@ -30,7 +30,7 @@ class UsersController < ApplicationController
def list def list
sort_init 'login', 'asc' sort_init 'login', 'asc'
sort_update sort_update %w(login firstname lastname mail admin created_on last_login_on)
@status = params[:status] ? params[:status].to_i : 1 @status = params[:status] ? params[:status].to_i : 1
c = ARCondition.new(@status == 0 ? "status <> 0" : ["status = ?", @status]) c = ARCondition.new(@status == 0 ? "status <> 0" : ["status = ?", @status])
......
...@@ -22,8 +22,8 @@ module QueriesHelper ...@@ -22,8 +22,8 @@ module QueriesHelper
end end
def column_header(column) def column_header(column)
column.sortable ? sort_header_tag(column.sortable, :caption => column.caption, column.sortable ? sort_header_tag(column.name.to_s, :caption => column.caption,
:default_order => column.default_order) : :default_order => column.default_order) :
content_tag('th', column.caption) content_tag('th', column.caption)
end end
......
...@@ -67,23 +67,31 @@ module SortHelper ...@@ -67,23 +67,31 @@ module SortHelper
# Updates the sort state. Call this in the controller prior to calling # Updates the sort state. Call this in the controller prior to calling
# sort_clause. # sort_clause.
# # sort_keys can be either an array or a hash of allowed keys
def sort_update() def sort_update(sort_keys)
if params[:sort_key] sort_key = params[:sort_key]
sort = {:key => params[:sort_key], :order => params[:sort_order]} sort_key = nil unless (sort_keys.is_a?(Array) ? sort_keys.include?(sort_key) : sort_keys[sort_key])
sort_order = (params[:sort_order] == 'desc' ? 'DESC' : 'ASC')
if sort_key
sort = {:key => sort_key, :order => sort_order}
elsif session[@sort_name] elsif session[@sort_name]
sort = session[@sort_name] # Previous sort. sort = session[@sort_name] # Previous sort.
else else
sort = @sort_default sort = @sort_default
end end
session[@sort_name] = sort session[@sort_name] = sort
sort_column = (sort_keys.is_a?(Hash) ? sort_keys[sort[:key]] : sort[:key])
@sort_clause = (sort_column.blank? ? '' : "#{sort_column} #{sort[:order]}")
end end
# Returns an SQL sort clause corresponding to the current sort state. # Returns an SQL sort clause corresponding to the current sort state.
# Use this to sort the controller's table items collection. # Use this to sort the controller's table items collection.
# #
def sort_clause() def sort_clause()
session[@sort_name][:key] + ' ' + (session[@sort_name][:order] || 'ASC') @sort_clause || '' #session[@sort_name][:key] + ' ' + (session[@sort_name][:order] || 'ASC')
end end
# Returns a link which sorts by the named column. # Returns a link which sorts by the named column.
......
...@@ -58,7 +58,7 @@ class Mailer < ActionMailer::Base ...@@ -58,7 +58,7 @@ class Mailer < ActionMailer::Base
subject l(:mail_subject_reminder, issues.size) subject l(:mail_subject_reminder, issues.size)
body :issues => issues, body :issues => issues,
:days => days, :days => days,
:issues_url => url_for(:controller => 'issues', :action => 'index', :set_filter => 1, :assigned_to_id => user.id, :sort_key => 'issues.due_date', :sort_order => 'asc') :issues_url => url_for(:controller => 'issues', :action => 'index', :set_filter => 1, :assigned_to_id => user.id, :sort_key => 'due_date', :sort_order => 'asc')
end end
def document_added(document) def document_added(document)
......
...@@ -33,9 +33,9 @@ ...@@ -33,9 +33,9 @@
<thead><tr> <thead><tr>
<th><%= l(:field_subject) %></th> <th><%= l(:field_subject) %></th>
<th><%= l(:field_author) %></th> <th><%= l(:field_author) %></th>
<%= sort_header_tag("#{Message.table_name}.created_on", :caption => l(:field_created_on)) %> <%= sort_header_tag('created_on', :caption => l(:field_created_on)) %>
<%= sort_header_tag("#{Message.table_name}.replies_count", :caption => l(:label_reply_plural)) %> <%= sort_header_tag('replies', :caption => l(:label_reply_plural)) %>
<%= sort_header_tag("#{Message.table_name}.updated_on", :caption => l(:label_message_last)) %> <%= sort_header_tag('updated_on', :caption => l(:label_message_last)) %>
</tr></thead> </tr></thead>
<tbody> <tbody>
<% @topics.each do |topic| %> <% @topics.each do |topic| %>
......
...@@ -4,7 +4,7 @@ ...@@ -4,7 +4,7 @@
<th><%= link_to image_tag('toggle_check.png'), {}, :onclick => 'toggleIssuesSelection(Element.up(this, "form")); return false;', <th><%= link_to image_tag('toggle_check.png'), {}, :onclick => 'toggleIssuesSelection(Element.up(this, "form")); return false;',
:title => "#{l(:button_check_all)}/#{l(:button_uncheck_all)}" %> :title => "#{l(:button_check_all)}/#{l(:button_uncheck_all)}" %>
</th> </th>
<%= sort_header_tag("#{Issue.table_name}.id", :caption => '#', :default_order => 'desc') %> <%= sort_header_tag('id', :caption => '#', :default_order => 'desc') %>
<% query.columns.each do |column| %> <% query.columns.each do |column| %>
<%= column_header(column) %> <%= column_header(column) %>
<% end %> <% end %>
......
...@@ -8,10 +8,10 @@ ...@@ -8,10 +8,10 @@
<table class="list"> <table class="list">
<thead><tr> <thead><tr>
<%= sort_header_tag("#{Attachment.table_name}.filename", :caption => l(:field_filename)) %> <%= sort_header_tag('filename', :caption => l(:field_filename)) %>
<%= sort_header_tag("#{Attachment.table_name}.created_on", :caption => l(:label_date), :default_order => 'desc') %> <%= sort_header_tag('created_on', :caption => l(:label_date), :default_order => 'desc') %>
<%= sort_header_tag("#{Attachment.table_name}.filesize", :caption => l(:field_filesize), :default_order => 'desc') %> <%= sort_header_tag('size', :caption => l(:field_filesize), :default_order => 'desc') %>
<%= sort_header_tag("#{Attachment.table_name}.downloads", :caption => l(:label_downloads_abbr), :default_order => 'desc') %> <%= sort_header_tag('downloads', :caption => l(:label_downloads_abbr), :default_order => 'desc') %>
<th>MD5</th> <th>MD5</th>
<th></th> <th></th>
</tr></thead> </tr></thead>
......
...@@ -2,10 +2,10 @@ ...@@ -2,10 +2,10 @@
<thead> <thead>
<tr> <tr>
<%= sort_header_tag('spent_on', :caption => l(:label_date), :default_order => 'desc') %> <%= sort_header_tag('spent_on', :caption => l(:label_date), :default_order => 'desc') %>
<%= sort_header_tag('user_id', :caption => l(:label_member)) %> <%= sort_header_tag('user', :caption => l(:label_member)) %>
<%= sort_header_tag('activity_id', :caption => l(:label_activity)) %> <%= sort_header_tag('activity', :caption => l(:label_activity)) %>
<%= sort_header_tag("#{Project.table_name}.name", :caption => l(:label_project)) %> <%= sort_header_tag('project', :caption => l(:label_project)) %>
<%= sort_header_tag('issue_id', :caption => l(:label_issue), :default_order => 'desc') %> <%= sort_header_tag('issue', :caption => l(:label_issue), :default_order => 'desc') %>
<th><%= l(:field_comments) %></th> <th><%= l(:field_comments) %></th>
<%= sort_header_tag('hours', :caption => l(:field_hours)) %> <%= sort_header_tag('hours', :caption => l(:field_hours)) %>
<th></th> <th></th>
......
...@@ -137,6 +137,16 @@ class IssuesControllerTest < Test::Unit::TestCase ...@@ -137,6 +137,16 @@ class IssuesControllerTest < Test::Unit::TestCase
assert_not_nil assigns(:issues) assert_not_nil assigns(:issues)
assert_equal 'application/pdf', @response.content_type assert_equal 'application/pdf', @response.content_type
end end
def test_index_sort
get :index, :sort_key => 'tracker'
assert_response :success
sort_params = @request.session['issuesindex_sort']
assert sort_params.is_a?(Hash)
assert_equal 'tracker', sort_params[:key]
assert_equal 'ASC', sort_params[:order]
end
def test_gantt def test_gantt
get :gantt, :project_id => 1 get :gantt, :project_id => 1
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment