Merge pull request #753 from opf/fix/ma_query_controller

app/controllers/queries_controller.rb

@@ -27,6 +27,8 @@
 # See doc/COPYRIGHT.rdoc for more details.
 #++
 
+#TODO: convert this controller and its routes to not use the catch-all-route
+
 class QueriesController < ApplicationController
   menu_item :issues
   before_filter :find_query, :except => :new
@@ -59,31 +61,29 @@ class QueriesController < ApplicationController
 
 private
   def prepare_for_creating
-    @query = Query.new(params[:query])
-    @query.project = params[:query_is_for_all] ? nil : @project
+    @query = Query.new params[:query] ? permitted_params.query : nil
+    @query.project = @project unless params[:query_is_for_all]
+    prepare_query @query
     @query.user = User.current
-    @query.is_public = false unless User.current.allowed_to?(:manage_public_queries, @project) || User.current.admin?
-
-    @query.add_filters(params[:fields] || params[:f], params[:operators] || params[:op], params[:values] || params[:v]) if params[:fields] || params[:f]
-    @query.group_by ||= params[:group_by]
-    @query.display_sums ||= params[:display_sums].present?
-    @query.column_names = params[:c] if params[:c]
-    @query.column_names = nil if params[:default_columns]
   end
 
   def prepare_for_editing
     if request.post?
-      @query.filters = []
-      @query.add_filters(params[:fields] || params[:f], params[:operators] || params[:op], params[:values] || params[:v]) if params[:fields] || params[:f]
-      @query.attributes = params[:query]
-      @query.project = nil if params[:query_is_for_all]
-      @query.is_public = false unless User.current.allowed_to?(:manage_public_queries, @project) || User.current.admin?
-      @query.group_by ||= params[:group_by]
-      @query.column_names = params[:c] if params[:c]
-      @query.column_names = nil if params[:default_columns]
+      @query.update_attributes permitted_params.query
+      prepare_query @query
     end
   end
 
+  def prepare_query(query)
+    @query.is_public = false unless User.current.allowed_to?(:manage_public_queries, @project) || User.current.admin?
+    view_context.add_filter_from_params if params[:fields] || params[:f]
+    @query.group_by ||= params[:group_by]
+    @query.project = nil if params[:query_is_for_all]
+    @query.display_sums ||= params[:display_sums].present?
+    @query.column_names = params[:c] if params[:c]
+    @query.column_names = nil if params[:default_columns]
+  end
+
   def find_query
     @query = Query.find(params[:id])
     @project = @query.project

app/helpers/queries_helper.rb

@@ -76,6 +76,11 @@ module QueriesHelper
     end
   end
 
+  def add_filter_from_params
+    @query.filters = []
+    @query.add_filters(params[:fields] || params[:f], params[:operators] || params[:op], params[:values] || params[:v])
+  end
+
   # Retrieve query from session or build a new query
   def retrieve_query
     if !params[:query_id].blank?
@@ -91,8 +96,7 @@ module QueriesHelper
         @query = Query.new({name: "_"}, initialize_with_default_filter: true)
         @query.project = @project
         if params[:fields] || params[:f]
-          @query.filters = []
-          @query.add_filters(params[:fields] || params[:f], params[:operators] || params[:op], params[:values] || params[:v])
+          add_filter_from_params
         else
           @query.available_work_package_filters.keys.each do |field|
             @query.add_short_filter(field, params[field]) if params[field]

app/models/permitted_params.rb

@@ -66,12 +66,8 @@ class PermittedParams < Struct.new(:params, :user)
     permitted_attributes[key].concat(params)
   end
 
-  def project_type
-    params.require(:project_type).permit(*self.class.permitted_attributes[:project_type])
-  end
-
-  def project_type_move
-    params.require(:project_type).permit(*self.class.permitted_attributes[:move_to])
+  def board_move
+    params.require(:board).permit(*self.class.permitted_attributes[:move_to])
   end
 
   def color
@@ -110,6 +106,16 @@ class PermittedParams < Struct.new(:params, :user)
     params.permit(*self.class.permitted_attributes[:group_membership])
   end
 
+  def new_work_package(args = {})
+    permitted = permitted_attributes(:new_work_package, args)
+
+    permitted_params = params.require(:work_package).permit(*permitted)
+
+    permitted_params.merge!(custom_field_values(:work_package))
+
+    permitted_params
+  end
+
   def planning_element_type
     params.require(:planning_element_type).permit(*self.class.permitted_attributes[:planning_element_type])
   end
@@ -122,18 +128,27 @@ class PermittedParams < Struct.new(:params, :user)
     params.require(:planning_element).permit(*self.class.permitted_attributes[:planning_element])
   end
 
-  def status
-    params.require(:status).permit(*self.class.permitted_attributes[:status])
+  def project_type
+    params.require(:project_type).permit(*self.class.permitted_attributes[:project_type])
   end
 
-  def new_work_package(args = {})
-    permitted = permitted_attributes(:new_work_package, args)
-
-    permitted_params = params.require(:work_package).permit(*permitted)
+  def project_type_move
+    params.require(:project_type).permit(*self.class.permitted_attributes[:move_to])
+  end
 
-    permitted_params.merge!(custom_field_values(:work_package))
+  def query
+    # there is a wierd bug in strong_parameters gem which makes the permit call
+    # on the sort_criteria pattern return the sort_criteria-hash contens AND
+    # the sort_criteria hash itself (again with content) in the same hash.
+    # Here we try to circumvent this
+    p = params.require(:query).permit(*self.class.permitted_attributes[:query])
+    p[:sort_criteria] = params.require(:query).permit(:sort_criteria => {'0' => [], '1' => [], '2' => []})
+    p[:sort_criteria].delete :sort_criteria
+    p
+  end
 
-    permitted_params
+  def status
+    params.require(:status).permit(*self.class.permitted_attributes[:status])
   end
 
   alias :update_work_package :new_work_package
@@ -192,10 +207,6 @@ class PermittedParams < Struct.new(:params, :user)
     params.require(:content).permit(*self.class.permitted_attributes[:wiki_content])
   end
 
-  def board_move
-    params.require(:board).permit(*self.class.permitted_attributes[:move_to])
-  end
-
   protected
 
   def custom_field_values(key)
@@ -326,6 +337,11 @@ class PermittedParams < Struct.new(:params, :user)
         :allows_association,
         :type_ids => [],
         :reported_project_status_ids => []],
+      :query => [
+        :name,
+        :display_sums,
+        :is_public,
+        :group_by],
       :status => [
         :name,
         :default_done_ratio,

app/models/query.rb

@@ -28,6 +28,7 @@
 #++
 
 class Query < ActiveRecord::Base
+  include ActiveModel::ForbiddenAttributesProtection
   include Queries::WorkPackages::AvailableFilterOptions
 
   alias_method :available_filters, :available_work_package_filters # referenced in plugin patches - currently there are only work package queries and filters

doc/CHANGELOG.md

@@ -31,6 +31,7 @@ See doc/COPYRIGHT.rdoc for more details.
 
 * `#3407` Fix: [Roadmap] Missing dropdown menu for displaying work packages by different criteria
 * `#3334` [CodeClimate] Mass Assignment WikiController
+* `#3336` Fix: use permitted_params for queries controller
 * `#3455` Fix: [Projects] Tab "Types" missing in newly created projects
 
 ## 3.0.0pre40