Merge pull request #5819 from betagouv/dev

2020-12-18-01

app/controllers/instructeurs/procedures_controller.rb

@@ -44,13 +44,12 @@ module Instructeurs
 
     def show
       @procedure = procedure
-
-      @current_filters = current_filters
-      @available_fields_to_filters = available_fields_to_filters
       # Technically, procedure_presentation already sets the attribute.
       # Setting it here to make clear that it is used by the view
       @procedure_presentation = procedure_presentation
-      @displayed_fields_values = displayed_fields_values
+
+      @current_filters = current_filters
+      @displayed_fields_options, @displayed_fields_selected = procedure_presentation.displayed_fields_for_select
 
       @a_suivre_dossiers = current_instructeur
         .dossiers
@@ -139,63 +138,25 @@ module Instructeurs
     end
 
     def update_displayed_fields
-      values = params[:values]
+      procedure_presentation.update_displayed_fields(params[:values])
-
-      if values.nil?
-        values = []
-      end
-
-      fields = values.map do |value|
-        find_field(*value.split('/'))
-      end
-
-      procedure_presentation.update(displayed_fields: fields)
-
-      current_sort = procedure_presentation.sort
-      if !values.include?(field_id(current_sort))
-        procedure_presentation.update(sort: Procedure.default_sort)
-      end
 
       redirect_back(fallback_location: instructeur_procedure_url(procedure))
     end
 
     def update_sort
-      current_sort = procedure_presentation.sort
+      procedure_presentation.update_sort(params[:table], params[:column])
-      table = params[:table]
-      column = params[:column]
-
-      if table == current_sort['table'] && column == current_sort['column']
-        order = current_sort['order'] == 'asc' ? 'desc' : 'asc'
-      else
-        order = 'asc'
-      end
-
-      sort = {
-        'table' => table,
-        'column' => column,
-        'order' => order
-      }
-
-      procedure_presentation.update(sort: sort)
 
       redirect_back(fallback_location: instructeur_procedure_url(procedure))
     end
 
     def add_filter
-      if params[:value].present?
+      procedure_presentation.add_filter(statut, params[:field], params[:value])
-        procedure_presentation.add_filter(statut, params[:field], params[:value])
-      end
 
       redirect_back(fallback_location: instructeur_procedure_url(procedure))
     end
 
     def remove_filter
-      filters = procedure_presentation.filters
+      procedure_presentation.remove_filter(statut, params[:field], params[:value])
-
-      to_remove = params.values_at(:table, :column, :value)
-      filters[statut].reject! { |filter| filter.values_at('table', 'column', 'value') == to_remove }
-
-      procedure_presentation.update(filters: filters)
 
       redirect_back(fallback_location: instructeur_procedure_url(procedure))
     end
@@ -271,14 +232,6 @@ module Instructeurs
       @ods_export = Export.find_for_format_and_groupe_instructeurs(:ods, groupe_instructeurs_for_procedure)
     end
 
-    def find_field(table, column)
-      procedure_presentation.fields.find { |c| c['table'] == table && c['column'] == column }
-    end
-
-    def field_id(field)
-      field.values_at('table', 'column').join('/')
-    end
-
     def assign_to
       current_instructeur.assign_to.joins(:groupe_instructeur).find_by(groupe_instructeurs: { procedure: procedure })
     end
@@ -316,18 +269,10 @@ module Instructeurs
       procedure_presentation
     end
 
-    def displayed_fields_values
-      procedure_presentation.displayed_fields.map { |field| field_id(field) }
-    end
-
     def current_filters
       @current_filters ||= procedure_presentation.filters[statut]
     end
 
-    def available_fields_to_filters
-      procedure_presentation.fields_for_select
-    end
-
     def kaminarize(current_page, total)
       @dossiers.instance_eval <<-EVAL
         def current_page

app/graphql/api/v2/schema.rb

@@ -50,6 +50,7 @@ class Api::V2::Schema < GraphQL::Schema
     Types::Champs::RepetitionChampType,
     Types::Champs::SiretChampType,
     Types::Champs::TextChampType,
+    Types::Champs::TitreIdentiteChampType,
     Types::GeoAreas::ParcelleCadastraleType,
     Types::GeoAreas::SelectionUtilisateurType,
     Types::PersonneMoraleType,

app/graphql/schema.graphql

@@ -1197,6 +1197,33 @@ type TextChamp implements Champ {
   value: String
 }
 
