From ebd4d6a067bd40a4aa094053994e1b6810dadb95 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Wed, 2 Mar 2022 11:03:15 +0100 Subject: [PATCH 1/9] update FC/AG images --- .../images/agentconnect-btn-principal-hover.svg | 1 + app/assets/images/agentconnect-btn-principal.svg | 1 + app/assets/images/franceconnect-btn-hover.svg | 1 + app/assets/images/franceconnect-btn.svg | 1 + app/assets/images/login-with-fc-hover.svg | 1 - app/assets/images/login-with-fc.svg | 1 - app/assets/images/logo-agent-connect.png | Bin 3587 -> 0 bytes .../stylesheets/france-connect-agent-login.scss | 8 ++++++-- app/assets/stylesheets/france-connect-login.scss | 4 ++-- 9 files changed, 12 insertions(+), 6 deletions(-) create mode 100644 app/assets/images/agentconnect-btn-principal-hover.svg create mode 100644 app/assets/images/agentconnect-btn-principal.svg create mode 100644 app/assets/images/franceconnect-btn-hover.svg create mode 100644 app/assets/images/franceconnect-btn.svg delete mode 100644 app/assets/images/login-with-fc-hover.svg delete mode 100644 app/assets/images/login-with-fc.svg delete mode 100644 app/assets/images/logo-agent-connect.png diff --git a/app/assets/images/agentconnect-btn-principal-hover.svg b/app/assets/images/agentconnect-btn-principal-hover.svg new file mode 100644 index 000000000..19dbeb74e --- /dev/null +++ b/app/assets/images/agentconnect-btn-principal-hover.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/app/assets/images/agentconnect-btn-principal.svg b/app/assets/images/agentconnect-btn-principal.svg new file mode 100644 index 000000000..d842f6818 --- /dev/null +++ b/app/assets/images/agentconnect-btn-principal.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/app/assets/images/franceconnect-btn-hover.svg b/app/assets/images/franceconnect-btn-hover.svg new file mode 100644 index 000000000..f1a24a139 --- /dev/null +++ b/app/assets/images/franceconnect-btn-hover.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/app/assets/images/franceconnect-btn.svg b/app/assets/images/franceconnect-btn.svg new file mode 100644 index 000000000..d70581b70 --- /dev/null +++ b/app/assets/images/franceconnect-btn.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/app/assets/images/login-with-fc-hover.svg b/app/assets/images/login-with-fc-hover.svg deleted file mode 100644 index 5a2d16909..000000000 --- a/app/assets/images/login-with-fc-hover.svg +++ /dev/null @@ -1 +0,0 @@ - \ No newline at end of file diff --git a/app/assets/images/login-with-fc.svg b/app/assets/images/login-with-fc.svg deleted file mode 100644 index 3a95ff212..000000000 --- a/app/assets/images/login-with-fc.svg +++ /dev/null @@ -1 +0,0 @@ - \ No newline at end of file diff --git a/app/assets/images/logo-agent-connect.png b/app/assets/images/logo-agent-connect.png deleted file mode 100644 index 6ad68703c82105c2ece68128f5aca64c62d7ffcc..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 3587 zcmV+e4*c0K4`Y)9GFO~Z)l>07}`!1FHE|vQ-m;W=F|NsC0E|mHMPNMA`Bmim5GM4)1^8Wt* z|N8v?w%PsU@&2*b{e8Uq_WJ%Wl=`mK{qgwzX0ZGH{{Ft+{YjzvO{4l>uKP5X`n%ly zLZ15J@c#7q{%o}So6P(>oBBVU`nKBr&E@^z@BV1A`*OGY`1}6+{r*#@`l8VM`uqJo zoccDH`qAh8jm7+bzWc4!{dT$g%;f!BtNY93{qXnxM4$SL!~8gz`k>DIOQZUT!u;s- z{!OI%W3T&qyZfcl{Oi*m8{+P=A&*uF_p!%WD{Di># z$m9Lp?f$vj{RL2?fxi4trTW$B{=whkJ0dAj@VCNA0Q{_ytv4OXY~_WH)%^t0Ig)am{I`TB>e)4|l~?>a&5F*&@@=23;e z7hI}Gq55!}%bU2`V35X+uhlMWvl3aTCTFh?X;_CE2d%fT7`P}OGgtq2~Yn61U-d}O2{p96roYfp&sraL&|NZ^HO=7!U zcH}5CIa!;3wC47Xl7n%g=X8Aj@$znav))f#p}pKLSe~E9?C|vZugBs_f4tY@@m*n; zE`G~Nf4s5L@3m)vNQTGvmYU7s^_#@&da>nFn%%|Vuopm~3o)tBdT+%{e8^+!nWOF(2WW z##lc982Td`v#JS_E-$Y)oH7)N}h}Pb!~y{r7_2CMb@@4B1%Zi zTQa7obV@Nt@+6M${_-J#a>Xkm=I=HMocDk41KC|zHL0@20C+Z7iTW|OH2#)A{=AIi zwxZFTK$HMi+i0(%(kaBetW#pR=;xa>*S&K&$8lfRikRHf8jWVtQ_Ho7eij$PU2rti zR9MpVhEhtm5 z@@lD=enRI%A9D{Ah`0I1y$FfvN(?5?`}D@9*!t&g(8%(%U`zp%d-5r*EVthwmKpk6!k8o|g~m8}aJbE-_#@Ow4ZuDK)=7 zeDz)8;&RFEEeDndIUiHOq=XvGUM)Z9qA6v)pScpVkU=yPY;NxKl#>qmL=IiNQSP*{ zNS2ejXew>!;EBc%L$UB%B;V3n>mbTRV@iA~`=0vRpgw-Ip^nJ7q%Ta&Ef33(*Ss}D zu|c`%(su)t&umh|Bn%zN*ZF|XICViFbjkru#AgHxV@awWeC zi-nHK)|^6h%wegZTg(qE$yFsw;!SfeNv+FLiIU8Xh5=IO>-KaCdn#x6nBV`&HIpd;_pPt8Mdjdjbsi=8QF!E3H}A0lhw-by~_t9P2beea4w?=3*D01m=sXlz5%v-qk*{ zDHJAAvb}qJMrl=*BM{ZL*G-x&jODp1Tfut!q%PQ!#~@`|`6 z3poW%gZ*Xz^Q91&Z*bhR*G3NJqaiSFDBI}hB-aomHEgSl7Dl%9Pr zzQkUB?SGBwu+;Pa56rHWaVnU5x?VcEg5$WKJ|7aZ96)uLjp%im0rx!nr;qu*_HQbf zGxi+CK4#7zhs10FP%|aVw9gl#TgDi9W6*fA6tb#tF)u%;y;83}2bo{yIPUEop)d^? z1*T+|HWUfJeLz#D!$B90#Dy9Xn8;~kG-c4@Z zR+FiOsPx>;wuLO5kz+Dh5^D@CJk2pR!*4S6QnDUR!q(ZjNU&|i>}4H3D|2>Ci%K@z zin7Nii82RkDmw*7Yjw1%Cb1x>Doo4^@ZsUxkNx|oI_9H7T<8gP%vtdAX<$2F8Ulo) zrgh}(5X!a@B>^6mJePeBCTWxpC=O-#`9gNhDxw&%Xq- z>v(dpK*>F*ikV`AkH!FIm`YSInCgD2hBvFc4CXP0>8>p?K%X-wrUU+%m@KLW6n(W3 zNQ~`?!zfQNKvTt5Gy6J#JuwufIE(b_78+bMGdn6 zEUeLtc|^RK1wm?1PrM#2R%@{z?F=(k=-HOQfJ%i;;o#aZAgbx77>wo;ZNLUnCVSv5 zw_1;&hK=b$BEJ5l$L5vV!yny!=q`24OC`)l)G*fq8wLjaKsiwds+|kqDW%nN)nSG? zhsdbM#z5QB0IPC2+9-C4G>SpGU|ts~lXAEmDe3I-lT1Oj{~Xe4@1CJb=@Btk-1&kE zrqv@C>mjlWgQ$gQ1js95vQfgMIJRMw+EyTTaZy%*1$bG1@JVblVIXC40DeK&01aU9 zlVJiM{P*5}sT=cxE=GCjeNYQWmjKZ|pU=0RHKfG3+$h!z0e_BHi=HjFwb4-MIrn+nhB5SVtLFNGXSlPMFZ z4xaC>hYQ`*DCzNT< zCn_`pn+*`BpaA9^58xhVab)?+wi9K0fc0*$Sh_kU4Fa}2H`eD;B4G1_^;cO{IOm&z zJ2K`A14lx^g`{h@yqu<%Ppnx6NFnzDEKBKu*~30aEYXOIg)>^lTa#jK(r4*qul0Gt z@>$=^ik28dVy3jQG2h%xlv2jj7g?36!a1!D+;}=aNV!uOlibVSl%5w_PT+vmn{`g2 z;v&o!4ir_wBVi2RjdF2002ov JPDHLkV1iTadZYjV diff --git a/app/assets/stylesheets/france-connect-agent-login.scss b/app/assets/stylesheets/france-connect-agent-login.scss index 1d1139385..8b9bf4c6e 100644 --- a/app/assets/stylesheets/france-connect-agent-login.scss +++ b/app/assets/stylesheets/france-connect-agent-login.scss @@ -2,10 +2,14 @@ @import "constants"; .france-connect-agent-login-button { - background-image: image-url("logo-agent-connect.png"); + background-image: image-url("agentconnect-btn-principal.svg"), image-url("agentconnect-btn-principal-hover.svg"); display: block; height: 60px; - width: 230px; + width: 206px; margin: 20px auto; font-size: 0; + + &:hover { + background-image: image-url("agentconnect-btn-principal-hover.svg"); + } } diff --git a/app/assets/stylesheets/france-connect-login.scss b/app/assets/stylesheets/france-connect-login.scss index 20c66f404..494dd2e57 100644 --- a/app/assets/stylesheets/france-connect-login.scss +++ b/app/assets/stylesheets/france-connect-login.scss @@ -19,14 +19,14 @@ width: 230px; margin: auto; margin-bottom: 8px; - background-image: image-url("login-with-fc.svg"), image-url("login-with-fc-hover.svg"); + background-image: image-url("franceconnect-btn.svg"), image-url("franceconnect-btn-hover.svg"); background-repeat: no-repeat; background-size: cover; cursor: pointer; font-size: 0; &:hover { - background-image: image-url("login-with-fc-hover.svg"); + background-image: image-url("franceconnect-btn-hover.svg"); } } From 4c432b2ce8206f1e4dbad5b657199e21bff430ad Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Wed, 2 Mar 2022 14:26:06 +0100 Subject: [PATCH 2/9] wording --- config/locales/views/agent_connect/agent/en.yml | 2 +- config/locales/views/agent_connect/agent/fr.yml | 4 ++-- config/locales/views/shared/en.yml | 2 +- config/locales/views/shared/fr.yml | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/config/locales/views/agent_connect/agent/en.yml b/config/locales/views/agent_connect/agent/en.yml index a7b89e3ae..c58228f81 100644 --- a/config/locales/views/agent_connect/agent/en.yml +++ b/config/locales/views/agent_connect/agent/en.yml @@ -11,7 +11,7 @@ en: The ministries and operators that can currently benefit from it are :

    -
  • the Ministry of Ecological Transition
    • +
  • the Insee
you_are_a_citizen: You are an individual ? citizen_page: Go to our dedicated page diff --git a/config/locales/views/agent_connect/agent/fr.yml b/config/locales/views/agent_connect/agent/fr.yml index a81ac0c1e..67d8a5c35 100644 --- a/config/locales/views/agent_connect/agent/fr.yml +++ b/config/locales/views/agent_connect/agent/fr.yml @@ -8,10 +8,10 @@ fr:

AgentConnect est en cours de déploiement.
- Les ministères et opérateurs qui peuvent l'utiliser à ce jour sont : + Les ministères et opérateurs qui peuvent lʼutiliser à ce jour sont :

    -
  • le ministère de la Transition écologique
    • +
  • lʼInsee
you_are_a_citizen: Vous êtes un particulier ? citizen_page: Accéder à notre page dédiée diff --git a/config/locales/views/shared/en.yml b/config/locales/views/shared/en.yml index a6788d90b..5770dd40f 100644 --- a/config/locales/views/shared/en.yml +++ b/config/locales/views/shared/en.yml @@ -11,7 +11,7 @@ en: en_attente: "Waiting for response" france_connect_login: title: "With FranceConnect" - description: "France connect is a solution proposed by the government to secure and simplify the connection to web services." + description: "FranceConnect is a solution proposed by the government to secure and simplify the connection to web services." login_button: "Sign in with FranceConnect" help_link: What is FranceConnect? separator: or diff --git a/config/locales/views/shared/fr.yml b/config/locales/views/shared/fr.yml index 5ee3f4ecd..8325949e8 100644 --- a/config/locales/views/shared/fr.yml +++ b/config/locales/views/shared/fr.yml @@ -11,7 +11,7 @@ fr: en_attente: "En attente de réponse" france_connect_login: title: 'Avec FranceConnect' - description: "France connect est la solution proposée par l’État pour sécuriser et simplifier la connexion aux services en ligne." + description: "FranceConnect est la solution proposée par l’État pour sécuriser et simplifier la connexion aux services en ligne." login_button: "S’identifier avec FranceConnect" help_link: "Qu’est-ce que FranceConnect ?" separator: 'ou' From a01e523f088109bf9d6c018c7766525049a986ae Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Wed, 2 Mar 2022 14:26:24 +0100 Subject: [PATCH 3/9] more compression --- app/assets/stylesheets/auth.scss | 14 +++++++++++--- app/assets/stylesheets/france-connect-login.scss | 4 ++-- app/views/users/sessions/new.html.haml | 3 +-- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/app/assets/stylesheets/auth.scss b/app/assets/stylesheets/auth.scss index 410771461..835b8c86f 100644 --- a/app/assets/stylesheets/auth.scss +++ b/app/assets/stylesheets/auth.scss @@ -18,7 +18,15 @@ } .column { - padding-top: 2 * $default-spacer; + padding-top: $default-spacer; + } + + h1 { + margin-bottom: $default-spacer; + } + + .form label { + margin-bottom: $default-spacer / 2; } } @@ -26,7 +34,7 @@ .auth-options { display: flex; justify-content: space-between; - margin-bottom: 4 * $default-spacer; + margin-bottom: 2 * $default-spacer; } .remember-me { @@ -60,7 +68,7 @@ .sign-in-form .form { input[type="email"] { - margin-bottom: $default-padding; + margin-bottom: $default-spacer; } input[type="password"] { diff --git a/app/assets/stylesheets/france-connect-login.scss b/app/assets/stylesheets/france-connect-login.scss index 494dd2e57..72c5edf87 100644 --- a/app/assets/stylesheets/france-connect-login.scss +++ b/app/assets/stylesheets/france-connect-login.scss @@ -36,8 +36,8 @@ align-items: center; color: $black; text-transform: uppercase; - padding-bottom: $default-padding; - padding-top: $default-padding; + padding-bottom: $default-spacer; + padding-top: $default-spacer; &::before, &::after { diff --git a/app/views/users/sessions/new.html.haml b/app/views/users/sessions/new.html.haml index 42811868a..bfebfda9f 100644 --- a/app/views/users/sessions/new.html.haml +++ b/app/views/users/sessions/new.html.haml @@ -27,6 +27,5 @@ .france-connect-login-separator = t('views.shared.france_connect_login.separator') .center - %h2.important-header= t('views.users.sessions.new.state_civil_servant') - %br + %h2.important-header.mb-1= t('views.users.sessions.new.state_civil_servant') = link_to t('views.users.sessions.new.connect_with_agent_connect'), agent_connect_path, class: "button expend secondary" From 02661b5ec77b05dca586a25ae391bec3a2d914fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Bernier?= <43164960+benoit-bernier@users.noreply.github.com> Date: Thu, 3 Mar 2022 16:46:02 +0100 Subject: [PATCH 4/9] Typo correction --- .../dossier_submitted_messages/_informations.html.haml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/administrateurs/dossier_submitted_messages/_informations.html.haml b/app/views/administrateurs/dossier_submitted_messages/_informations.html.haml index f5ae76910..fa06d1c84 100644 --- a/app/views/administrateurs/dossier_submitted_messages/_informations.html.haml +++ b/app/views/administrateurs/dossier_submitted_messages/_informations.html.haml @@ -1,3 +1,3 @@ = f.label :message_on_submit_by_usager do - Message affiché après l'envoie du dossier -= f.text_area :message_on_submit_by_usager, placeholder: "Merci votre dossier sera traité dans les plus bref delais" + Message affiché après l'envoi du dossier += f.text_area :message_on_submit_by_usager, placeholder: "Merci, votre dossier sera traité dans les plus bref delais" From 8ee6c5fe1b3f73d3b361ba4df9c8b27f4410cdff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Bernier?= <43164960+benoit-bernier@users.noreply.github.com> Date: Thu, 3 Mar 2022 16:48:26 +0100 Subject: [PATCH 5/9] Typo correction --- app/views/administrateurs/procedures/show.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/administrateurs/procedures/show.html.haml b/app/views/administrateurs/procedures/show.html.haml index 05b43bcbe..6dd302899 100644 --- a/app/views/administrateurs/procedures/show.html.haml +++ b/app/views/administrateurs/procedures/show.html.haml @@ -246,6 +246,6 @@ %p.card-admin-status-todo À configurer %div %p.card-admin-title Fin de dépot - %p.card-admin-subtitle Orienter l'usager suite à l'envoie de son dossier + %p.card-admin-subtitle Orienter l'usager suite à l'envoi de son dossier %p.button Modifier From a645c5781d01f22c020bccfaa2090a40f5b703a5 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Wed, 2 Mar 2022 12:09:35 +0100 Subject: [PATCH 6/9] models: delete AdministrateursProcedure when destroying Procedure --- app/models/procedure.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/procedure.rb b/app/models/procedure.rb index d3be28e47..a7cc0e1a9 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -176,7 +176,7 @@ class Procedure < ApplicationRecord end end - has_many :administrateurs_procedures + has_many :administrateurs_procedures, dependent: :delete_all has_many :administrateurs, through: :administrateurs_procedures, after_remove: -> (procedure, _admin) { procedure.validate! } has_many :groupe_instructeurs, dependent: :destroy has_many :instructeurs, through: :groupe_instructeurs From 3a16235868fb113905786435852bc0911252bee7 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 8 Mar 2022 13:47:27 +0000 Subject: [PATCH 7/9] db: Add a `delete_orphans` db helper --- app/lib/database/migration_helpers.rb | 30 ++- spec/lib/database/migration_helpers_spec.rb | 238 ++++++++++++++------ 2 files changed, 200 insertions(+), 68 deletions(-) diff --git a/app/lib/database/migration_helpers.rb b/app/lib/database/migration_helpers.rb index 07061d888..e0a224d83 100644 --- a/app/lib/database/migration_helpers.rb +++ b/app/lib/database/migration_helpers.rb @@ -1,4 +1,4 @@ -# Some of this file is lifted from Gitlab's `lib/gitlab/database/migration_helpers.rb`` +# Some of this file is lifted from Gitlab's `lib/gitlab/database/migration_helpers.rb` # Copyright (c) 2011-present GitLab B.V. # @@ -103,6 +103,34 @@ module Database::MigrationHelpers end end + # Delete records from `from_table` having a reference to a missing record in `to_table`. + # This is useful to rectify data before adding a proper foreign_key. + # + # Example: + # + # delete_orphans :appointments, :physicians + # + def delete_orphans(from_table, to_table) + say_with_time "Deleting records from #{from_table} where the associated #{to_table.to_s.singularize} no longer exists" do + from_table = Arel::Table.new(from_table) + to_table = Arel::Table.new(to_table) + foreign_key_column = foreign_key_column_for(to_table.name) + + # Select the ids of orphan records + arel_select = from_table + .join(to_table, Arel::Nodes::OuterJoin).on(to_table[:id].eq(from_table[foreign_key_column])) + .where(to_table[:id].eq(nil)) + .project(from_table[foreign_key_column]) + missing_record_ids = query_values(arel_select.to_sql) + + # Delete the records having ids referencing missing data + arel_delete = Arel::DeleteManager.new() + .from(from_table) + .where(from_table[foreign_key_column].in(missing_record_ids.uniq)) + exec_delete(arel_delete.to_sql) + end + end + private def statement_timeout_disabled? diff --git a/spec/lib/database/migration_helpers_spec.rb b/spec/lib/database/migration_helpers_spec.rb index 4fab02099..1ca896cfb 100644 --- a/spec/lib/database/migration_helpers_spec.rb +++ b/spec/lib/database/migration_helpers_spec.rb @@ -1,90 +1,194 @@ describe Database::MigrationHelpers do - class TestLabel < ApplicationRecord - end + describe 'handling duplicates' do + class TestLabel < ApplicationRecord + end - before(:all) do - ActiveRecord::Migration.suppress_messages do - ActiveRecord::Migration.create_table "test_labels", force: true do |t| - t.string :label - t.integer :user_id + before(:all) do + ActiveRecord::Migration.suppress_messages do + ActiveRecord::Migration.create_table "test_labels", force: true do |t| + t.string :label + t.integer :user_id + end + ActiveRecord::Migration.create_table "test_labels", force: true do |t| + t.string :label + t.integer :user_id + end end end - end - before(:each) do - # User 1 labels - TestLabel.create({ id: 1, label: 'Important', user_id: 1 }) - TestLabel.create({ id: 2, label: 'Urgent', user_id: 1 }) - TestLabel.create({ id: 3, label: 'Done', user_id: 1 }) - TestLabel.create({ id: 4, label: 'Bug', user_id: 1 }) + before(:each) do + # User 1 labels + TestLabel.create({ id: 1, label: 'Important', user_id: 1 }) + TestLabel.create({ id: 2, label: 'Urgent', user_id: 1 }) + TestLabel.create({ id: 3, label: 'Done', user_id: 1 }) + TestLabel.create({ id: 4, label: 'Bug', user_id: 1 }) - # User 2 labels - TestLabel.create({ id: 5, label: 'Important', user_id: 2 }) - TestLabel.create({ id: 6, label: 'Critical', user_id: 2 }) + # User 2 labels + TestLabel.create({ id: 5, label: 'Important', user_id: 2 }) + TestLabel.create({ id: 6, label: 'Critical', user_id: 2 }) - # Duplicates - TestLabel.create({ id: 7, label: 'Urgent', user_id: 1 }) - TestLabel.create({ id: 8, label: 'Important', user_id: 2 }) - end - - after(:all) do - ActiveRecord::Migration.suppress_messages do - ActiveRecord::Migration.drop_table :test_labels, force: true + # Duplicates + TestLabel.create({ id: 7, label: 'Urgent', user_id: 1 }) + TestLabel.create({ id: 8, label: 'Important', user_id: 2 }) end - end - let(:model) { ActiveRecord::Migration.new.extend(Database::MigrationHelpers) } + after(:all) do + ActiveRecord::Migration.suppress_messages do + ActiveRecord::Migration.drop_table :test_labels, force: true + end + end - describe '.find_duplicates' do - context 'using a single column for uniqueness' do + let(:model) { ActiveRecord::Migration.new.extend(Database::MigrationHelpers) } + + describe '.find_duplicates' do + context 'using a single column for uniqueness' do + subject do + model.find_duplicates(:test_labels, [:label]) + end + + it 'finds duplicates' do + expect(subject.length).to eq 2 + end + + it 'finds three labels with "Important"' do + expect(subject).to include [1, 5, 8] + end + + it 'finds two labels with "Urgent"' do + expect(subject).to include [2, 7] + end + end + + context 'using multiple columns for uniqueness' do + subject do + model.find_duplicates(:test_labels, [:label, :user_id]) + end + + it 'finds duplicates' do + expect(subject.length).to eq 2 + end + + it 'finds two labels with "Important" for user 2' do + expect(subject).to include [5, 8] + end + + it 'finds two labels with "Urgent" for user 1' do + expect(subject).to include [2, 7] + end + end + end + + describe '.delete_duplicates' do subject do - model.find_duplicates(:test_labels, [:label]) + model.delete_duplicates(:test_labels, [:label]) end - it 'finds duplicates' do - expect(subject.length).to eq 2 - end - - it 'finds three labels with "Important"' do - expect(subject).to include [1, 5, 8] - end - - it 'finds two labels with "Urgent"' do - expect(subject).to include [2, 7] - end - end - - context 'using multiple columns for uniqueness' do - subject do - model.find_duplicates(:test_labels, [:label, :user_id]) - end - - it 'finds duplicates' do - expect(subject.length).to eq 2 - end - - it 'finds two labels with "Important" for user 2' do - expect(subject).to include [5, 8] - end - - it 'finds two labels with "Urgent" for user 1' do - expect(subject).to include [2, 7] + it 'keeps the first item, and delete the others' do + expect { subject }.to change(TestLabel, :count).by(-3) + expect(TestLabel.where(label: 'Critical').count).to eq(1) + expect(TestLabel.where(label: 'Important').count).to eq(1) + expect(TestLabel.where(label: 'Urgent').count).to eq(1) + expect(TestLabel.where(label: 'Bug').count).to eq(1) + expect(TestLabel.where(label: 'Done').count).to eq(1) end end end - describe '.delete_duplicates' do + describe '.delete_orphans' do + class TestPhysician < ApplicationRecord; end + + class TestPatient < ApplicationRecord; end + + class TestAppointment < ApplicationRecord; end + + before(:all) do + ActiveRecord::Migration.suppress_messages do + ActiveRecord::Migration.create_table "test_physicians", force: true do |t| + t.string :name + end + ActiveRecord::Migration.create_table "test_patients", force: true do |t| + t.string :name + end + ActiveRecord::Migration.create_table "test_appointments", id: false, force: true do |t| + t.integer :test_physician_id + t.integer :test_patient_id + t.datetime :datetime + end + end + end + + after(:all) do + ActiveRecord::Migration.suppress_messages do + ActiveRecord::Migration.drop_table :test_physicians, force: true + ActiveRecord::Migration.drop_table :test_patients, force: true + ActiveRecord::Migration.drop_table :test_appointments, force: true + end + end + + let(:model) { ActiveRecord::Migration.new.extend(Database::MigrationHelpers) } + subject do - model.delete_duplicates(:test_labels, [:label]) + model.delete_orphans(:test_appointments, :test_patients) end - it 'keeps the first item, and delete the others' do - expect { subject }.to change(TestLabel, :count).by(-3) - expect(TestLabel.where(label: 'Critical').count).to eq(1) - expect(TestLabel.where(label: 'Important').count).to eq(1) - expect(TestLabel.where(label: 'Urgent').count).to eq(1) - expect(TestLabel.where(label: 'Bug').count).to eq(1) - expect(TestLabel.where(label: 'Done').count).to eq(1) + context 'when there are orphan records' do + before(:each) do + phy1 = TestPhysician.create({ name: 'Ibn Sina' }) + phy2 = TestPhysician.create({ name: 'Louis Pasteur' }) + pa1 = TestPatient.create({ name: 'Chams ad-Dawla' }) + pa2 = TestPatient.create({ name: 'Joseph Meister' }) + ap1 = TestAppointment.create({ test_physician_id: phy1.id, test_patient_id: pa1.id, datetime: 2.months.ago }) + ap2 = TestAppointment.create({ test_physician_id: phy1.id, test_patient_id: pa1.id, datetime: 1.month.ago }) + ap3 = TestAppointment.create({ test_physician_id: phy2.id, test_patient_id: pa2.id, datetime: 2.days.ago }) + ap4 = TestAppointment.create({ test_physician_id: phy1.id, test_patient_id: pa2.id, datetime: 1.day.ago }) + ap5 = TestAppointment.create({ test_physician_id: phy1.id, test_patient_id: pa1.id, datetime: Time.zone.today }) + + # Appointments missing the associated patient + ap6 = TestAppointment.create({ test_physician_id: phy1.id, test_patient_id: 9999, datetime: 3.months.ago }) + ap7 = TestAppointment.create({ test_physician_id: phy1.id, test_patient_id: 8888, datetime: 2.months.ago }) + ap8 = TestAppointment.create({ test_physician_id: phy2.id, test_patient_id: 8888, datetime: 1.month.ago }) + + # Appointments missing the associated physician + ap9 = TestAppointment.create({ test_physician_id: 7777, test_patient_id: pa1.id, datetime: 3.months.ago }) + end + + it 'deletes orphaned records on the specified key' do + expect { subject }.to change { TestAppointment.count }.by(-3) + + # rubocop:disable Rails/WhereEquals + appointments_with_missing_patients = TestAppointment + .joins('LEFT OUTER JOIN test_patients ON test_patients.id = test_appointments.test_patient_id') + .where('test_patients.id IS NULL') + # rubocop:enable Rails/WhereEquals + expect(appointments_with_missing_patients.count).to eq(0) + end + + it 'keeps orphaned records on another key' do + subject + + # rubocop:disable Rails/WhereEquals + appointments_with_missing_physicians = TestAppointment + .joins('LEFT OUTER JOIN test_physicians ON test_physicians.id = test_appointments.test_physician_id') + .where('test_physicians.id IS NULL') + # rubocop:enable Rails/WhereEquals + expect(appointments_with_missing_physicians.count).not_to eq(0) + end + + it 'keeps valid associated records' do + expect { subject }.not_to change { [TestPhysician.count, TestPatient.count] } + end + end + + context 'when there are no orphaned records' do + before(:each) do + phy1 = TestPhysician.create({ name: 'Ibn Sina' }) + pa1 = TestPatient.create({ name: 'Chams ad-Dawla' }) + ap1 = TestAppointment.create({ test_physician_id: phy1.id, test_patient_id: pa1.id, datetime: 2.months.ago }) + end + + it 'doesn’t remove any records' do + expect { subject }.not_to change { [TestPhysician.count, TestPatient.count, TestAppointment.count] } + end end end From a4108c778747c751f947eab13218187b3f9633c5 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 8 Mar 2022 14:47:06 +0100 Subject: [PATCH 8/9] db: backport `delete_orphans` in recent migrations Recent migrations used ActiveRecord when removing invalid data. This may break if the ActiveRecord model later changes. To ensure these migrations will run correctly, even when the code will have changed, let's use an SQL-based helper instead of ActiveRecord. --- ...r_foreign_key_to_administrateurs_procedure.rb | 11 ++++------- ...reign_keys_to_administrateurs_instructeurs.rb | 16 +++++----------- 2 files changed, 9 insertions(+), 18 deletions(-) diff --git a/db/migrate/20220301160753_add_administrateur_foreign_key_to_administrateurs_procedure.rb b/db/migrate/20220301160753_add_administrateur_foreign_key_to_administrateurs_procedure.rb index d8d0bbb0a..944940df1 100644 --- a/db/migrate/20220301160753_add_administrateur_foreign_key_to_administrateurs_procedure.rb +++ b/db/migrate/20220301160753_add_administrateur_foreign_key_to_administrateurs_procedure.rb @@ -1,12 +1,9 @@ class AddAdministrateurForeignKeyToAdministrateursProcedure < ActiveRecord::Migration[6.1] - def up - # Sanity check - say_with_time 'Removing AdministrateursProcedures where the associated Administrateur no longer exists ' do - deleted_administrateur_ids = AdministrateursProcedure.where.missing(:administrateur).pluck(:administrateur_id) - AdministrateursProcedure.where(administrateur_id: deleted_administrateur_ids).delete_all - end + include Database::MigrationHelpers - add_foreign_key :administrateurs_procedures, :administrateurs + def up + delete_orphans :administrateurs_procedures, :administrateurs_procedures + add_foreign_key :administrateurs_procedures, :administrateurs_procedures end def down diff --git a/db/migrate/20220302101337_add_foreign_keys_to_administrateurs_instructeurs.rb b/db/migrate/20220302101337_add_foreign_keys_to_administrateurs_instructeurs.rb index fc524565f..ae863654d 100644 --- a/db/migrate/20220302101337_add_foreign_keys_to_administrateurs_instructeurs.rb +++ b/db/migrate/20220302101337_add_foreign_keys_to_administrateurs_instructeurs.rb @@ -1,17 +1,11 @@ class AddForeignKeysToAdministrateursInstructeurs < ActiveRecord::Migration[6.1] + include Database::MigrationHelpers + def up - # Sanity check - say_with_time 'Removing AdministrateursInstructeur where the associated Administrateur no longer exists ' do - deleted_administrateurs_ids = AdministrateursInstructeur.where.missing(:administrateur).pluck(:administrateur_id) - AdministrateursInstructeur.where(administrateur_id: deleted_administrateurs_ids).delete_all - end - - say_with_time 'Removing AdministrateursInstructeur where the associated Instructeur no longer exists ' do - deleted_instructeurs_ids = AdministrateursInstructeur.where.missing(:instructeur).pluck(:instructeur_id) - AdministrateursInstructeur.where(instructeur_id: deleted_instructeurs_ids).delete_all - end - + delete_orphans :administrateurs_instructeurs, :administrateurs add_foreign_key :administrateurs_instructeurs, :administrateurs + + delete_orphans :administrateurs_instructeurs, :instructeurs add_foreign_key :administrateurs_instructeurs, :instructeurs end From 27edc6676dc16827511705938a1a9ccaf998520b Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 8 Mar 2022 08:21:22 +0000 Subject: [PATCH 9/9] db: add foreign key contraint to AdministrateursProcedure In production there are around 20 000 AdministrateursProcedure records that reference a deleted Procedure. --- ...edure_foreign_key_to_administrateurs_procedure.rb | 12 ++++++++++++ db/schema.rb | 3 ++- 2 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20220308110720_add_procedure_foreign_key_to_administrateurs_procedure.rb diff --git a/db/migrate/20220308110720_add_procedure_foreign_key_to_administrateurs_procedure.rb b/db/migrate/20220308110720_add_procedure_foreign_key_to_administrateurs_procedure.rb new file mode 100644 index 000000000..2b6fc5304 --- /dev/null +++ b/db/migrate/20220308110720_add_procedure_foreign_key_to_administrateurs_procedure.rb @@ -0,0 +1,12 @@ +class AddProcedureForeignKeyToAdministrateursProcedure < ActiveRecord::Migration[6.1] + include Database::MigrationHelpers + + def up + delete_orphans :administrateurs_procedures, :procedures + add_foreign_key :administrateurs_procedures, :procedures + end + + def down + remove_foreign_key :administrateurs_procedures, :procedures + end +end diff --git a/db/schema.rb b/db/schema.rb index 16c13c8d7..3e2d4a1f7 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2022_03_02_101337) do +ActiveRecord::Schema.define(version: 2022_03_08_110720) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -855,6 +855,7 @@ ActiveRecord::Schema.define(version: 2022_03_02_101337) do add_foreign_key "administrateurs_instructeurs", "administrateurs" add_foreign_key "administrateurs_instructeurs", "instructeurs" add_foreign_key "administrateurs_procedures", "administrateurs" + add_foreign_key "administrateurs_procedures", "procedures" add_foreign_key "archives_groupe_instructeurs", "archives" add_foreign_key "archives_groupe_instructeurs", "groupe_instructeurs" add_foreign_key "assign_tos", "groupe_instructeurs"