move validations concern to filtered_column
Co-authored-by: mfo <mfo@users.noreply.github.com>
This commit is contained in:
parent
6b5efbda07
commit
b2754cd26c
9 changed files with 100 additions and 43 deletions
|
@ -12,7 +12,7 @@
|
||||||
- if column_type == :enum
|
- if column_type == :enum
|
||||||
= select_tag :filter, options_for_select(options_for_select_of_column), id: 'value', name: "#{prefix}[filter]", class: 'fr-select', data: { no_autosubmit: true }
|
= select_tag :filter, options_for_select(options_for_select_of_column), id: 'value', name: "#{prefix}[filter]", class: 'fr-select', data: { no_autosubmit: true }
|
||||||
- else
|
- else
|
||||||
%input#value.fr-input{ type: column_type, name: "#{prefix}[filter]", maxlength: ProcedurePresentation::FILTERS_VALUE_MAX_LENGTH, disabled: column.nil? ? true : false, data: { no_autosubmit: true } }
|
%input#value.fr-input{ type: column_type, name: "#{prefix}[filter]", maxlength: FilteredColumn::FILTERS_VALUE_MAX_LENGTH, disabled: column.nil? ? true : false, data: { no_autosubmit: true } }
|
||||||
|
|
||||||
= hidden_field_tag :statut, statut
|
= hidden_field_tag :statut, statut
|
||||||
= submit_tag t('.add_filter'), class: 'fr-btn fr-btn--secondary fr-mt-2w'
|
= submit_tag t('.add_filter'), class: 'fr-btn fr-btn--secondary fr-mt-2w'
|
||||||
|
|
|
@ -150,7 +150,7 @@ module Instructeurs
|
||||||
if !procedure_presentation.update(filter_params)
|
if !procedure_presentation.update(filter_params)
|
||||||
# complicated way to display inner error messages
|
# complicated way to display inner error messages
|
||||||
flash.alert = procedure_presentation.errors
|
flash.alert = procedure_presentation.errors
|
||||||
.flat_map { _1.detail[:value].errors.full_messages }
|
.flat_map { _1.detail[:value].flat_map { |c| c.errors.full_messages } }
|
||||||
end
|
end
|
||||||
|
|
||||||
redirect_back(fallback_location: instructeur_procedure_url(procedure))
|
redirect_back(fallback_location: instructeur_procedure_url(procedure))
|
||||||
|
|
|
@ -1,8 +1,19 @@
|
||||||
# frozen_string_literal: true
|
# frozen_string_literal: true
|
||||||
|
|
||||||
class FilteredColumn
|
class FilteredColumn
|
||||||
|
include ActiveModel::Validations
|
||||||
|
|
||||||
|
FILTERS_VALUE_MAX_LENGTH = 4048
|
||||||
|
# https://www.postgresql.org/docs/current/datatype-numeric.html
|
||||||
|
PG_INTEGER_MAX_VALUE = 2147483647
|
||||||
|
|
||||||
attr_reader :column, :filter
|
attr_reader :column, :filter
|
||||||
|
|
||||||
|
delegate :label, to: :column
|
||||||
|
|
||||||
|
validate :check_filter_max_length
|
||||||
|
validate :check_filter_max_integer
|
||||||
|
|
||||||
def initialize(column:, filter:)
|
def initialize(column:, filter:)
|
||||||
@column = column
|
@column = column
|
||||||
@filter = filter
|
@filter = filter
|
||||||
|
@ -11,4 +22,21 @@ class FilteredColumn
|
||||||
def ==(other)
|
def ==(other)
|
||||||
other&.column == column && other.filter == filter
|
other&.column == column && other.filter == filter
|
||||||
end
|
end
|
||||||
|
|
||||||
|
private
|
||||||
|
|
||||||
|
def check_filter_max_length
|
||||||
|
if @filter.present? && @filter.length.to_i > FILTERS_VALUE_MAX_LENGTH
|
||||||
|
errors.add(
|
||||||
|
:base,
|
||||||
|
"Le filtre #{label} est trop long (maximum: #{FILTERS_VALUE_MAX_LENGTH} caractères)"
|
||||||
|
)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def check_filter_max_integer
|
||||||
|
if @column.column == 'id' && @filter.to_i > PG_INTEGER_MAX_VALUE
|
||||||
|
errors.add(:base, "Le filtre #{label} n'est pas un numéro de dossier possible")
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -8,18 +8,11 @@ class ProcedurePresentation < ApplicationRecord
|
||||||
SLASH = '/'
|
SLASH = '/'
|
||||||
TYPE_DE_CHAMP = 'type_de_champ'
|
TYPE_DE_CHAMP = 'type_de_champ'
|
||||||
|
|
||||||
FILTERS_VALUE_MAX_LENGTH = 4048
|
|
||||||
# https://www.postgresql.org/docs/current/datatype-numeric.html
|
|
||||||
PG_INTEGER_MAX_VALUE = 2147483647
|
|
||||||
|
|
||||||
belongs_to :assign_to, optional: false
|
belongs_to :assign_to, optional: false
|
||||||
has_many :exports, dependent: :destroy
|
has_many :exports, dependent: :destroy
|
||||||
|
|
||||||
delegate :procedure, :instructeur, to: :assign_to
|
delegate :procedure, :instructeur, to: :assign_to
|
||||||
|
|
||||||
validate :check_filters_max_length
|
|
||||||
validate :check_filters_max_integer
|
|
||||||
|
|
||||||
attribute :displayed_columns, :column, array: true
|
attribute :displayed_columns, :column, array: true
|
||||||
|
|
||||||
attribute :sorted_column, :sorted_column
|
attribute :sorted_column, :sorted_column
|
||||||
|
@ -34,6 +27,9 @@ class ProcedurePresentation < ApplicationRecord
|
||||||
attribute :expirant_filters, :filtered_column, array: true
|
attribute :expirant_filters, :filtered_column, array: true
|
||||||
attribute :archives_filters, :filtered_column, array: true
|
attribute :archives_filters, :filtered_column, array: true
|
||||||
|
|
||||||
|
validates_associated :a_suivre_filters, :suivis_filters, :traites_filters,
|
||||||
|
:tous_filters, :supprimes_filters, :expirant_filters, :archives_filters
|
||||||
|
|
||||||
def filters_for(statut)
|
def filters_for(statut)
|
||||||
send(filters_name_for(statut))
|
send(filters_name_for(statut))
|
||||||
end
|
end
|
||||||
|
@ -219,25 +215,6 @@ class ProcedurePresentation < ApplicationRecord
|
||||||
.find_by(stable_id: column)
|
.find_by(stable_id: column)
|
||||||
end
|
end
|
||||||
|
|
||||||
def check_filters_max_length
|
|
||||||
filters.values.flatten.each do |filter|
|
|
||||||
next if !filter.is_a?(Hash)
|
|
||||||
next if filter['value']&.length.to_i <= FILTERS_VALUE_MAX_LENGTH
|
|
||||||
|
|
||||||
errors.add(:base, "Le filtre #{filter['label']} est trop long (maximum: #{FILTERS_VALUE_MAX_LENGTH} caractères)")
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
def check_filters_max_integer
|
|
||||||
filters.values.flatten.each do |filter|
|
|
||||||
next if !filter.is_a?(Hash)
|
|
||||||
next if filter['column'] != 'id'
|
|
||||||
next if filter['value']&.to_i&. < PG_INTEGER_MAX_VALUE
|
|
||||||
|
|
||||||
errors.add(:base, "Le filtre #{filter['label']} n'est pas un numéro de dossier possible")
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
def self.sanitized_column(association, column)
|
def self.sanitized_column(association, column)
|
||||||
table = if association == 'self'
|
table = if association == 'self'
|
||||||
Dossier.table_name
|
Dossier.table_name
|
||||||
|
|
|
@ -2,7 +2,7 @@
|
||||||
|
|
||||||
module Maintenance
|
module Maintenance
|
||||||
# PR: 10774
|
# PR: 10774
|
||||||
# why: postgres does not support integer greater than ProcedurePresentation::PG_INTEGER_MAX_VALUE)
|
# why: postgres does not support integer greater than FilteredColumn::PG_INTEGER_MAX_VALUE)
|
||||||
# it occures when user copypaste the dossier id twice (like missed copy paste,paste)
|
# it occures when user copypaste the dossier id twice (like missed copy paste,paste)
|
||||||
# once this huge integer is saved on procedure presentation, page with this filter can't be loaded
|
# once this huge integer is saved on procedure presentation, page with this filter can't be loaded
|
||||||
# when: run this migration when it appears in your maintenance tasks list, this file fix the data and we added some validations too
|
# when: run this migration when it appears in your maintenance tasks list, this file fix the data and we added some validations too
|
||||||
|
@ -16,7 +16,7 @@ module Maintenance
|
||||||
filters_by_status.reject do |filter|
|
filters_by_status.reject do |filter|
|
||||||
filter.is_a?(Hash) &&
|
filter.is_a?(Hash) &&
|
||||||
filter['column'] == 'id' &&
|
filter['column'] == 'id' &&
|
||||||
(filter['value']&.to_i&. >= ProcedurePresentation::PG_INTEGER_MAX_VALUE)
|
(filter['value']&.to_i&. >= FilteredColumn::PG_INTEGER_MAX_VALUE)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
element.save
|
element.save
|
||||||
|
|
|
@ -906,7 +906,7 @@ describe Instructeurs::ProceduresController, type: :controller do
|
||||||
|
|
||||||
subject do
|
subject do
|
||||||
column = procedure.find_column(label: "Nom")
|
column = procedure.find_column(label: "Nom")
|
||||||
post :add_filter, params: { procedure_id: procedure.id, a_suivre_filters: { id: column.id, filter: "n" * 110 } }
|
post :add_filter, params: { procedure_id: procedure.id, a_suivre_filters: [{ id: column.id, filter: "n" * 4049 }] }
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'should render the error' do
|
it 'should render the error' do
|
||||||
|
|
59
spec/models/filtered_column_spec.rb
Normal file
59
spec/models/filtered_column_spec.rb
Normal file
|
@ -0,0 +1,59 @@
|
||||||
|
# frozen_string_literal: true
|
||||||
|
|
||||||
|
describe FilteredColumn do
|
||||||
|
describe '#check_filters_max_length' do
|
||||||
|
let(:column) { Column.new(procedure_id: 1, table: 'table', column: 'column', label: 'label') }
|
||||||
|
let(:filtered_column) { described_class.new(column:, filter:) }
|
||||||
|
|
||||||
|
before { filtered_column.valid? }
|
||||||
|
|
||||||
|
context 'when the filter is too long' do
|
||||||
|
let(:filter) { 'a' * (FilteredColumn::FILTERS_VALUE_MAX_LENGTH + 1) }
|
||||||
|
|
||||||
|
it 'adds an error' do
|
||||||
|
expect(filtered_column.errors.map(&:message)).to include(/Le filtre label est trop long/)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when then filter is not too long' do
|
||||||
|
let(:filter) { 'a' * FilteredColumn::FILTERS_VALUE_MAX_LENGTH }
|
||||||
|
|
||||||
|
it 'does not add an error' do
|
||||||
|
expect(filtered_column.errors).to be_empty
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when the filter is empty' do
|
||||||
|
let(:filter) { nil }
|
||||||
|
|
||||||
|
it 'does not add an error' do
|
||||||
|
expect(filtered_column.errors).to be_empty
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe '#check_filters_max_integer' do
|
||||||
|
context 'when the target column is an id column' do
|
||||||
|
let(:column) { Column.new(procedure_id: 1, table: 'table', column: 'id', label: 'label') }
|
||||||
|
let(:filtered_column) { described_class.new(column:, filter:) }
|
||||||
|
|
||||||
|
before { filtered_column.valid? }
|
||||||
|
|
||||||
|
context 'when the filter is too high' do
|
||||||
|
let(:filter) { (FilteredColumn::PG_INTEGER_MAX_VALUE + 1).to_s }
|
||||||
|
|
||||||
|
it 'adds an error' do
|
||||||
|
expect(filtered_column.errors.map(&:message)).to include(/Le filtre label n'est pas un numéro de dossier possible/)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when the filter is not too high' do
|
||||||
|
let(:filter) { FilteredColumn::PG_INTEGER_MAX_VALUE.to_s }
|
||||||
|
|
||||||
|
it 'does not add an error' do
|
||||||
|
expect(filtered_column.errors).to be_empty
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
|
@ -48,16 +48,9 @@ describe ProcedurePresentation do
|
||||||
end
|
end
|
||||||
|
|
||||||
context 'of filters' do
|
context 'of filters' do
|
||||||
it do
|
it 'validates the filter_column objects' do
|
||||||
expect(build(:procedure_presentation, filters: { "suivis" => [{ table: "user", column: "reset_password_token", "order" => "asc" }] })).to be_invalid
|
expect(build(:procedure_presentation, "suivis_filters": [{ id: { column_id: "user/email", procedure_id: }, "filter": "not so long filter value" }])).to be_valid
|
||||||
expect(build(:procedure_presentation, filters: { "suivis" => [{ table: "user", column: "email", "value" => "exceedingly long filter value" * 1000 }] })).to be_invalid
|
expect(build(:procedure_presentation, "suivis_filters": [{ id: { column_id: "user/email", procedure_id: }, "filter": "exceedingly long filter value" * 400 }])).to be_invalid
|
||||||
end
|
|
||||||
|
|
||||||
describe 'check_filters_max_integer' do
|
|
||||||
it do
|
|
||||||
expect(build(:procedure_presentation, filters: { "suivis" => [{ table: "self", column: "id", "value" => ProcedurePresentation::PG_INTEGER_MAX_VALUE.to_s }] })).to be_invalid
|
|
||||||
expect(build(:procedure_presentation, filters: { "suivis" => [{ table: "self", column: "id", "value" => (ProcedurePresentation::PG_INTEGER_MAX_VALUE - 1).to_s }] })).to be_valid
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -14,14 +14,14 @@ module Maintenance
|
||||||
before { element.update_column(:filters, filters) }
|
before { element.update_column(:filters, filters) }
|
||||||
|
|
||||||
context 'when filter is valid' do
|
context 'when filter is valid' do
|
||||||
let(:filters) { { "suivis" => [{ 'table' => "self", 'column' => "id", "value" => (ProcedurePresentation::PG_INTEGER_MAX_VALUE - 1).to_s }] } }
|
let(:filters) { { "suivis" => [{ 'table' => "self", 'column' => "id", "value" => (FilteredColumn::PG_INTEGER_MAX_VALUE - 1).to_s }] } }
|
||||||
it 'keeps it filters' do
|
it 'keeps it filters' do
|
||||||
expect { subject }.not_to change { element.reload.filters }
|
expect { subject }.not_to change { element.reload.filters }
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
context 'when filter is invalid, drop it' do
|
context 'when filter is invalid, drop it' do
|
||||||
let(:filters) { { "suivis" => [{ 'table' => "self", 'column' => "id", "value" => (ProcedurePresentation::PG_INTEGER_MAX_VALUE).to_s }] } }
|
let(:filters) { { "suivis" => [{ 'table' => "self", 'column' => "id", "value" => (FilteredColumn::PG_INTEGER_MAX_VALUE).to_s }] } }
|
||||||
it 'drop invalid filters' do
|
it 'drop invalid filters' do
|
||||||
expect { subject }.to change { element.reload.filters }.to({ "suivis" => [] })
|
expect { subject }.to change { element.reload.filters }.to({ "suivis" => [] })
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in a new issue