Merge pull request #5579 from tchak/filters-by-stable-id

Enable filters across revisions
This commit is contained in:
LeSim 2020-12-17 11:05:05 +01:00 committed by GitHub
commit 03107d095b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 237 additions and 162 deletions

View file

@ -44,13 +44,12 @@ module Instructeurs
def show def show
@procedure = procedure @procedure = procedure
@current_filters = current_filters
@available_fields_to_filters = available_fields_to_filters
# Technically, procedure_presentation already sets the attribute. # Technically, procedure_presentation already sets the attribute.
# Setting it here to make clear that it is used by the view # Setting it here to make clear that it is used by the view
@procedure_presentation = procedure_presentation @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 @a_suivre_dossiers = current_instructeur
.dossiers .dossiers
@ -139,63 +138,25 @@ module Instructeurs
end end
def update_displayed_fields 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)) redirect_back(fallback_location: instructeur_procedure_url(procedure))
end end
def update_sort 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)) redirect_back(fallback_location: instructeur_procedure_url(procedure))
end end
def add_filter 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)) redirect_back(fallback_location: instructeur_procedure_url(procedure))
end end
def remove_filter 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)) redirect_back(fallback_location: instructeur_procedure_url(procedure))
end end
@ -271,14 +232,6 @@ module Instructeurs
@ods_export = Export.find_for_format_and_groupe_instructeurs(:ods, groupe_instructeurs_for_procedure) @ods_export = Export.find_for_format_and_groupe_instructeurs(:ods, groupe_instructeurs_for_procedure)
end 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 def assign_to
current_instructeur.assign_to.joins(:groupe_instructeur).find_by(groupe_instructeurs: { procedure: procedure }) current_instructeur.assign_to.joins(:groupe_instructeur).find_by(groupe_instructeurs: { procedure: procedure })
end end
@ -316,18 +269,10 @@ module Instructeurs
procedure_presentation procedure_presentation
end end
def displayed_fields_values
procedure_presentation.displayed_fields.map { |field| field_id(field) }
end
def current_filters def current_filters
@current_filters ||= procedure_presentation.filters[statut] @current_filters ||= procedure_presentation.filters[statut]
end end
def available_fields_to_filters
procedure_presentation.fields_for_select
end
def kaminarize(current_page, total) def kaminarize(current_page, total)
@dossiers.instance_eval <<-EVAL @dossiers.instance_eval <<-EVAL
def current_page def current_page

View file

