Merge pull request #10422 from colinux/better-sanitize-stats-year

Tech (carte): ignore silencieusement les params invalides

app/controllers/carte_controller.rb

@@ -1,6 +1,13 @@
 class CarteController < ApplicationController
   def show
-    @map_filter = MapFilter.new(params)
+    @map_filter = MapFilter.new(params.fetch(:map_filter, {}).permit(:kind, :year))
+    @map_filter.validate
+
+    # Reset to default params in case of invalid params injection
+    @map_filter.kind = MapFilter.new.kind if @map_filter.errors.key?(:kind)
+    @map_filter.year = MapFilter.new.year if @map_filter.errors.key?(:year)
+    @map_filter.errors.clear
+
     @map_filter.stats = stats
   end
 

app/models/map_filter.rb

@@ -1,34 +1,21 @@
 class MapFilter
-  # https://api.rubyonrails.org/v7.1.1/classes/ActiveModel/Errors.html
-
-  include ActiveModel::Conversion
-  extend ActiveModel::Translation
-  extend ActiveModel::Naming
+  include ActiveModel::Model
+  include ActiveModel::Attributes
 
   LEGEND = {
-    nb_demarches: { 'nothing': -1, 'small': 20, 'medium': 50, 'large': 100, 'xlarge': 500 },
-    nb_dossiers: { 'nothing': -1, 'small': 500, 'medium': 2000, 'large': 10000, 'xlarge': 50000 }
-  }
+    "nb_demarches" => { 'nothing': -1, 'small': 20, 'medium': 50, 'large': 100, 'xlarge': 500 },
+    "nb_dossiers" => { 'nothing': -1, 'small': 500, 'medium': 2000, 'large': 10000, 'xlarge': 50000 }
+  }.freeze
+
+  YEARS_INTERVAL = 2018..Date.current.year
 
   attr_accessor :stats
-  attr_reader :errors
 
-  def initialize(params)
-    @params = params[:map_filter]&.permit(:kind, :year) || {}
-    @errors = ActiveModel::Errors.new(self)
-  end
+  attribute :year, :integer
+  validates :year, numericality: { only_integer: true, greater_than_or_equal_to: YEARS_INTERVAL.begin, less_than_or_equal_to: YEARS_INTERVAL.end }
 
-  def persisted?
-    false
-  end
-
-  def kind
-    @params[:kind]&.to_sym || :nb_demarches
-  end
-
-  def year
-    @params[:year].presence
-  end
+  attribute :kind, default: "nb_demarches"
+  validates :kind, inclusion: { in: LEGEND.keys }
 
   def kind_buttons
     LEGEND.keys.map do
@@ -41,7 +28,7 @@ class MapFilter
   end
 
   def css_class_for_departement(departement)
-    if kind == :nb_demarches
+    if kind == "nb_demarches"
       kind_legend_keys.reverse.find do
         nb_demarches_for_departement(departement) > LEGEND[kind][_1]
       end

app/views/carte/show.html.erb

@@ -249,7 +249,7 @@
           <% end %>
           <div class="fr-select-group">
             <%= map_form.label :year, class: 'fr-label' %>
-            <%= map_form.select(:year, (2018..Date.current.year).to_a.reverse, { include_blank: t(:from_beginning, scope: 'activemodel.attributes.map_filter') }, { class: "fr-select" }) %>
+            <%= map_form.select(:year, MapFilter::YEARS_INTERVAL.to_a.reverse, { include_blank: t(:from_beginning, scope: 'activemodel.attributes.map_filter') }, { class: "fr-select" }) %>
           </div>
           <%= map_form.submit(name: nil, class: 'hidden', data: { autosubmit_target: 'submitter' } ) %>
         <% end %>

spec/controllers/carte_controller_spec.rb

@@ -18,5 +18,13 @@ describe CarteController do
       get :show, params: { map_filter: { year: 2020 } }
       expect(subject.stats['75']).to eq({ nb_demarches: 1, nb_dossiers: 20 })
     end
+
+    it 'gracefully ignore invalid params' do
+      get :show, params: { map_filter: { year: "not!" } }
+      expect(subject.stats['75']).to eq({ nb_demarches: 2, nb_dossiers: 50 })
+
+      get :show, params: { map_filter: { kind: "nimp" } }
+      expect(subject.stats['75']).to eq({ nb_demarches: 2, nb_dossiers: 50 })
+    end
   end
 end

spec/models/map_filter_spec.rb

@@ -6,7 +6,7 @@ describe MapFilter do
   end
 
   describe 'css_class_for_departement' do
-    let(:params) { { kind: :nb_demarches } }
+    let(:params) { { kind: "nb_demarches" } }
     context 'for nb_demarches' do
       it 'return class css' do
         expect(map_filter.css_class_for_departement('63')).to eq :medium
@@ -14,7 +14,7 @@ describe MapFilter do
     end
 
     context 'fr nb_dossiers' do
-      let(:params) { { kind: :nb_dossiers } }
+      let(:params) { { kind: "nb_dossiers" } }
       it 'return class css' do
         expect(map_filter.css_class_for_departement('63')).to eq :medium
       end