From 471f6799c82b8d9fad2a1cfb30d8a14ffe8b5873 Mon Sep 17 00:00:00 2001 From: gregoirenovel Date: Wed, 19 Sep 2018 10:15:42 +0200 Subject: [PATCH 1/8] Refactor Siret tests --- spec/models/siret_spec.rb | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/spec/models/siret_spec.rb b/spec/models/siret_spec.rb index c5d197660..c5dcc2f8f 100644 --- a/spec/models/siret_spec.rb +++ b/spec/models/siret_spec.rb @@ -1,26 +1,41 @@ require 'spec_helper' describe Siret, type: :model do - let(:valid_siret) { '41816609600051' } - let(:invalid_siret) { '111111111' } + subject { Siret.new(siret: siret) } context 'with no siret provided' do - it { is_expected.to validate_presence_of(:siret) } + let(:siret) { '' } + + it { is_expected.to be_invalid } end - context 'init with valid siret' do - it { is_expected.to allow_value(valid_siret).for(:siret) } + context 'with a siret that contains letters' do + let(:siret) { 'A1B1C6D9E0F0G1' } + + it { is_expected.to be_invalid } end - context 'init with invalid siret' do - it { is_expected.not_to allow_value(invalid_siret).for(:siret) } + context 'with a siret that is too short' do + let(:siret) { '1234567890' } + + it { is_expected.to be_invalid } end - context 'init with bullshit siret' do - it { is_expected.not_to allow_value('bullshit').for(:siret) } + context 'with a siret that is too long' do + let(:siret) { '12345678901234567890' } + + it { is_expected.to be_invalid } end - context 'init with a siret that is too long' do - it { is_expected.not_to allow_value('9' * 15).for(:siret) } + context 'with a lunh-invalid siret' do + let(:siret) { '41816609600052' } + + it { is_expected.to be_invalid } + end + + context 'with a valid siret' do + let(:siret) { '41816609600051' } + + it { is_expected.to be_valid } end end From 49d872452a00f619fbaac8a92600a2a2da278be3 Mon Sep 17 00:00:00 2001 From: gregoirenovel Date: Tue, 18 Sep 2018 22:14:07 +0200 Subject: [PATCH 2/8] Improve SiretFormatValidator readability --- app/validators/siret_format_validator.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/validators/siret_format_validator.rb b/app/validators/siret_format_validator.rb index c82b01f20..89d41f2d2 100644 --- a/app/validators/siret_format_validator.rb +++ b/app/validators/siret_format_validator.rb @@ -1,8 +1,9 @@ class SiretFormatValidator < ActiveModel::EachValidator - def validate_each(record,attribute,value) + def validate_each(record, attribute, value) if !(value =~ /^\d{14}$/) record.errors.add(attribute, :format) end + if value.nil? || (luhn_checksum(value) % 10 != 0) record.errors.add(attribute, :checksum) end @@ -12,11 +13,13 @@ class SiretFormatValidator < ActiveModel::EachValidator def luhn_checksum(value) accum = 0 + value.reverse.each_char.map(&:to_i).each_with_index do |digit, index| t = index.even? ? digit : digit * 2 t = t - 9 if t >= 10 accum += t end + accum end end From 36950b985bcff485211865c70dcf3c9950bc9e55 Mon Sep 17 00:00:00 2001 From: gregoirenovel Date: Tue, 18 Sep 2018 22:19:07 +0200 Subject: [PATCH 3/8] Refactor SiretFormatValidator#validate_each --- app/validators/siret_format_validator.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/validators/siret_format_validator.rb b/app/validators/siret_format_validator.rb index 89d41f2d2..268b3a1d4 100644 --- a/app/validators/siret_format_validator.rb +++ b/app/validators/siret_format_validator.rb @@ -1,10 +1,10 @@ class SiretFormatValidator < ActiveModel::EachValidator def validate_each(record, attribute, value) - if !(value =~ /^\d{14}$/) + if !value&.match?(/^\d{14}$/) record.errors.add(attribute, :format) end - if value.nil? || (luhn_checksum(value) % 10 != 0) + if value.present? && (luhn_checksum(value) % 10 != 0) record.errors.add(attribute, :checksum) end end From 940dfe422b31683457c1a4d5aed79c2c39a8fd1b Mon Sep 17 00:00:00 2001 From: gregoirenovel Date: Wed, 19 Sep 2018 10:28:07 +0200 Subject: [PATCH 4/8] Further refactor SiretFormatValidator#validate_each --- app/validators/siret_format_validator.rb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/app/validators/siret_format_validator.rb b/app/validators/siret_format_validator.rb index 268b3a1d4..f048757c1 100644 --- a/app/validators/siret_format_validator.rb +++ b/app/validators/siret_format_validator.rb @@ -1,16 +1,24 @@ class SiretFormatValidator < ActiveModel::EachValidator def validate_each(record, attribute, value) - if !value&.match?(/^\d{14}$/) + if !format_is_valid(value) record.errors.add(attribute, :format) end - if value.present? && (luhn_checksum(value) % 10 != 0) + if !luhn_passed(value) record.errors.add(attribute, :checksum) end end private + def format_is_valid(value) + value&.match?(/^\d{14}$/) + end + + def luhn_passed(value) + value.present? && (luhn_checksum(value) % 10 == 0) + end + def luhn_checksum(value) accum = 0 From 4e4b7c617f3a20579e78b89ac8c6596b7562c3c9 Mon Sep 17 00:00:00 2001 From: gregoirenovel Date: Wed, 19 Sep 2018 10:29:43 +0200 Subject: [PATCH 5/8] [Fix #2618] Do not enforce Luhn for La Poste SIRET numbers --- app/validators/siret_format_validator.rb | 9 ++++++++- spec/models/siret_spec.rb | 6 ++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/app/validators/siret_format_validator.rb b/app/validators/siret_format_validator.rb index f048757c1..3d4520b70 100644 --- a/app/validators/siret_format_validator.rb +++ b/app/validators/siret_format_validator.rb @@ -11,12 +11,19 @@ class SiretFormatValidator < ActiveModel::EachValidator private + LA_POSTE_SIREN = '356000000' + def format_is_valid(value) value&.match?(/^\d{14}$/) end def luhn_passed(value) - value.present? && (luhn_checksum(value) % 10 == 0) + # Do not enforce Luhn for La Poste SIRET numbers, the only exception to this rule + value.present? && (siret_is_attached_to_la_poste(value) || (luhn_checksum(value) % 10 == 0)) + end + + def siret_is_attached_to_la_poste(value) + value[0..8] == LA_POSTE_SIREN end def luhn_checksum(value) diff --git a/spec/models/siret_spec.rb b/spec/models/siret_spec.rb index c5dcc2f8f..579c2b916 100644 --- a/spec/models/siret_spec.rb +++ b/spec/models/siret_spec.rb @@ -33,6 +33,12 @@ describe Siret, type: :model do it { is_expected.to be_invalid } end + context 'with a lunh-invalid La Poste siret' do + let(:siret) { '35600000018723' } + + it { is_expected.to be_valid } + end + context 'with a valid siret' do let(:siret) { '41816609600051' } From 9d18f4187cf0895ac974795e92208bcbcbe8863b Mon Sep 17 00:00:00 2001 From: gregoirenovel Date: Tue, 18 Sep 2018 22:27:38 +0200 Subject: [PATCH 6/8] Simplify SiretFormatValidator We can remove nil checks as we never get nil values --- app/validators/siret_format_validator.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/validators/siret_format_validator.rb b/app/validators/siret_format_validator.rb index 3d4520b70..e1f3e3103 100644 --- a/app/validators/siret_format_validator.rb +++ b/app/validators/siret_format_validator.rb @@ -14,12 +14,12 @@ class SiretFormatValidator < ActiveModel::EachValidator LA_POSTE_SIREN = '356000000' def format_is_valid(value) - value&.match?(/^\d{14}$/) + value.match?(/^\d{14}$/) end def luhn_passed(value) # Do not enforce Luhn for La Poste SIRET numbers, the only exception to this rule - value.present? && (siret_is_attached_to_la_poste(value) || (luhn_checksum(value) % 10 == 0)) + siret_is_attached_to_la_poste(value) || (luhn_checksum(value) % 10 == 0) end def siret_is_attached_to_la_poste(value) From a0660d17eb99f1d3e5d51a2b5f9e466aebe726eb Mon Sep 17 00:00:00 2001 From: gregoirenovel Date: Wed, 19 Sep 2018 10:28:55 +0200 Subject: [PATCH 7/8] Refactor SiretFormatValidator#luhn_checksum --- app/validators/siret_format_validator.rb | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/app/validators/siret_format_validator.rb b/app/validators/siret_format_validator.rb index e1f3e3103..9bb9c3c7e 100644 --- a/app/validators/siret_format_validator.rb +++ b/app/validators/siret_format_validator.rb @@ -27,14 +27,9 @@ class SiretFormatValidator < ActiveModel::EachValidator end def luhn_checksum(value) - accum = 0 - - value.reverse.each_char.map(&:to_i).each_with_index do |digit, index| + value.reverse.each_char.map(&:to_i).map.with_index do |digit, index| t = index.even? ? digit : digit * 2 - t = t - 9 if t >= 10 - accum += t - end - - accum + t < 10 ? t : t - 9 + end.sum end end From 9a338ec14016ff90a5a0617de9a159cd25a709c5 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 18 Sep 2018 14:10:15 +0200 Subject: [PATCH 8/8] =?UTF-8?q?Change=20how=20we=20format=20brouillon=20d?= =?UTF-8?q?=C3=A9marche=20label?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/helpers/procedure_helper.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/app/helpers/procedure_helper.rb b/app/helpers/procedure_helper.rb index 1107fc343..7f4325c1b 100644 --- a/app/helpers/procedure_helper.rb +++ b/app/helpers/procedure_helper.rb @@ -10,11 +10,9 @@ module ProcedureHelper end def procedure_libelle(procedure) - parts = [procedure.libelle] - if procedure.brouillon? - parts << '(brouillon)' - end - parts.join(' ') + parts = procedure.brouillon? ? [content_tag(:span, 'démarche non publiée', class: 'badge')] : [] + parts << procedure.libelle + safe_join(parts, ' ') end def procedure_modal_text(procedure, key)