@ -152,6 +152,16 @@ class Dossier < ApplicationRecord
scope :updated_since, -> (since) { where('dossiers.updated_at >= ?', since) } scope :updated_since, -> (since) { where('dossiers.updated_at >= ?', since) }
scope :created_since, -> (since) { where('dossiers.en_construction_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 :all_state, -> { not_archived.state_not_brouillon }
scope :en_construction, -> { not_archived.state_en_construction } scope :en_construction, -> { not_archived.state_en_construction }
scope :en_instruction, -> { not_archived.state_en_instruction } scope :en_instruction, -> { not_archived.state_en_instruction }

View file

@ -65,23 +65,24 @@ class ProcedurePresentation < ApplicationRecord
fields.concat procedure.types_de_champ fields.concat procedure.types_de_champ
.where.not(type_champ: explanatory_types_de_champ) .where.not(type_champ: explanatory_types_de_champ)
.order(:id) .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 fields.concat procedure.types_de_champ_private
.where.not(type_champ: explanatory_types_de_champ) .where.not(type_champ: explanatory_types_de_champ)
.order(:id) .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 fields
end 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 end
def displayed_field_values(dossier) def displayed_fields_values(dossier)
displayed_fields.map { |field| get_value(dossier, field['table'], field['column']) } displayed_fields.map { |field| get_value(dossier, field['table'], field['column']) }
end end
@ -92,30 +93,34 @@ class ProcedurePresentation < ApplicationRecord
when 'notifications' when 'notifications'
dossiers_id_with_notification = dossiers.merge(instructeur.followed_dossiers).with_notifications.ids dossiers_id_with_notification = dossiers.merge(instructeur.followed_dossiers).with_notifications.ids
if order == 'desc' if order == 'desc'
return dossiers_id_with_notification + dossiers_id_with_notification +
(dossiers.order('dossiers.updated_at desc').ids - dossiers_id_with_notification) (dossiers.order('dossiers.updated_at desc').ids - dossiers_id_with_notification)
else 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 dossiers_id_with_notification
end 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' when 'followers_instructeurs'
assert_supported_column(table, column) assert_supported_column(table, column)
# LEFT OUTER JOIN allows to keep dossiers without assignated instructeurs yet # 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' 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
end end
@ -128,12 +133,12 @@ class ProcedurePresentation < ApplicationRecord
.map { |v| Time.zone.parse(v).beginning_of_day rescue nil } .map { |v| Time.zone.parse(v).beginning_of_day rescue nil }
.compact .compact
dossiers.filter_by_datetimes(column, dates) 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' when 'etablissement'
if column == 'entreprise_date_creation' if column == 'entreprise_date_creation'
dates = values dates = values
@ -187,8 +192,7 @@ class ProcedurePresentation < ApplicationRecord
def human_value_for_filter(filter) def human_value_for_filter(filter)
case filter['table'] case filter['table']
when 'type_de_champ', 'type_de_champ_private' 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 else
filter['value'] filter['value']
end end
@ -196,16 +200,15 @@ class ProcedurePresentation < ApplicationRecord
def add_filter(statut, field, value) def add_filter(statut, field, value)
if value.present? if value.present?
updated_filters = self.filters
table, column = field.split('/') table, column = field.split('/')
label = find_field(table, column)['label'] label = find_field(table, column)['label']
case table case table
when 'type_de_champ', 'type_de_champ_private' 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 end
updated_filters = filters.dup
updated_filters[statut] << { updated_filters[statut] << {
'label' => label, 'label' => label,
'table' => table, 'table' => table,
@ -213,14 +216,61 @@ class ProcedurePresentation < ApplicationRecord
'value' => value 'value' => value
} }
update(filters: updated_filters) update!(filters: updated_filters)
end end
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 private
def field_id(field)
field.values_at('table', 'column').join('/')
end
def find_field(table, column) 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 end
def check_allowed_displayed_fields def check_allowed_displayed_fields
@ -241,7 +291,8 @@ class ProcedurePresentation < ApplicationRecord
end end
def check_allowed_filter_columns def check_allowed_filter_columns
filters.each do |_, columns| filters.each do |key, columns|
return true if key == 'migrated'
columns.each do |column| columns.each do |column|
check_allowed_field(:filters, column) check_allowed_field(:filters, column)
end end
@ -264,9 +315,9 @@ class ProcedurePresentation < ApplicationRecord
when 'followers_instructeurs' when 'followers_instructeurs'
dossier.send(table)&.map { |g| g.send(column) }&.join(', ') dossier.send(table)&.map { |g| g.send(column) }&.join(', ')
when 'type_de_champ' 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' 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' when 'groupe_instructeur'
dossier.groupe_instructeur.label dossier.groupe_instructeur.label
end end
@ -309,10 +360,6 @@ class ProcedurePresentation < ApplicationRecord
.join('.') .join('.')
end end
def dossier_field_service
@dossier_field_service ||= DossierFieldService.new
end
def assert_supported_column(table, column) def assert_supported_column(table, column)
if table == 'followers_instructeurs' && column != 'email' if table == 'followers_instructeurs' && column != 'email'
raise ArgumentError, 'Table `followers_instructeurs` only supports the `email` column.' raise ArgumentError, 'Table `followers_instructeurs` only supports the `email` column.'

View file

@ -83,7 +83,7 @@
#filter-menu.dropdown-content.left-aligned.fade-in-down #filter-menu.dropdown-content.left-aligned.fade-in-down
= form_tag add_filter_instructeur_procedure_path(@procedure), method: :post, class: 'dropdown-form large' do = form_tag add_filter_instructeur_procedure_path(@procedure), method: :post, class: 'dropdown-form large' do
= label_tag :field, "Colonne" = label_tag :field, "Colonne"
= select_tag :field, options_for_select(@available_fields_to_filters) = select_tag :field, options_for_select(@displayed_fields_options)
%br %br
= label_tag :value, "Valeur" = label_tag :value, "Valeur"
= text_field_tag :value = text_field_tag :value
@ -98,7 +98,7 @@
- if i > 0 - if i > 0
ou ou
%span.filter %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") } %img.close-icon{ src: image_url("close.svg") }
= "#{filter['label'].truncate(50)} : #{@procedure_presentation.human_value_for_filter(filter)}" = "#{filter['label'].truncate(50)} : #{@procedure_presentation.human_value_for_filter(filter)}"
%table.table.dossiers-table.hoverable %table.table.dossiers-table.hoverable
@ -123,8 +123,7 @@
#custom-menu.dropdown-content.fade-in-down #custom-menu.dropdown-content.fade-in-down
= form_tag update_displayed_fields_instructeur_procedure_path(@procedure), method: :patch, class: 'dropdown-form columns-form' do = form_tag update_displayed_fields_instructeur_procedure_path(@procedure), method: :patch, class: 'dropdown-form columns-form' do
= select_tag :values, = 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, multiple: true,
class: 'select2-limited' class: 'select2-limited'
= submit_tag "Enregistrer", class: 'button' = submit_tag "Enregistrer", class: 'button'
@ -142,11 +141,9 @@
= link_to(instructeur_dossier_path(@procedure, dossier), class: 'cell-link') do = link_to(instructeur_dossier_path(@procedure, dossier), class: 'cell-link') do
= dossier.id = dossier.id
- @procedure_presentation.displayed_field_values(dossier).each do |value| - @procedure_presentation.displayed_fields_values(dossier).each do |value|
%td %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 %td.status-col
= link_to(instructeur_dossier_path(@procedure, dossier), class: 'cell-link') do = link_to(instructeur_dossier_path(@procedure, dossier), class: 'cell-link') do

View file

@ -1,5 +1,25 @@
{ {
"ignored_warnings": [ "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_type": "Cross-Site Scripting",
"warning_code": 2, "warning_code": 2,
@ -10,7 +30,19 @@
"line": 28, "line": 28,
"link": "https://brakemanscanner.org/docs/warning_types/cross_site_scripting", "link": "https://brakemanscanner.org/docs/warning_types/cross_site_scripting",
"code": "current_user.dossiers.includes(:procedure).find(params[:id]).procedure.monavis_embed", "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": { "location": {
"type": "template", "type": "template",
"template": "users/dossiers/merci" "template": "users/dossiers/merci"
@ -26,7 +58,7 @@
"check_name": "Redirect", "check_name": "Redirect",
"message": "Possible unprotected redirect", "message": "Possible unprotected redirect",
"file": "app/controllers/instructeurs/procedures_controller.rb", "file": "app/controllers/instructeurs/procedures_controller.rb",
"line": 198, "line": 176,
"link": "https://brakemanscanner.org/docs/warning_types/redirect/", "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)", "code": "redirect_to(Export.find_or_create_export(params[:export_format], current_instructeur.groupe_instructeurs.where(:procedure => procedure)).file.service_url)",
"render_path": null, "render_path": null,
@ -59,6 +91,26 @@
"confidence": "Medium", "confidence": "Medium",
"note": "The table and column are escaped, which should make this safe" "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_type": "SQL Injection",
"warning_code": 0, "warning_code": 0,
@ -66,7 +118,7 @@
"check_name": "SQL", "check_name": "SQL",
"message": "Possible SQL injection", "message": "Possible SQL injection",
"file": "app/models/procedure_presentation.rb", "file": "app/models/procedure_presentation.rb",
"line": 107, "line": 123,
"link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
"code": "((\"self\" == \"self\") ? (dossiers) : (dossiers.includes(\"self\"))).order(\"#{self.class.sanitized_column(\"self\", column)} #{order}\")", "code": "((\"self\" == \"self\") ? (dossiers) : (dossiers.includes(\"self\"))).order(\"#{self.class.sanitized_column(\"self\", column)} #{order}\")",
"render_path": null, "render_path": null,
@ -79,26 +131,6 @@
"confidence": "Weak", "confidence": "Weak",
"note": "`table`, `column` and `order` come from the model, which is validated to prevent injection attacks. Furthermore, `table` and `column` are escaped." "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_type": "SQL Injection",
"warning_code": 0, "warning_code": 0,
@ -106,7 +138,7 @@
"check_name": "SQL", "check_name": "SQL",
"message": "Possible SQL injection", "message": "Possible SQL injection",
"file": "app/models/procedure_presentation.rb", "file": "app/models/procedure_presentation.rb",
"line": 103, "line": 119,
"link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "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}\")", "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, "render_path": null,
@ -117,9 +149,9 @@
}, },
"user_input": "order", "user_input": "order",
"confidence": "Weak", "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"
} }

View file

@ -0,0 +1,44 @@
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
['tous', 'suivis', 'traites', 'a-suivre', 'archives'].each do |statut|
filters[statut] = filters[statut].map do |filter|
table, column, value = filter.values_at('table', 'column', 'value')
if table && (table == 'type_de_champ' || table == 'type_de_champ_private')
type_de_champ = TypeDeChamp.find_by(id: column)
if type_de_champ
column = type_de_champ.stable_id
end
end
[table, column, value]
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)
if type_de_champ
sort['column'] = type_de_champ.stable_id
end
end
filters['migrated'] = true
procedure_presentation.update_columns(filters: filters, sort: sort)
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

View file

@ -3,7 +3,7 @@ describe ProcedurePresentation do
let(:instructeur) { create(:instructeur) } let(:instructeur) { create(:instructeur) }
let(:assign_to) { create(:assign_to, procedure: procedure, instructeur: 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) { 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) { let(:procedure_presentation) {
create(:procedure_presentation, create(:procedure_presentation,
assign_to: assign_to, assign_to: assign_to,
@ -72,10 +72,10 @@ describe ProcedurePresentation do
{ "label" => 'SIRET', "table" => 'etablissement', "column" => 'siret' }, { "label" => 'SIRET', "table" => 'etablissement', "column" => 'siret' },
{ "label" => 'Libellé NAF', "table" => 'etablissement', "column" => 'libelle_naf' }, { "label" => 'Libellé NAF', "table" => 'etablissement', "column" => 'libelle_naf' },
{ "label" => 'Code postal', "table" => 'etablissement', "column" => 'code_postal' }, { "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
end end
describe "#fields_for_select" do describe "#displayed_fields_for_select" do
subject { create(:procedure_presentation, assign_to: assign_to) } subject { create(:procedure_presentation, assign_to: assign_to) }
before do before do
@ -122,13 +122,13 @@ describe ProcedurePresentation do
]) ])
end 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 end
describe '#get_value' do describe '#get_value' do
let(:procedure_presentation) { create(:procedure_presentation, procedure: procedure, assign_to: assign_to, displayed_fields: [{ 'table' => table, 'column' => column }]) } 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 context 'for self table' do
let(:table) { 'self' } let(:table) { 'self' }
@ -217,14 +217,14 @@ describe ProcedurePresentation do
let!(:follow2) { create(:follow, dossier: dossier, instructeur: create(:instructeur, email: 'user2@host')) } let!(:follow2) { create(:follow, dossier: dossier, instructeur: create(:instructeur, email: 'user2@host')) }
it "return emails of followers instructeurs" do 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"] expect(emails_to_display).to eq ["user1@host", "user2@host"]
end end
end end
context 'for type_de_champ table' do context 'for type_de_champ table' do
let(:table) { 'type_de_champ' } 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) } let(:dossier) { create(:dossier, procedure: procedure) }
@ -235,7 +235,7 @@ describe ProcedurePresentation do
context 'for type_de_champ_private table' do context 'for type_de_champ_private table' do
let(:table) { 'type_de_champ_private' } 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) } let(:dossier) { create(:dossier, procedure: procedure) }
@ -325,7 +325,7 @@ describe ProcedurePresentation do
context 'for type_de_champ table' do context 'for type_de_champ table' do
let(:table) { 'type_de_champ' } 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(:order) { 'desc' } # Asc works the same, no extra test required
let(:beurre_dossier) { create(:dossier, procedure: procedure) } let(:beurre_dossier) { create(:dossier, procedure: procedure) }
@ -341,7 +341,7 @@ describe ProcedurePresentation do
context 'for type_de_champ_private table' do context 'for type_de_champ_private table' do
let(:table) { 'type_de_champ_private' } 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(:order) { 'asc' } # Desc works the same, no extra test required
let(:biere_dossier) { create(:dossier, procedure: procedure) } let(:biere_dossier) { create(:dossier, procedure: procedure) }
@ -496,7 +496,7 @@ describe ProcedurePresentation do
end end
context 'for type_de_champ table' do 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(:kept_dossier) { create(:dossier, procedure: procedure) }
let(:discarded_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 context 'with multiple search values' do
let(:filter) 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 end
@ -533,7 +533,7 @@ describe ProcedurePresentation do
end end
context 'with yes_no type_de_champ' do 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) } let(:procedure) { create(:procedure, :with_yes_no) }
before do before do
@ -546,7 +546,7 @@ describe ProcedurePresentation do
end end
context 'for type_de_champ_private table' do 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(:kept_dossier) { create(:dossier, procedure: procedure) }
let(:discarded_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 context 'with multiple search values' do
let(:filter) 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 end
@ -762,7 +762,7 @@ describe ProcedurePresentation do
context 'for type de champ' do context 'for type de champ' do
let(:table) { 'type_de_champ' } 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 it 'preloads the champs relation' do
# Ideally, we would only preload the champs for the matching column # 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 context 'for type de champ private' do
let(:table) { 'type_de_champ_private' } 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 it 'preloads the champs relation' do
# Ideally, we would only preload the champs for the matching column # Ideally, we would only preload the champs for the matching column