+type TitreIdentiteChamp implements Champ {
+  grantType: TitreIdentiteGrantType!
+  id: ID!
+
+  """
+  Libellé du champ.
+  """
+  label: String!
+
+  """
+  La valeur du champ sous forme texte.
+  """
+  stringValue: String
+}
+
+enum TitreIdentiteGrantType {
+  """
+  Françe Connect
+  """
+  france_connect
+
+  """
+  Pièce justificative
+  """
+  piece_justificative
+}
+
 enum TypeDeChamp {
   """
   Adresse

app/graphql/types/champ_type.rb

@@ -33,6 +33,8 @@ module Types
           Types::Champs::LinkedDropDownListChampType
         when ::Champs::CiviliteChamp
           Types::Champs::CiviliteChampType
+        when ::Champs::TitreIdentiteChamp
+          Types::Champs::TitreIdentiteChampType
         else
           Types::Champs::TextChampType
         end

app/graphql/types/champs/titre_identite_champ_type.rb

@@ -0,0 +1,16 @@
+module Types::Champs
+  class TitreIdentiteChampType < Types::BaseObject
+    implements Types::ChampType
+
+    class TitreIdentiteGrantTypeType < Types::BaseEnum
+      value(TypesDeChamp::TitreIdentiteTypeDeChamp::FRANCE_CONNECT, "Françe Connect")
+      value(TypesDeChamp::TitreIdentiteTypeDeChamp::PIECE_JUSTIFICATIVE, "Pièce justificative")
+    end
+
+    field :grant_type, TitreIdentiteGrantTypeType, null: false
+
+    def grant_type
+      TypesDeChamp::TitreIdentiteTypeDeChamp::PIECE_JUSTIFICATIVE
+    end
+  end
+end

app/jobs/cron/fix_missing_antivirus_analysis_job.rb

@@ -0,0 +1,12 @@
+class Cron::FixMissingAntivirusAnalysis < Cron::CronJob
+  self.schedule_expression = "every day at 2 am"
+
+  def perform
+    ActiveStorage::Blob.where("metadata like '%\"virus_scan_result\":\"pending%'").each do |b|
+      begin
+        VirusScannerJob.perform_now(b)
+      rescue ActiveStorage::IntegrityError
+      end
+    end
+  end
+end

app/models/dossier.rb

@@ -152,6 +152,16 @@ class Dossier < ApplicationRecord
   scope :updated_since,       -> (since) { where('dossiers.updated_at >= ?', since) }
   scope :created_since,       -> (since) { where('dossiers.en_construction_at >= ?', since) }
 
+  scope :with_type_de_champ, -> (stable_id) {
+    joins('INNER JOIN champs ON champs.dossier_id = dossiers.id INNER JOIN types_de_champ ON types_de_champ.id = champs.type_de_champ_id')
+      .where('types_de_champ.private = FALSE AND types_de_champ.stable_id = ?', stable_id)
+  }
+
+  scope :with_type_de_champ_private, -> (stable_id) {
+    joins('INNER JOIN champs ON champs.dossier_id = dossiers.id INNER JOIN types_de_champ ON types_de_champ.id = champs.type_de_champ_id')
+      .where('types_de_champ.private = TRUE AND types_de_champ.stable_id = ?', stable_id)
+  }
+
   scope :all_state,                   -> { not_archived.state_not_brouillon }
   scope :en_construction,             -> { not_archived.state_en_construction }
   scope :en_instruction,              -> { not_archived.state_en_instruction }

app/models/procedure_presentation.rb

@@ -65,23 +65,24 @@ class ProcedurePresentation < ApplicationRecord
     fields.concat procedure.types_de_champ
       .where.not(type_champ: explanatory_types_de_champ)
       .order(:id)
-      .map { |type_de_champ| field_hash(type_de_champ.libelle, 'type_de_champ', type_de_champ.id.to_s) }
+      .map { |type_de_champ| field_hash(type_de_champ.libelle, 'type_de_champ', type_de_champ.stable_id.to_s) }
 
     fields.concat procedure.types_de_champ_private
       .where.not(type_champ: explanatory_types_de_champ)
       .order(:id)
-      .map { |type_de_champ| field_hash(type_de_champ.libelle, 'type_de_champ_private', type_de_champ.id.to_s) }
+      .map { |type_de_champ| field_hash(type_de_champ.libelle, 'type_de_champ_private', type_de_champ.stable_id.to_s) }
 
     fields
   end
 
-  def fields_for_select
+  def displayed_fields_for_select
-    fields.map do |field|
+    [
-      [field['label'], "#{field['table']}/#{field['column']}"]
+      fields.map { |field| [field['label'], field_id(field)] },
-    end
+      displayed_fields.map { |field| field_id(field) }
+    ]
   end
 
-  def displayed_field_values(dossier)
+  def displayed_fields_values(dossier)
     displayed_fields.map { |field| get_value(dossier, field['table'], field['column']) }
   end
 
@@ -92,30 +93,34 @@ class ProcedurePresentation < ApplicationRecord
     when 'notifications'
       dossiers_id_with_notification = dossiers.merge(instructeur.followed_dossiers).with_notifications.ids
       if order == 'desc'
-        return dossiers_id_with_notification +
+        dossiers_id_with_notification +
             (dossiers.order('dossiers.updated_at desc').ids - dossiers_id_with_notification)
       else
-        return (dossiers.order('dossiers.updated_at asc').ids - dossiers_id_with_notification) +
+        (dossiers.order('dossiers.updated_at asc').ids - dossiers_id_with_notification) +
             dossiers_id_with_notification
       end
-    when 'type_de_champ', 'type_de_champ_private'
+    when 'type_de_champ'
-      return dossiers
+      dossiers
-          .includes(table == 'type_de_champ' ? :champs : :champs_private)
+        .with_type_de_champ(column)
-          .where("champs.type_de_champ_id = #{column.to_i}")
+        .order("champs.value #{order}")
-          .order("champs.value #{order}")
+        .pluck(:id)
-          .pluck(:id)
+    when 'type_de_champ_private'
+      dossiers
+        .with_type_de_champ_private(column)
+        .order("champs.value #{order}")
+        .pluck(:id)
     when 'followers_instructeurs'
       assert_supported_column(table, column)
       # LEFT OUTER JOIN allows to keep dossiers without assignated instructeurs yet
-      return dossiers
+      dossiers
-          .includes(:followers_instructeurs)
+        .includes(:followers_instructeurs)
-          .joins('LEFT OUTER JOIN users instructeurs_users ON instructeurs_users.instructeur_id = instructeurs.id')
+        .joins('LEFT OUTER JOIN users instructeurs_users ON instructeurs_users.instructeur_id = instructeurs.id')
-          .order("instructeurs_users.email #{order}")
+        .order("instructeurs_users.email #{order}")
-          .pluck(:id)
+        .pluck(:id)
     when 'self', 'user', 'individual', 'etablissement', 'groupe_instructeur'
-      return (table == 'self' ? dossiers : dossiers.includes(table))
+      (table == 'self' ? dossiers : dossiers.includes(table))
-          .order("#{self.class.sanitized_column(table, column)} #{order}")
+        .order("#{self.class.sanitized_column(table, column)} #{order}")
-          .pluck(:id)
+        .pluck(:id)
     end
   end
 
@@ -128,12 +133,12 @@ class ProcedurePresentation < ApplicationRecord
           .map { |v| Time.zone.parse(v).beginning_of_day rescue nil }
           .compact
         dossiers.filter_by_datetimes(column, dates)
-      when 'type_de_champ', 'type_de_champ_private'
+      when 'type_de_champ'
-        relation = table == 'type_de_champ' ? :champs : :champs_private
+        dossiers.with_type_de_champ(column)
-        dossiers
+          .filter_ilike(:champs, :value, values)
-          .includes(relation)
+      when 'type_de_champ_private'
-          .where("champs.type_de_champ_id = ?", column.to_i)
+        dossiers.with_type_de_champ_private(column)
-          .filter_ilike(relation, :value, values)
+          .filter_ilike(:champs_private, :value, values)
       when 'etablissement'
         if column == 'entreprise_date_creation'
           dates = values
@@ -187,8 +192,7 @@ class ProcedurePresentation < ApplicationRecord
   def human_value_for_filter(filter)
     case filter['table']
     when 'type_de_champ', 'type_de_champ_private'
-      type_de_champ = TypeDeChamp.find_by(id: filter['column'])
+      find_type_de_champ(filter['column']).dynamic_type.filter_to_human(filter['value'])
-      type_de_champ.dynamic_type.filter_to_human(filter['value'])
     else
       filter['value']
     end
@@ -196,16 +200,15 @@ class ProcedurePresentation < ApplicationRecord
 
   def add_filter(statut, field, value)
     if value.present?
-      updated_filters = self.filters
       table, column = field.split('/')
       label = find_field(table, column)['label']
 
       case table
       when 'type_de_champ', 'type_de_champ_private'
-        type_de_champ = TypeDeChamp.find_by(id: column)
+        value = find_type_de_champ(column).dynamic_type.human_to_filter(value)
-        value = type_de_champ.dynamic_type.human_to_filter(value)
       end
 
+      updated_filters = filters.dup
       updated_filters[statut] << {
         'label' => label,
         'table' => table,
@@ -213,14 +216,61 @@ class ProcedurePresentation < ApplicationRecord
         'value' => value
       }
 
-      update(filters: updated_filters)
+      update!(filters: updated_filters)
     end
   end
 
+  def remove_filter(statut, field, value)
+    table, column = field.split('/')
+
+    updated_filters = filters.dup
+    updated_filters[statut] = filters[statut].reject do |filter|
+      filter.values_at('table', 'column', 'value') == [table, column, value]
+    end
+
+    update!(filters: updated_filters)
+  end
+
+  def update_displayed_fields(values)
+    if values.nil?
+      values = []
+    end
+
+    fields = values.map { |value| find_field(*value.split('/')) }
+
+    update!(displayed_fields: fields)
+
+    if !values.include?(field_id(sort))
+      update!(sort: Procedure.default_sort)
+    end
+  end
+
+  def update_sort(table, column)
+    order = if sort.values_at('table', 'column') == [table, column]
+      sort['order'] == 'asc' ? 'desc' : 'asc'
+    else
+      'asc'
+    end
+
+    update!(sort: {
+      'table' => table,
+      'column' => column,
+      'order' => order
+    })
+  end
+
   private
 
+  def field_id(field)
+    field.values_at('table', 'column').join('/')
+  end
+
   def find_field(table, column)
-    fields.find { |c| c['table'] == table && c['column'] == column }
+    fields.find { |field| field.values_at('table', 'column') == [table, column] }
+  end
+
+  def find_type_de_champ(column)
+    TypeDeChamp.order(:revision_id).find_by(stable_id: column)
   end
 
   def check_allowed_displayed_fields
@@ -241,7 +291,8 @@ class ProcedurePresentation < ApplicationRecord
   end
 
   def check_allowed_filter_columns
-    filters.each do |_, columns|
+    filters.each do |key, columns|
+      return true if key == 'migrated'
       columns.each do |column|
         check_allowed_field(:filters, column)
       end
@@ -264,9 +315,9 @@ class ProcedurePresentation < ApplicationRecord
     when 'followers_instructeurs'
       dossier.send(table)&.map { |g| g.send(column) }&.join(', ')
     when 'type_de_champ'
-      dossier.champs.find { |c| c.type_de_champ_id == column.to_i }.value
+      dossier.champs.find { |c| c.stable_id == column.to_i }.to_s
     when 'type_de_champ_private'
-      dossier.champs_private.find { |c| c.type_de_champ_id == column.to_i }.value
+      dossier.champs_private.find { |c| c.stable_id == column.to_i }.to_s
     when 'groupe_instructeur'
       dossier.groupe_instructeur.label
     end
@@ -309,10 +360,6 @@ class ProcedurePresentation < ApplicationRecord
       .join('.')
   end
 
-  def dossier_field_service
-    @dossier_field_service ||= DossierFieldService.new
-  end
-
   def assert_supported_column(table, column)
     if table == 'followers_instructeurs' && column != 'email'
       raise ArgumentError, 'Table `followers_instructeurs` only supports the `email` column.'

app/models/types_de_champ/titre_identite_type_de_champ.rb

@@ -1,2 +1,4 @@
 class TypesDeChamp::TitreIdentiteTypeDeChamp < TypesDeChamp::TypeDeChampBase
+  FRANCE_CONNECT = 'france_connect'
+  PIECE_JUSTIFICATIVE = 'piece_justificative'
 end

app/views/instructeurs/procedures/show.html.haml

@@ -83,7 +83,7 @@
         #filter-menu.dropdown-content.left-aligned.fade-in-down
           = form_tag add_filter_instructeur_procedure_path(@procedure), method: :post, class: 'dropdown-form large' do
             = label_tag :field,  "Colonne"
-            = select_tag :field, options_for_select(@available_fields_to_filters)
+            = select_tag :field, options_for_select(@displayed_fields_options)
             %br
             = label_tag :value, "Valeur"
             = text_field_tag :value
@@ -98,7 +98,7 @@
           - if i > 0
             ou
           %span.filter
-            = link_to remove_filter_instructeur_procedure_path(@procedure, { statut: @statut, table: filter['table'], column: filter['column'], value: filter['value'] }) do
+            = link_to remove_filter_instructeur_procedure_path(@procedure, { statut: @statut, field: "#{filter['table']}/#{filter['column']}", value: filter['value'] }) do
               %img.close-icon{ src: image_url("close.svg") }
             = "#{filter['label'].truncate(50)} : #{@procedure_presentation.human_value_for_filter(filter)}"
       %table.table.dossiers-table.hoverable
@@ -123,8 +123,7 @@
                 #custom-menu.dropdown-content.fade-in-down
                   = form_tag update_displayed_fields_instructeur_procedure_path(@procedure), method: :patch, class: 'dropdown-form columns-form' do
                     = select_tag :values,
-                      options_for_select(@procedure_presentation.fields_for_select,
+                      options_for_select(@displayed_fields_options, selected: @displayed_fields_selected),
-                        selected: @displayed_fields_values),
                       multiple: true,
                       class: 'select2-limited'
                     = submit_tag "Enregistrer", class: 'button'
@@ -142,11 +141,9 @@
                 = link_to(instructeur_dossier_path(@procedure, dossier), class: 'cell-link') do
                   = dossier.id
 
-              - @procedure_presentation.displayed_field_values(dossier).each do |value|
+              - @procedure_presentation.displayed_fields_values(dossier).each do |value|
                 %td
-                  / FIXME: value should automatically fallback to `""` instead of nil
+                  = link_to(value, instructeur_dossier_path(@procedure, dossier), class: 'cell-link')
-                  / #get_value should call to_s on the champ
-                  = link_to(value || "", instructeur_dossier_path(@procedure, dossier), class: 'cell-link')
 
               %td.status-col
                 = link_to(instructeur_dossier_path(@procedure, dossier), class: 'cell-link') do

config/brakeman.ignore

@@ -1,5 +1,25 @@
 {
   "ignored_warnings": [
+    {
+      "warning_type": "SQL Injection",
+      "warning_code": 0,
+      "fingerprint": "25d6ed4f7f9120faf69596aa97d9e0558fd86817583b99b9b7879aff43ec2751",
+      "check_name": "SQL",
+      "message": "Possible SQL injection",
+      "file": "app/models/procedure_presentation.rb",
+      "line": 111,
+      "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
+      "code": "dossiers.with_type_de_champ_private(column).order(\"champs.value #{order}\")",
+      "render_path": null,
+      "location": {
+        "type": "method",
+        "class": "ProcedurePresentation",
+        "method": "sorted_ids"
+      },
+      "user_input": "order",
+      "confidence": "Weak",
+      "note": "`table`, `column` and `order` come from the model, which is validated to prevent injection attacks. Furthermore, `table` and `column` are escaped."
+    },
     {
       "warning_type": "Cross-Site Scripting",
       "warning_code": 2,
@@ -10,7 +30,19 @@
       "line": 28,
       "link": "https://brakemanscanner.org/docs/warning_types/cross_site_scripting",
       "code": "current_user.dossiers.includes(:procedure).find(params[:id]).procedure.monavis_embed",
-      "render_path": [{"type":"controller","class":"Users::DossiersController","method":"merci","line":181,"file":"app/controllers/users/dossiers_controller.rb"}],
+      "render_path": [
+        {
+          "type": "controller",
+          "class": "Users::DossiersController",
+          "method": "merci",
+          "line": 195,
+          "file": "app/controllers/users/dossiers_controller.rb",
+          "rendered": {
+            "name": "users/dossiers/merci",
+            "file": "app/views/users/dossiers/merci.html.haml"
+          }
+        }
+      ],
       "location": {
         "type": "template",
         "template": "users/dossiers/merci"
@@ -26,7 +58,7 @@
       "check_name": "Redirect",
       "message": "Possible unprotected redirect",
       "file": "app/controllers/instructeurs/procedures_controller.rb",
-      "line": 198,
+      "line": 176,
       "link": "https://brakemanscanner.org/docs/warning_types/redirect/",
       "code": "redirect_to(Export.find_or_create_export(params[:export_format], current_instructeur.groupe_instructeurs.where(:procedure => procedure)).file.service_url)",
       "render_path": null,
@@ -59,6 +91,26 @@
       "confidence": "Medium",
       "note": "The table and column are escaped, which should make this safe"
     },
+    {
+      "warning_type": "SQL Injection",
+      "warning_code": 0,
+      "fingerprint": "d6031dd493ff36d62af2d75d0b1e4606c665413a62ef26a847902af4ad97d81f",
+      "check_name": "SQL",
+      "message": "Possible SQL injection",
+      "file": "app/models/procedure_presentation.rb",
+      "line": 106,
+      "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
+      "code": "dossiers.with_type_de_champ(column).order(\"champs.value #{order}\")",
+      "render_path": null,
+      "location": {
+        "type": "method",
+        "class": "ProcedurePresentation",
+        "method": "sorted_ids"
+      },
+      "user_input": "order",
+      "confidence": "Weak",
+      "note": "`table`, `column` and `order` come from the model, which is validated to prevent injection attacks. Furthermore, `table` and `column` are escaped."
+    },
     {
       "warning_type": "SQL Injection",
       "warning_code": 0,
@@ -66,7 +118,7 @@
       "check_name": "SQL",
       "message": "Possible SQL injection",
       "file": "app/models/procedure_presentation.rb",
-      "line": 107,
+      "line": 123,
       "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
       "code": "((\"self\" == \"self\") ? (dossiers) : (dossiers.includes(\"self\"))).order(\"#{self.class.sanitized_column(\"self\", column)} #{order}\")",
       "render_path": null,
@@ -79,26 +131,6 @@
       "confidence": "Weak",
       "note": "`table`, `column` and `order` come from the model, which is validated to prevent injection attacks. Furthermore, `table` and `column` are escaped."
     },
-    {
-      "warning_type": "SQL Injection",
-      "warning_code": 0,
-      "fingerprint": "f85ed20c14a223884f624d744ff99070f6fc0697d918f54a08e7786ad70bb243",
-      "check_name": "SQL",
-      "message": "Possible SQL injection",
-      "file": "app/models/procedure_presentation.rb",
-      "line": 95,
-      "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
-      "code": "dossiers.includes(((\"type_de_champ\" == \"type_de_champ\") ? (:champs) : (:champs_private))).where(\"champs.type_de_champ_id = #{column.to_i}\").order(\"champs.value #{order}\")",
-      "render_path": null,
-      "location": {
-        "type": "method",
-        "class": "ProcedurePresentation",
-        "method": "sorted_ids"
-      },
-      "user_input": "order",
-      "confidence": "Weak",
-      "note": "`column` and `order` come from the model, which is validated to prevent injection attacks. Furthermore, the sql injection attack on `column` would need to survive the `to_i`"
-    },
     {
       "warning_type": "SQL Injection",
       "warning_code": 0,
@@ -106,7 +138,7 @@
       "check_name": "SQL",
       "message": "Possible SQL injection",
       "file": "app/models/procedure_presentation.rb",
-      "line": 103,
+      "line": 119,
       "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
       "code": "dossiers.includes(:followers_instructeurs).joins(\"LEFT OUTER JOIN users instructeurs_users ON instructeurs_users.instructeur_id = instructeurs.id\").order(\"instructeurs_users.email #{order}\")",
       "render_path": null,
@@ -117,9 +149,9 @@
       },
       "user_input": "order",
       "confidence": "Weak",
-      "note": ""
+      "note": "`table`, `column` and `order` come from the model, which is validated to prevent injection attacks. Furthermore, `table` and `column` are escaped."
     }
   ],
-  "updated": "2019-12-12 16:36:32 +0100",
+  "updated": "2020-09-17 13:28:51 +0200",
-  "brakeman_version": "4.3.1"
+  "brakeman_version": "4.8.1"
 }

lib/tasks/deployment/20201001161931_migrate_filters_to_use_stable_id.rake

@@ -0,0 +1,50 @@
+namespace :after_party do
+  desc 'Deployment task: migrate_filters_to_use_stable_id'
+  task migrate_filters_to_use_stable_id: :environment do
+    puts "Running deploy task 'migrate_filters_to_use_stable_id'"
+
+    procedure_presentations = ProcedurePresentation.where("filters -> 'migrated' IS NULL")
+    progress = ProgressReport.new(procedure_presentations.count)
+    procedure_presentations.find_each do |procedure_presentation|
+      filters = procedure_presentation.filters
+      sort = procedure_presentation.sort
+      displayed_fields = procedure_presentation.displayed_fields
+
+      ['tous', 'suivis', 'traites', 'a-suivre', 'archives'].each do |statut|
+        filters[statut] = filters[statut].map do |filter|
+          table, column = filter.values_at('table', 'column')
+          if table && (table == 'type_de_champ' || table == 'type_de_champ_private')
+            type_de_champ = TypeDeChamp.find_by(id: column)
+            filter['column'] = type_de_champ&.stable_id&.to_s
+          end
+          filter
+        end
+      end
+
+      table, column = sort.values_at('table', 'column')
+      if table && (table == 'type_de_champ' || table == 'type_de_champ_private')
+        type_de_champ = TypeDeChamp.find_by(id: column)
+        sort['column'] = type_de_champ&.stable_id&.to_s
+      end
+
+      displayed_fields = displayed_fields.map do |displayed_field|
+        table, column = displayed_field.values_at('table', 'column')
+        if table && (table == 'type_de_champ' || table == 'type_de_champ_private')
+          type_de_champ = TypeDeChamp.find_by(id: column)
+          displayed_field['column'] = type_de_champ&.stable_id&.to_s
+        end
+        displayed_field
+      end
+
+      filters['migrated'] = true
+      procedure_presentation.update_columns(filters: filters, sort: sort, displayed_fields: displayed_fields)
+      progress.inc
+    end
+    progress.finish
+
+    # Update task as completed.  If you remove the line below, the task will
+    # run with every deploy (or every time you call after_party:run).
+    AfterParty::TaskRecord
+      .create version: AfterParty::TaskRecorder.new(__FILE__).timestamp
+  end
+end

spec/lib/tasks/deployment/20201001161931_migrate_filters_to_use_stable_id_spec.rb

@@ -0,0 +1,60 @@
+describe '20201001161931_migrate_filters_to_use_stable_id' do
+  let(:rake_task) { Rake::Task['after_party:migrate_filters_to_use_stable_id'] }
+
+  let(:procedure) { create(:procedure, :with_instructeur, :with_type_de_champ) }
+  let(:type_de_champ) { procedure.types_de_champ.first }
+  let(:sort) do
+    {
+      "table" => "type_de_champ",
+      "column" => type_de_champ.id.to_s,
+      "order" => "asc"
+    }
+  end
+  let(:filters) do
+    {
+      'tous' => [
+        {
+          "label" => "test",
+          "table" => "type_de_champ",
+          "column" => type_de_champ.id.to_s,
+          "value" => "test"
+        }
+      ],
+      'suivis' => [],
+      'traites' => [],
+      'a-suivre' => [],
+      'archives' => []
+    }
+  end
+  let(:displayed_fields) do
+    [
+      {
+        "label" => "test",
+        "table" => "type_de_champ",
+        "column" => type_de_champ.id.to_s
+      }
+    ]
+  end
+  let!(:procedure_presentation) do
+    type_de_champ.update_column(:stable_id, 13)
+    procedure_presentation = create(:procedure_presentation, procedure: procedure, assign_to: procedure.groupe_instructeurs.first.assign_tos.first)
+    procedure_presentation.update_columns(sort: sort, filters: filters, displayed_fields: displayed_fields)
+    procedure_presentation
+  end
+
+  before do
+    rake_task.invoke
+    procedure_presentation.reload
+  end
+
+  after { rake_task.reenable }
+
+  context "should migrate procedure_presentation" do
+    it "columns are updated" do
+      expect(procedure_presentation.sort['column']).to eq(type_de_champ.stable_id.to_s)
+      expect(procedure_presentation.filters['tous'][0]['column']).to eq(type_de_champ.stable_id.to_s)
+      expect(procedure_presentation.displayed_fields[0]['column']).to eq(type_de_champ.stable_id.to_s)
+      expect(procedure_presentation.filters['migrated']).to eq(true)
+    end
+  end
+end

spec/models/procedure_presentation_spec.rb

@@ -3,7 +3,7 @@ describe ProcedurePresentation do
   let(:instructeur) { create(:instructeur) }
   let(:assign_to) { create(:assign_to, procedure: procedure, instructeur: instructeur) }
   let(:first_type_de_champ) { assign_to.procedure.types_de_champ.first }
-  let(:first_type_de_champ_id) { first_type_de_champ.id.to_s }
+  let(:first_type_de_champ_id) { first_type_de_champ.stable_id.to_s }
   let(:procedure_presentation) {
     create(:procedure_presentation,
       assign_to: assign_to,
@@ -72,10 +72,10 @@ describe ProcedurePresentation do
           { "label" => 'SIRET', "table" => 'etablissement', "column" => 'siret' },
           { "label" => 'Libellé NAF', "table" => 'etablissement', "column" => 'libelle_naf' },
           { "label" => 'Code postal', "table" => 'etablissement', "column" => 'code_postal' },
-          { "label" => tdc_1.libelle, "table" => 'type_de_champ', "column" => tdc_1.id.to_s },
+          { "label" => tdc_1.libelle, "table" => 'type_de_champ', "column" => tdc_1.stable_id.to_s },
-          { "label" => tdc_2.libelle, "table" => 'type_de_champ', "column" => tdc_2.id.to_s },
+          { "label" => tdc_2.libelle, "table" => 'type_de_champ', "column" => tdc_2.stable_id.to_s },
-          { "label" => tdc_private_1.libelle, "table" => 'type_de_champ_private', "column" => tdc_private_1.id.to_s },
+          { "label" => tdc_private_1.libelle, "table" => 'type_de_champ_private', "column" => tdc_private_1.stable_id.to_s },
-          { "label" => tdc_private_2.libelle, "table" => 'type_de_champ_private', "column" => tdc_private_2.id.to_s }
+          { "label" => tdc_private_2.libelle, "table" => 'type_de_champ_private', "column" => tdc_private_2.stable_id.to_s }
         ]
       }
 
@@ -104,7 +104,7 @@ describe ProcedurePresentation do
     end
   end
 
-  describe "#fields_for_select" do
+  describe "#displayed_fields_for_select" do
     subject { create(:procedure_presentation, assign_to: assign_to) }
 
     before do
@@ -122,13 +122,13 @@ describe ProcedurePresentation do
       ])
     end
 
-    it { expect(subject.fields_for_select).to eq([["label1", "table1/column1"], ["label2", "table2/column2"]]) }
+    it { expect(subject.displayed_fields_for_select).to eq([[["label1", "table1/column1"], ["label2", "table2/column2"]], ["user/email"]]) }
   end
 
   describe '#get_value' do
     let(:procedure_presentation) { create(:procedure_presentation, procedure: procedure, assign_to: assign_to, displayed_fields: [{ 'table' => table, 'column' => column }]) }
 
-    subject { procedure_presentation.displayed_field_values(dossier).first }
+    subject { procedure_presentation.displayed_fields_values(dossier).first }
 
     context 'for self table' do
       let(:table) { 'self' }
@@ -217,14 +217,14 @@ describe ProcedurePresentation do
       let!(:follow2) { create(:follow, dossier: dossier, instructeur: create(:instructeur, email: 'user2@host')) }
 
       it "return emails of followers instructeurs" do
-        emails_to_display = procedure_presentation.displayed_field_values(dossier).first.split(', ').sort
+        emails_to_display = procedure_presentation.displayed_fields_values(dossier).first.split(', ').sort
         expect(emails_to_display).to eq ["user1@host", "user2@host"]
       end
     end
 
     context 'for type_de_champ table' do
       let(:table) { 'type_de_champ' }
-      let(:column) { procedure.types_de_champ.first.id.to_s }
+      let(:column) { procedure.types_de_champ.first.stable_id.to_s }
 
       let(:dossier) { create(:dossier, procedure: procedure) }
 
@@ -235,7 +235,7 @@ describe ProcedurePresentation do
 
     context 'for type_de_champ_private table' do
       let(:table) { 'type_de_champ_private' }
-      let(:column) { procedure.types_de_champ_private.first.id.to_s }
+      let(:column) { procedure.types_de_champ_private.first.stable_id.to_s }
 
       let(:dossier) { create(:dossier, procedure: procedure) }
 
@@ -325,7 +325,7 @@ describe ProcedurePresentation do
 
     context 'for type_de_champ table' do
       let(:table) { 'type_de_champ' }
-      let(:column) { procedure.types_de_champ.first.id.to_s }
+      let(:column) { procedure.types_de_champ.first.stable_id.to_s }
       let(:order) { 'desc' } # Asc works the same, no extra test required
 
       let(:beurre_dossier) { create(:dossier, procedure: procedure) }
@@ -341,7 +341,7 @@ describe ProcedurePresentation do
 
     context 'for type_de_champ_private table' do
       let(:table) { 'type_de_champ_private' }
-      let(:column) { procedure.types_de_champ_private.first.id.to_s }
+      let(:column) { procedure.types_de_champ_private.first.stable_id.to_s }
       let(:order) { 'asc' } # Desc works the same, no extra test required
 
       let(:biere_dossier) { create(:dossier, procedure: procedure) }
@@ -496,7 +496,7 @@ describe ProcedurePresentation do
     end
 
     context 'for type_de_champ table' do
-      let(:filter) { [{ 'table' => 'type_de_champ', 'column' => type_de_champ.id.to_s, 'value' => 'keep' }] }
+      let(:filter) { [{ 'table' => 'type_de_champ', 'column' => type_de_champ.stable_id.to_s, 'value' => 'keep' }] }
 
       let(:kept_dossier) { create(:dossier, procedure: procedure) }
       let(:discarded_dossier) { create(:dossier, procedure: procedure) }
@@ -514,8 +514,8 @@ describe ProcedurePresentation do
       context 'with multiple search values' do
         let(:filter) do
           [
-            { 'table' => 'type_de_champ', 'column' => type_de_champ.id.to_s, 'value' => 'keep' },
+            { 'table' => 'type_de_champ', 'column' => type_de_champ.stable_id.to_s, 'value' => 'keep' },
-            { 'table' => 'type_de_champ', 'column' => type_de_champ.id.to_s, 'value' => 'and' }
+            { 'table' => 'type_de_champ', 'column' => type_de_champ.stable_id.to_s, 'value' => 'and' }
           ]
         end
 
@@ -533,7 +533,7 @@ describe ProcedurePresentation do
       end
 
       context 'with yes_no type_de_champ' do
-        let(:filter) { [{ 'table' => 'type_de_champ', 'column' => type_de_champ.id.to_s, 'value' => 'true' }] }
+        let(:filter) { [{ 'table' => 'type_de_champ', 'column' => type_de_champ.stable_id.to_s, 'value' => 'true' }] }
         let(:procedure) { create(:procedure, :with_yes_no) }
 
         before do
@@ -546,7 +546,7 @@ describe ProcedurePresentation do
     end
 
     context 'for type_de_champ_private table' do
-      let(:filter) { [{ 'table' => 'type_de_champ_private', 'column' => type_de_champ_private.id.to_s, 'value' => 'keep' }] }
+      let(:filter) { [{ 'table' => 'type_de_champ_private', 'column' => type_de_champ_private.stable_id.to_s, 'value' => 'keep' }] }
 
       let(:kept_dossier) { create(:dossier, procedure: procedure) }
       let(:discarded_dossier) { create(:dossier, procedure: procedure) }
@@ -562,8 +562,8 @@ describe ProcedurePresentation do
       context 'with multiple search values' do
         let(:filter) do
           [
-            { 'table' => 'type_de_champ_private', 'column' => type_de_champ_private.id.to_s, 'value' => 'keep' },
+            { 'table' => 'type_de_champ_private', 'column' => type_de_champ_private.stable_id.to_s, 'value' => 'keep' },
-            { 'table' => 'type_de_champ_private', 'column' => type_de_champ_private.id.to_s, 'value' => 'and' }
+            { 'table' => 'type_de_champ_private', 'column' => type_de_champ_private.stable_id.to_s, 'value' => 'and' }
           ]
         end
 
@@ -762,7 +762,7 @@ describe ProcedurePresentation do
 
     context 'for type de champ' do
       let(:table) { 'type_de_champ' }
-      let(:column) { procedure.types_de_champ.first.id.to_s }
+      let(:column) { procedure.types_de_champ.first.stable_id.to_s }
 
       it 'preloads the champs relation' do
         # Ideally, we would only preload the champs for the matching column
@@ -779,7 +779,7 @@ describe ProcedurePresentation do
 
     context 'for type de champ private' do
       let(:table) { 'type_de_champ_private' }
-      let(:column) { procedure.types_de_champ_private.first.id.to_s }
+      let(:column) { procedure.types_de_champ_private.first.stable_id.to_s }
 
       it 'preloads the champs relation' do
         # Ideally, we would only preload the champs for the matching column