From 7d189575afebeb1e6d1b5f96a7ceafa5b318231a Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 25 Nov 2021 13:28:40 +0300 Subject: [PATCH 01/13] fix(commune): do not display foreign departement when selecting commune --- app/javascript/components/ComboCommunesSearch.jsx | 1 + .../components/ComboDepartementsSearch.jsx | 15 +++++++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/app/javascript/components/ComboCommunesSearch.jsx b/app/javascript/components/ComboCommunesSearch.jsx index fa9aca1ab..16a2815fe 100644 --- a/app/javascript/components/ComboCommunesSearch.jsx +++ b/app/javascript/components/ComboCommunesSearch.jsx @@ -91,6 +91,7 @@ function ComboCommunesSearch(params) { inputId={!departementCode ? inputId : null} aria-describedby={departementDescribedBy} placeholder={placeholderDepartement} + addForeignDepartement={false} required={params.mandatory} onChange={(value, result) => { setDepartementCode(result?.code); diff --git a/app/javascript/components/ComboDepartementsSearch.jsx b/app/javascript/components/ComboDepartementsSearch.jsx index 745a29803..3f4c3350a 100644 --- a/app/javascript/components/ComboDepartementsSearch.jsx +++ b/app/javascript/components/ComboDepartementsSearch.jsx @@ -1,4 +1,5 @@ import React from 'react'; +import PropTypes from 'prop-types'; import { QueryClientProvider } from 'react-query'; import { matchSorter } from 'match-sorter'; @@ -16,14 +17,19 @@ function expandResultsWithForeignDepartement(term, results) { ]; } -export function ComboDepartementsSearch(params) { +export function ComboDepartementsSearch({ + addForeignDepartement = true, + ...params +}) { return ( [code, `${code} - ${nom}`]} - transformResults={expandResultsWithForeignDepartement} + transformResults={ + addForeignDepartement ? expandResultsWithForeignDepartement : undefined + } /> ); } @@ -39,4 +45,9 @@ function ComboDepartementsSearchDefault(params) { ); } +ComboDepartementsSearch.propTypes = { + ...ComboSearch.propTypes, + addForeignDepartement: PropTypes.bool +}; + export default ComboDepartementsSearchDefault; From ff073f8884af40a43c292f633a4954e927e83bb9 Mon Sep 17 00:00:00 2001 From: Martin Date: Wed, 17 Nov 2021 16:21:55 +0100 Subject: [PATCH 02/13] Add confirmation by email when merging DC/FC accounts feat(fci.confirmation_code): add confirmation code to france_connect_informations feat(user_mailer.france_connect_confirmation_code): add confirmation by email mail method/preview/spec, pointing to merge_mail_with_existing_account (reuse existing method) feat(mail_merge): mail merge feat(merge.cannot_use_france_connect): same behaviour as callback clean(fci.confirmation_code): use same token for mail validation as merge feat(resend_france_connect/particulier/merge_confirmation): resend email with link. also enhance some trads, cleanup halfy finished refacto clean(tech): finalize story by plugging merge_with_new_account to email validation fix(deadspec): was removed fix(spec): broken after last refactoring lint(rubocop): space before parenthesis lint(haml-lint): yoohoooo space before = fix(lint): scss now :D Update app/assets/stylesheets/buttons.scss cleanup feat(france_connect): re-add confirm by email, with an option for confirmation by email instead of only confirmation by email fixup! Add confirmation by email when merging DC/FC accounts fix(lint): haml_spec failure --- app/assets/stylesheets/forms.scss | 6 ++ .../france_connect/particulier_controller.rb | 30 +++++++-- app/mailers/user_mailer.rb | 7 +++ .../_password_confirmation.html.haml | 6 +- .../particulier/merge.html.haml | 16 +++-- .../france_connect_merge_confirmation.haml | 20 ++++++ config/routes.rb | 2 + .../particulier_controller_spec.rb | 61 ++++++++++++++++++- spec/mailers/previews/user_mailer_preview.rb | 4 ++ spec/mailers/user_mailer_spec.rb | 10 +++ vendor/assets/stylesheets/franceconnect.scss | 1 + 11 files changed, 152 insertions(+), 11 deletions(-) create mode 100644 app/views/user_mailer/france_connect_merge_confirmation.haml diff --git a/app/assets/stylesheets/forms.scss b/app/assets/stylesheets/forms.scss index c815ac74d..a5c34814e 100644 --- a/app/assets/stylesheets/forms.scss +++ b/app/assets/stylesheets/forms.scss @@ -556,3 +556,9 @@ [data-reach-combobox-popover] { z-index: 20; } + +.fconnect-form { + input[type=password] { + margin-bottom: 16px; + } +} diff --git a/app/controllers/france_connect/particulier_controller.rb b/app/controllers/france_connect/particulier_controller.rb index 655ab2cf2..ba20e2943 100644 --- a/app/controllers/france_connect/particulier_controller.rb +++ b/app/controllers/france_connect/particulier_controller.rb @@ -1,6 +1,6 @@ class FranceConnect::ParticulierController < ApplicationController before_action :redirect_to_login_if_fc_aborted, only: [:callback] - before_action :securely_retrieve_fci, only: [:merge, :merge_with_existing_account, :merge_with_new_account] + before_action :securely_retrieve_fci, only: [:merge, :merge_with_existing_account, :merge_with_new_account, :mail_merge_with_existing_account, :resend_and_renew_merge_confirmation] def login if FranceConnectService.enabled? @@ -20,7 +20,8 @@ class FranceConnect::ParticulierController < ApplicationController fci.associate_user!(fci.email_france_connect) connect_france_connect_particulier(fci.user) else - redirect_to france_connect_particulier_merge_path(fci.create_merge_token!) + merge_token = fci.create_merge_token! + redirect_to france_connect_particulier_merge_path(merge_token) end else user = fci.user @@ -28,7 +29,7 @@ class FranceConnect::ParticulierController < ApplicationController if user.can_france_connect? fci.update(updated_at: Time.zone.now) connect_france_connect_particulier(user) - else + else # same behaviour as redirect nicely with message when instructeur/administrateur fci.destroy redirect_to new_user_session_path, alert: t('errors.messages.france_connect.forbidden_html', reset_link: new_user_password_path) end @@ -64,6 +65,20 @@ class FranceConnect::ParticulierController < ApplicationController end end + def mail_merge_with_existing_account + user = User.find_by(email: @fci.email_france_connect.downcase) + if user.can_france_connect? + @fci.update(user: user) + @fci.delete_merge_token! + + flash.notice = "Les comptes FranceConnect et #{APPLICATION_NAME} sont à présent fusionnés" + connect_france_connect_particulier(user) + else # same behaviour as redirect nicely with message when instructeur/administrateur + @fci.destroy + redirect_to new_user_session_path, alert: t('errors.messages.france_connect.forbidden_html', reset_link: new_user_password_path) + end + end + def merge_with_new_account user = User.find_by(email: sanitized_email_params) @@ -79,13 +94,20 @@ class FranceConnect::ParticulierController < ApplicationController end end + def resend_and_renew_merge_confirmation + merge_token = @fci.create_merge_token! + UserMailer.france_connect_merge_confirmation(@fci.email_france_connect, merge_token).deliver_later + redirect_to france_connect_particulier_merge_path(merge_token), + notice: "Nous venons de vous envoyer le mail de confirmation, veuillez cliquer sur le lien contenu dans ce mail pour fusionner vos comptes" + end + private def securely_retrieve_fci @fci = FranceConnectInformation.find_by(merge_token: merge_token_params) if @fci.nil? || !@fci.valid_for_merge? - flash.alert = 'Votre compte FranceConnect a expiré, veuillez recommencer.' + flash.alert = "Le délai pour fusionner les comptes FranceConnect et #{APPLICATION_NAME} est expirée. Veuillez recommencer la procédure pour vous fusionner les comptes." respond_to do |format| format.html { redirect_to root_path } diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index e5b143566..72db2f384 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -20,6 +20,13 @@ class UserMailer < ApplicationMailer mail(to: requested_email, subject: @subject) end + def france_connect_merge_confirmation(email, merge_token) + @merge_token = merge_token + @subject = "Veuillez confirmer la fusion de compte" + + mail(to: email, subject: @subject) + end + def invite_instructeur(user, reset_password_token) @reset_password_token = reset_password_token @user = user diff --git a/app/views/france_connect/particulier/_password_confirmation.html.haml b/app/views/france_connect/particulier/_password_confirmation.html.haml index 0bc209dff..ce3cabbf5 100644 --- a/app/views/france_connect/particulier/_password_confirmation.html.haml +++ b/app/views/france_connect/particulier/_password_confirmation.html.haml @@ -3,10 +3,14 @@ %br entrez votre mot de passe pour fusionner les comptes -= form_tag france_connect_particulier_merge_with_existing_account_path, remote: true, class: 'mt-2 form' do += form_tag france_connect_particulier_merge_with_existing_account_path, remote: true, class: 'mt-2 form fconnect-form' do = hidden_field_tag :merge_token, merge_token = hidden_field_tag :email, email = label_tag :password, 'Mot de passe (8 caractères minimum)' = password_field_tag :password, nil, autocomplete: 'current-password', id: 'password-for-another-account' + .mb-2 + Mot de passe oublié ? + = link_to france_connect_particulier_resend_and_renew_merge_confirmation_path(merge_token: merge_token), method: :post do + Confirmer mon compte par mail = button_tag 'revenir en arrière', type: 'button', class: 'button secondary', onclick: 'DS.showNewAccount(event);' = submit_tag 'Fusionner les comptes', class: 'button primary' diff --git a/app/views/france_connect/particulier/merge.html.haml b/app/views/france_connect/particulier/merge.html.haml index c8f4ae656..3c10ffd73 100644 --- a/app/views/france_connect/particulier/merge.html.haml +++ b/app/views/france_connect/particulier/merge.html.haml @@ -23,22 +23,30 @@ Non .fusion.hidden - %p Pour les fusionner, entrez votre mot de passe + %p Pour fusionner ces comptes, veuillez cliquer sur le lien présent dans le mail que nous venons de vous envoyer. - = form_tag france_connect_particulier_merge_with_existing_account_path, remote: true, class: 'mt-2 form' do + = form_tag france_connect_particulier_merge_with_existing_account_path, remote: true, class: 'mt-2 form fconnect-form' do = hidden_field_tag :merge_token, @fci.merge_token = hidden_field_tag :email, @fci.email_france_connect + = label_tag :password, 'Mot de passe (8 caractères minimum)' - = password_field_tag :password, nil, autocomplete: 'current-password' + = password_field_tag :password, nil, autocomplete: 'current-password', class: 'mb-1' + .mb-2 + Mot de passe oublié ? + = link_to france_connect_particulier_resend_and_renew_merge_confirmation_path(merge_token: @fci.merge_token), method: :post do + Confirmer mon compte par mail + = submit_tag 'Fusionner les comptes', class: 'button primary' + .new-account.hidden %p Donnez-nous alors le mail que #{APPLICATION_NAME} utilisera pour vous contacter = form_tag france_connect_particulier_merge_with_new_account_path, remote: true, class: 'mt-2 form' do = hidden_field_tag :merge_token, @fci.merge_token = label_tag :email, 'Email (nom@site.com)' - = email_field_tag :email + = email_field_tag :email, "", required: true = submit_tag 'Utiliser ce mail', class: 'button primary' + .new-account-password-confirmation.hidden diff --git a/app/views/user_mailer/france_connect_merge_confirmation.haml b/app/views/user_mailer/france_connect_merge_confirmation.haml new file mode 100644 index 000000000..387d72749 --- /dev/null +++ b/app/views/user_mailer/france_connect_merge_confirmation.haml @@ -0,0 +1,20 @@ +- content_for(:title, @subject) + +%p + Bonjour, + +%p + Pour confirmer la fusion de votre compte, veuillez cliquer sur le lien suivant : += round_button 'Je confirme', france_connect_particulier_mail_merge_with_existing_account_url(merge_token: @merge_token), :primary + +%p + Vous pouvez aussi visiter ce lien : #{link_to france_connect_particulier_mail_merge_with_existing_account_url(merge_token: @merge_token), france_connect_particulier_mail_merge_with_existing_account_url(merge_token: @merge_token)} + +%p Ce lien est valide #{distance_of_time_in_words(FranceConnectInformation::MERGE_VALIDITY)}. + +%p + Si vous n’êtes pas à l’origine de cette demande, vous pouvez ignorer ce message. Et si vous avez besoin d’assistance, n’hésitez pas à nous contacter à + = succeed '.' do + = mail_to CONTACT_EMAIL + += render partial: "layouts/mailers/signature" diff --git a/config/routes.rb b/config/routes.rb index 3b4812185..51117675b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -125,6 +125,8 @@ Rails.application.routes.draw do get 'particulier' => 'particulier#login' get 'particulier/callback' => 'particulier#callback' get 'particulier/merge/:merge_token' => 'particulier#merge', as: :particulier_merge + get 'particulier/mail_merge_with_existing_account/:merge_token' => 'particulier#mail_merge_with_existing_account', as: :particulier_mail_merge_with_existing_account + post 'particulier/resend_and_renew_merge_confirmation' => 'particulier#resend_and_renew_merge_confirmation', as: :particulier_resend_and_renew_merge_confirmation post 'particulier/merge_with_existing_account' => 'particulier#merge_with_existing_account' post 'particulier/merge_with_new_account' => 'particulier#merge_with_new_account' end diff --git a/spec/controllers/france_connect/particulier_controller_spec.rb b/spec/controllers/france_connect/particulier_controller_spec.rb index 92f342022..fb93d508c 100644 --- a/spec/controllers/france_connect/particulier_controller_spec.rb +++ b/spec/controllers/france_connect/particulier_controller_spec.rb @@ -150,7 +150,7 @@ describe FranceConnect::ParticulierController, type: :controller do else expect(subject).to redirect_to root_path end - expect(flash.alert).to eq('Votre compte FranceConnect a expiré, veuillez recommencer.') + expect(flash.alert).to eq('Le délai pour fusionner les comptes FranceConnect et demarches-simplifiees.fr est expirée. Veuillez recommencer la procédure pour vous fusionner les comptes.') end end end @@ -173,7 +173,7 @@ describe FranceConnect::ParticulierController, type: :controller do it do expect(subject).to redirect_to root_path - expect(flash.alert).to eq('Votre compte FranceConnect a expiré, veuillez recommencer.') + expect(flash.alert).to eq("Le délai pour fusionner les comptes FranceConnect et demarches-simplifiees.fr est expirée. Veuillez recommencer la procédure pour vous fusionner les comptes.") end end end @@ -241,6 +241,54 @@ describe FranceConnect::ParticulierController, type: :controller do end end + describe '#mail_merge_with_existing_account' do + let(:fci) { FranceConnectInformation.create!(user_info) } + let!(:merge_token) { fci.create_merge_token! } + + context 'when the merge_token is ok and the user is found' do + subject { post :mail_merge_with_existing_account, params: { merge_token: fci.merge_token } } + + let!(:user) { create(:user, email: email, password: 'abcdefgh') } + + it 'merges the account, signs in, and delete the merge token' do + subject + fci.reload + + expect(fci.user).to eq(user) + expect(fci.merge_token).to be_nil + expect(controller.current_user).to eq(user) + end + + context 'but the targeted user is an instructeur' do + let!(:user) { create(:instructeur, email: email, password: 'abcdefgh').user } + + it 'redirects to the new session' do + subject + expect(FranceConnectInformation.exists?(fci.id)).to be_falsey + expect(controller.current_user).to be_nil + expect(response).to redirect_to(new_user_session_path) + expect(flash[:alert]).to eq(I18n.t('errors.messages.france_connect.forbidden_html', reset_link: new_user_password_path)) + end + end + end + + context 'when the merge_token is not ok' do + subject { post :mail_merge_with_existing_account, params: { merge_token: 'ko' } } + + let!(:user) { create(:user, email: email) } + + it 'increases the failed attempts counter' do + subject + fci.reload + + expect(fci.user).to be_nil + expect(fci.merge_token).not_to be_nil + expect(controller.current_user).to be_nil + expect(response).to redirect_to(root_path) + end + end + end + describe '#merge_with_new_account' do let(:fci) { FranceConnectInformation.create!(user_info) } let(:merge_token) { fci.create_merge_token! } @@ -280,4 +328,13 @@ describe FranceConnect::ParticulierController, type: :controller do end end end + + describe '#resend_and_renew_merge_confirmation' do + let(:fci) { FranceConnectInformation.create!(user_info) } + let(:merge_token) { fci.create_merge_token! } + it 'renew token' do + expect { post :resend_and_renew_merge_confirmation, params: { merge_token: merge_token } }.to change { fci.reload.merge_token } + expect(response).to redirect_to(france_connect_particulier_merge_path(fci.reload.merge_token)) + end + end end diff --git a/spec/mailers/previews/user_mailer_preview.rb b/spec/mailers/previews/user_mailer_preview.rb index 436ac435f..fc9782218 100644 --- a/spec/mailers/previews/user_mailer_preview.rb +++ b/spec/mailers/previews/user_mailer_preview.rb @@ -12,6 +12,10 @@ class UserMailerPreview < ActionMailer::Preview UserMailer.ask_for_merge(user, 'dircab@territoires.gouv.fr') end + def france_connect_merge_confirmation + UserMailer.france_connect_merge_confirmation('new.exemple.fr', '123456') + end + private def user diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index 88f4fa2dc..6ea3d81ae 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -25,4 +25,14 @@ RSpec.describe UserMailer, type: :mailer do it { expect(subject.to).to eq([requested_email]) } it { expect(subject.body).to include(requested_email) } end + + describe '.france_connect_merge_confirmation' do + let(:email) { 'new.exemple.fr' } + let(:code) { '123456' } + + subject { described_class.france_connect_merge_confirmation(email, code) } + + it { expect(subject.to).to eq([email]) } + it { expect(subject.body).to include(france_connect_particulier_mail_merge_with_existing_account_url(merge_token: code)) } + end end diff --git a/vendor/assets/stylesheets/franceconnect.scss b/vendor/assets/stylesheets/franceconnect.scss index 5a74703d4..bddbfbd26 100644 --- a/vendor/assets/stylesheets/franceconnect.scss +++ b/vendor/assets/stylesheets/franceconnect.scss @@ -140,3 +140,4 @@ height: 500px; margin: 60px auto 0 auto; } + From 21894d0a0a683332aa2a2f6629113d4aed84072f Mon Sep 17 00:00:00 2001 From: Martin Date: Tue, 23 Nov 2021 10:44:38 +0100 Subject: [PATCH 03/13] feat(france_connect/particulier#callback): in case the FC email exists as an DC account which is an instructor or and administrator, returns early to new_session_path so he can connect with this existing account --- .../france_connect/particulier_controller.rb | 3 +++ .../france_connect/particulier_controller_spec.rb | 10 ++++++++++ 2 files changed, 13 insertions(+) diff --git a/app/controllers/france_connect/particulier_controller.rb b/app/controllers/france_connect/particulier_controller.rb index ba20e2943..c42954df0 100644 --- a/app/controllers/france_connect/particulier_controller.rb +++ b/app/controllers/france_connect/particulier_controller.rb @@ -19,6 +19,9 @@ class FranceConnect::ParticulierController < ApplicationController if preexisting_unlinked_user.nil? fci.associate_user!(fci.email_france_connect) connect_france_connect_particulier(fci.user) + elsif !preexisting_unlinked_user.can_france_connect? + fci.destroy + redirect_to new_user_session_path, alert: t('errors.messages.france_connect.forbidden_html', reset_link: new_user_password_path) else merge_token = fci.create_merge_token! redirect_to france_connect_particulier_merge_path(merge_token) diff --git a/spec/controllers/france_connect/particulier_controller_spec.rb b/spec/controllers/france_connect/particulier_controller_spec.rb index fb93d508c..b2a7c0bc9 100644 --- a/spec/controllers/france_connect/particulier_controller_spec.rb +++ b/spec/controllers/france_connect/particulier_controller_spec.rb @@ -106,6 +106,16 @@ describe FranceConnect::ParticulierController, type: :controller do expect(response).to redirect_to(france_connect_particulier_merge_path(fci.reload.merge_token)) end end + context 'and an instructeur with the same email exists' do + let!(:preexisting_user) { create(:instructeur, email: email) } + + it 'redirects to the merge process' do + expect { subject }.not_to change { User.count } + + expect(response).to redirect_to(new_user_session_path) + expect(flash[:alert]).to eq(I18n.t('errors.messages.france_connect.forbidden_html', reset_link: new_user_password_path)) + end + end end end From 8a315a4ac1d0ba7409c5813e86ee86ffbb0e1fc8 Mon Sep 17 00:00:00 2001 From: Martin Date: Tue, 23 Nov 2021 10:54:59 +0100 Subject: [PATCH 04/13] feat(UserMailer.france_connect_merge_confirmation): in addition to distance_of_time_in_words, add exact expiration date --- app/mailers/user_mailer.rb | 3 ++- app/views/user_mailer/france_connect_merge_confirmation.haml | 2 +- spec/mailers/previews/user_mailer_preview.rb | 2 +- spec/mailers/user_mailer_spec.rb | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 72db2f384..a1ee3219a 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -20,8 +20,9 @@ class UserMailer < ApplicationMailer mail(to: requested_email, subject: @subject) end - def france_connect_merge_confirmation(email, merge_token) + def france_connect_merge_confirmation(email, merge_token, merge_token_created_at) @merge_token = merge_token + @merge_token_created_at = merge_token_created_at @subject = "Veuillez confirmer la fusion de compte" mail(to: email, subject: @subject) diff --git a/app/views/user_mailer/france_connect_merge_confirmation.haml b/app/views/user_mailer/france_connect_merge_confirmation.haml index 387d72749..134592e52 100644 --- a/app/views/user_mailer/france_connect_merge_confirmation.haml +++ b/app/views/user_mailer/france_connect_merge_confirmation.haml @@ -10,7 +10,7 @@ %p Vous pouvez aussi visiter ce lien : #{link_to france_connect_particulier_mail_merge_with_existing_account_url(merge_token: @merge_token), france_connect_particulier_mail_merge_with_existing_account_url(merge_token: @merge_token)} -%p Ce lien est valide #{distance_of_time_in_words(FranceConnectInformation::MERGE_VALIDITY)}. +%p Ce lien est valide #{distance_of_time_in_words(FranceConnectInformation::MERGE_VALIDITY)}, jusqu'à #{I18n.l(@merge_token_created_at, format: "%d-%m-%Y à %H:%M (%Z)")} %p Si vous n’êtes pas à l’origine de cette demande, vous pouvez ignorer ce message. Et si vous avez besoin d’assistance, n’hésitez pas à nous contacter à diff --git a/spec/mailers/previews/user_mailer_preview.rb b/spec/mailers/previews/user_mailer_preview.rb index fc9782218..c64abdeb1 100644 --- a/spec/mailers/previews/user_mailer_preview.rb +++ b/spec/mailers/previews/user_mailer_preview.rb @@ -13,7 +13,7 @@ class UserMailerPreview < ActionMailer::Preview end def france_connect_merge_confirmation - UserMailer.france_connect_merge_confirmation('new.exemple.fr', '123456') + UserMailer.france_connect_merge_confirmation('new.exemple.fr', '123456', 15.minutes.from_now) end private diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index 6ea3d81ae..73a012dad 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -30,7 +30,7 @@ RSpec.describe UserMailer, type: :mailer do let(:email) { 'new.exemple.fr' } let(:code) { '123456' } - subject { described_class.france_connect_merge_confirmation(email, code) } + subject { described_class.france_connect_merge_confirmation(email, code, 15.minutes.from_now) } it { expect(subject.to).to eq([email]) } it { expect(subject.body).to include(france_connect_particulier_mail_merge_with_existing_account_url(merge_token: code)) } From febe890d28f4d82d612cac225e94af61f947ecc7 Mon Sep 17 00:00:00 2001 From: Martin Date: Tue, 23 Nov 2021 11:00:06 +0100 Subject: [PATCH 05/13] fixup! Add confirmation by email when merging DC/FC accounts --- .../france_connect/particulier/_password_confirmation.html.haml | 2 +- app/views/france_connect/particulier/merge.html.haml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/france_connect/particulier/_password_confirmation.html.haml b/app/views/france_connect/particulier/_password_confirmation.html.haml index ce3cabbf5..860ca6895 100644 --- a/app/views/france_connect/particulier/_password_confirmation.html.haml +++ b/app/views/france_connect/particulier/_password_confirmation.html.haml @@ -11,6 +11,6 @@ .mb-2 Mot de passe oublié ? = link_to france_connect_particulier_resend_and_renew_merge_confirmation_path(merge_token: merge_token), method: :post do - Confirmer mon compte par mail + Confirmer mon compte par email = button_tag 'revenir en arrière', type: 'button', class: 'button secondary', onclick: 'DS.showNewAccount(event);' = submit_tag 'Fusionner les comptes', class: 'button primary' diff --git a/app/views/france_connect/particulier/merge.html.haml b/app/views/france_connect/particulier/merge.html.haml index 3c10ffd73..10fdd3d04 100644 --- a/app/views/france_connect/particulier/merge.html.haml +++ b/app/views/france_connect/particulier/merge.html.haml @@ -34,7 +34,7 @@ .mb-2 Mot de passe oublié ? = link_to france_connect_particulier_resend_and_renew_merge_confirmation_path(merge_token: @fci.merge_token), method: :post do - Confirmer mon compte par mail + Confirmer mon compte par email = submit_tag 'Fusionner les comptes', class: 'button primary' From fdf0f18fda03694e25b6acc8c9453265d1e2bd56 Mon Sep 17 00:00:00 2001 From: Martin Date: Tue, 23 Nov 2021 13:30:07 +0100 Subject: [PATCH 06/13] fix(i18n): wrap text under i18n.t i18n(france_connect/*): replace wording with i18n fix(lint): i18n key issue secu(views/france_connect/particulier/merge.html.haml): sanitize france_connect_email just in case fix(brakeman): sanitize FCI.email_france_connect when used with html_safe via an I18n.t, also add exception to brakeman --- .../france_connect/particulier_controller.rb | 16 ++++---- .../_password_confirmation.html.haml | 14 +++---- .../particulier/merge.html.haml | 32 +++++++--------- .../france_connect_merge_confirmation.haml | 2 +- config/brakeman.ignore | 37 +++++++++++++++++-- config/locales/en.yml | 20 ++++++++++ config/locales/fr.yml | 20 ++++++++++ .../particulier_controller_spec.rb | 1 + 8 files changed, 104 insertions(+), 38 deletions(-) diff --git a/app/controllers/france_connect/particulier_controller.rb b/app/controllers/france_connect/particulier_controller.rb index c42954df0..13f41b6f1 100644 --- a/app/controllers/france_connect/particulier_controller.rb +++ b/app/controllers/france_connect/particulier_controller.rb @@ -51,18 +51,18 @@ class FranceConnect::ParticulierController < ApplicationController if user.present? && user.valid_for_authentication? { user.valid_password?(password_params) } if !user.can_france_connect? - flash.alert = "#{user.email} ne peut utiliser FranceConnect" + flash.alert = t('errors.messages.france_connect.forbidden_html', reset_link: new_user_password_path) render js: ajax_redirect(root_path) else @fci.update(user: user) @fci.delete_merge_token! - flash.notice = "Les comptes FranceConnect et #{APPLICATION_NAME} sont à présent fusionnés" + flash.notice = t('france_connect.particulier.flash.connection_done', application_name: APPLICATION_NAME) connect_france_connect_particulier(user) end else - flash.alert = 'Mauvais mot de passe' + flash.alert = t('france_connect.particulier.flash.invalid_password') render js: helpers.render_flash end @@ -74,7 +74,7 @@ class FranceConnect::ParticulierController < ApplicationController @fci.update(user: user) @fci.delete_merge_token! - flash.notice = "Les comptes FranceConnect et #{APPLICATION_NAME} sont à présent fusionnés" + flash.notice = t('france_connect.particulier.flash.connection_done', application_name: APPLICATION_NAME) connect_france_connect_particulier(user) else # same behaviour as redirect nicely with message when instructeur/administrateur @fci.destroy @@ -89,7 +89,7 @@ class FranceConnect::ParticulierController < ApplicationController @fci.associate_user!(sanitized_email_params) @fci.delete_merge_token! - flash.notice = "Les comptes FranceConnect et #{APPLICATION_NAME} sont à présent fusionnés" + flash.notice = t('france_connect.particulier.flash.connection_done', application_name: APPLICATION_NAME) connect_france_connect_particulier(@fci.user) else @email = sanitized_email_params @@ -99,9 +99,9 @@ class FranceConnect::ParticulierController < ApplicationController def resend_and_renew_merge_confirmation merge_token = @fci.create_merge_token! - UserMailer.france_connect_merge_confirmation(@fci.email_france_connect, merge_token).deliver_later + UserMailer.france_connect_merge_confirmation(@fci.email_france_connect, merge_token, @fci.merge_token_created_at).deliver_later redirect_to france_connect_particulier_merge_path(merge_token), - notice: "Nous venons de vous envoyer le mail de confirmation, veuillez cliquer sur le lien contenu dans ce mail pour fusionner vos comptes" + notice: t('france_connect.particulier.flash.confirmation_mail_sent') end private @@ -110,7 +110,7 @@ class FranceConnect::ParticulierController < ApplicationController @fci = FranceConnectInformation.find_by(merge_token: merge_token_params) if @fci.nil? || !@fci.valid_for_merge? - flash.alert = "Le délai pour fusionner les comptes FranceConnect et #{APPLICATION_NAME} est expirée. Veuillez recommencer la procédure pour vous fusionner les comptes." + flash.alert = t('france_connect.particulier.flash.merger_token_expired', application_name: APPLICATION_NAME) respond_to do |format| format.html { redirect_to root_path } diff --git a/app/views/france_connect/particulier/_password_confirmation.html.haml b/app/views/france_connect/particulier/_password_confirmation.html.haml index 860ca6895..eda3ea1bc 100644 --- a/app/views/france_connect/particulier/_password_confirmation.html.haml +++ b/app/views/france_connect/particulier/_password_confirmation.html.haml @@ -1,16 +1,16 @@ %p - Le compte #{email} existe déjà sur #{APPLICATION_NAME} + = t('.already_exists', email: email, application_name: APPLICATION_NAME) %br - entrez votre mot de passe pour fusionner les comptes + = t('.fill_in_password') = form_tag france_connect_particulier_merge_with_existing_account_path, remote: true, class: 'mt-2 form fconnect-form' do = hidden_field_tag :merge_token, merge_token = hidden_field_tag :email, email - = label_tag :password, 'Mot de passe (8 caractères minimum)' + = label_tag :password, t('views.registrations.new.password_label', min_length: 8) = password_field_tag :password, nil, autocomplete: 'current-password', id: 'password-for-another-account' .mb-2 - Mot de passe oublié ? + = t('views.users.sessions.new.reset_password') = link_to france_connect_particulier_resend_and_renew_merge_confirmation_path(merge_token: merge_token), method: :post do - Confirmer mon compte par email - = button_tag 'revenir en arrière', type: 'button', class: 'button secondary', onclick: 'DS.showNewAccount(event);' - = submit_tag 'Fusionner les comptes', class: 'button primary' + = t('france_connect.particulier.merge.link_confirm_by_email') + = button_tag t('.back'), type: 'button', class: 'button secondary', onclick: 'DS.showNewAccount(event);' + = submit_tag t('france_connect.particulier.merge.button_merge'), class: 'button primary' diff --git a/app/views/france_connect/particulier/merge.html.haml b/app/views/france_connect/particulier/merge.html.haml index 10fdd3d04..ce8fedec8 100644 --- a/app/views/france_connect/particulier/merge.html.haml +++ b/app/views/france_connect/particulier/merge.html.haml @@ -1,52 +1,46 @@ = content_for :title, "Fusion des comptes FC et #{APPLICATION_NAME}" .container - %h1.page-title Fusion des comptes FranceConnect et #{APPLICATION_NAME} + %h1.page-title= t('.title', application_name: APPLICATION_NAME) - %p - Bonjour, - %br - %br - Votre compte FranceConnect utilise #{@fci.email_france_connect} comme email de contact. - %br - Or il existe un compte sur #{APPLICATION_NAME} avec cet email. + %p= t('.subtitle', email: sanitize(@fci.email_france_connect), application_name: APPLICATION_NAME).html_safe .form.mt-2 - %label Ce compte #{@fci.email_france_connect} vous appartient-il ? + %label= t('.label_select_merge_flow', email: @fci.email_france_connect) %fieldset.radios %label{ onclick: "DS.showFusion(event);" } = radio_button_tag :value, true, false, autocomplete: "off", id: 'it-is-mine' - Oui + = t('utils.yes') %label{ onclick: "DS.showNewAccount(event);" } = radio_button_tag :value, false, false, autocomplete: "off", id: 'it-is-not-mine' - Non + = t('utils.no') .fusion.hidden - %p Pour fusionner ces comptes, veuillez cliquer sur le lien présent dans le mail que nous venons de vous envoyer. + %p= t('.title_fill_in_password') = form_tag france_connect_particulier_merge_with_existing_account_path, remote: true, class: 'mt-2 form fconnect-form' do = hidden_field_tag :merge_token, @fci.merge_token = hidden_field_tag :email, @fci.email_france_connect - = label_tag :password, 'Mot de passe (8 caractères minimum)' + = label_tag :password, t('views.registrations.new.password_label', min_length: 8) = password_field_tag :password, nil, autocomplete: 'current-password', class: 'mb-1' .mb-2 - Mot de passe oublié ? + = t('views.users.sessions.new.reset_password') = link_to france_connect_particulier_resend_and_renew_merge_confirmation_path(merge_token: @fci.merge_token), method: :post do - Confirmer mon compte par email + = t('.link_confirm_by_email') - = submit_tag 'Fusionner les comptes', class: 'button primary' + = submit_tag t('.button_merge'), class: 'button primary' .new-account.hidden - %p Donnez-nous alors le mail que #{APPLICATION_NAME} utilisera pour vous contacter + %p= t('.title_fill_in_email', application_name: APPLICATION_NAME) = form_tag france_connect_particulier_merge_with_new_account_path, remote: true, class: 'mt-2 form' do = hidden_field_tag :merge_token, @fci.merge_token - = label_tag :email, 'Email (nom@site.com)' + = label_tag :email, t('views.registrations.new.email_label') = email_field_tag :email, "", required: true - = submit_tag 'Utiliser ce mail', class: 'button primary' + = submit_tag t('.button_use_this_email'), class: 'button primary' .new-account-password-confirmation.hidden diff --git a/app/views/user_mailer/france_connect_merge_confirmation.haml b/app/views/user_mailer/france_connect_merge_confirmation.haml index 134592e52..f7fb0f664 100644 --- a/app/views/user_mailer/france_connect_merge_confirmation.haml +++ b/app/views/user_mailer/france_connect_merge_confirmation.haml @@ -10,7 +10,7 @@ %p Vous pouvez aussi visiter ce lien : #{link_to france_connect_particulier_mail_merge_with_existing_account_url(merge_token: @merge_token), france_connect_particulier_mail_merge_with_existing_account_url(merge_token: @merge_token)} -%p Ce lien est valide #{distance_of_time_in_words(FranceConnectInformation::MERGE_VALIDITY)}, jusqu'à #{I18n.l(@merge_token_created_at, format: "%d-%m-%Y à %H:%M (%Z)")} +%p Ce lien est valide #{distance_of_time_in_words(FranceConnectInformation::MERGE_VALIDITY)}, jusqu'à #{@merge_token_created_at.strftime("%d-%m-%Y à %H:%M (%Z)")} %p Si vous n’êtes pas à l’origine de cette demande, vous pouvez ignorer ce message. Et si vous avez besoin d’assistance, n’hésitez pas à nous contacter à diff --git a/config/brakeman.ignore b/config/brakeman.ignore index bb1c24763..46908df59 100644 --- a/config/brakeman.ignore +++ b/config/brakeman.ignore @@ -1,5 +1,36 @@ { "ignored_warnings": [ + { + "warning_type": "Cross-Site Scripting", + "warning_code": 2, + "fingerprint": "42099f4550a8377f455e830e8ab645cecd5806248481c5c646b4e17548c3cb07", + "check_name": "CrossSiteScripting", + "message": "Unescaped model attribute", + "file": "app/views/france_connect/particulier/merge.html.haml", + "line": 6, + "link": "https://brakemanscanner.org/docs/warning_types/cross_site_scripting", + "code": "t(\".subtitle\", :email => sanitize(FranceConnectInformation.find_by(:merge_token => merge_token_params).email_france_connect), :application_name => (APPLICATION_NAME))", + "render_path": [ + { + "type": "controller", + "class": "FranceConnect::ParticulierController", + "method": "merge", + "line": 48, + "file": "app/controllers/france_connect/particulier_controller.rb", + "rendered": { + "name": "france_connect/particulier/merge", + "file": "app/views/france_connect/particulier/merge.html.haml" + } + } + ], + "location": { + "type": "template", + "template": "france_connect/particulier/merge" + }, + "user_input": "FranceConnectInformation.find_by(:merge_token => merge_token_params).email_france_connect", + "confidence": "Weak", + "note": "explicitely sanitized even if we are using html_safe" + }, { "warning_type": "Cross-Site Scripting", "warning_code": 2, @@ -15,7 +46,7 @@ "type": "controller", "class": "Users::DossiersController", "method": "merci", - "line": 188, + "line": 193, "file": "app/controllers/users/dossiers_controller.rb", "rendered": { "name": "users/dossiers/merci", @@ -58,7 +89,7 @@ "check_name": "Redirect", "message": "Possible unprotected redirect", "file": "app/controllers/instructeurs/procedures_controller.rb", - "line": 180, + "line": 190, "link": "https://brakemanscanner.org/docs/warning_types/redirect/", "code": "redirect_to(Export.find_or_create_export(params[:export_format], (params[:time_span_type] or \"everything\"), current_instructeur.groupe_instructeurs.where(:procedure => procedure)).file.service_url)", "render_path": null, @@ -72,6 +103,6 @@ "note": "" } ], - "updated": "2021-09-02 16:12:11 -0500", + "updated": "2021-11-23 14:09:21 +0100", "brakeman_version": "5.1.1" } diff --git a/config/locales/en.yml b/config/locales/en.yml index a45ad6868..713f9a6b9 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -382,3 +382,23 @@ en: identity_saved: "Identity data is registred" attestation: no_longer_available: "The certificate is no longer available on this file." + france_connect: + particulier: + password_confirmation: + back: 'back to previous step' + already_exists: An account with %{email} already existis on %{application_name} + fill_in_password: fill in your password to merge your accounts + merge: + title: "Merge your account FranceConnect and %{application_name}" + subtitle: "Hello,

Your account FranceConnect uses %{email} as contact email.
But there is an existing %{application_name} account using this email." + label_select_merge_flow: Is %{email} one of your email account ? + title_fill_in_password: To merge your accounts fill in your password + button_merge: Merge accounts + title_fill_in_email: Fill in the email that %{application_name} will use to contact you + button_use_this_email: Use this email + link_confirm_by_email: Confirm by receiving an email + flash: + confirmation_mail_sent: "An email with the confirmation link has been sent, please click on the link." + invalid_password: "The password is not correct." + connection_done: "The accounts for FranceConnect and %{application_name} are now merged." + merger_token_expired: "Le delay to merge your FranceConnect and %{application_name} accounts is expired. Please retry." diff --git a/config/locales/fr.yml b/config/locales/fr.yml index bcedad3dc..7851bb7b4 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -421,3 +421,23 @@ fr: ready: "Validé" needs_configuration: "À configurer" configure_api_particulier_token: "Configurer le jeton API particulier" + france_connect: + particulier: + password_confirmation: + back: 'revenir en arrière' + already_exists: Le compte %{email} existe déjà sur %{application_name} + fill_in_password: entrez votre mot de passe pour fusionner les comptes + merge: + title: "Fusion des comptes FranceConnect et %{application_name}" + subtitle: "Bonjour,

Votre compte FranceConnect utilise %{email} comme email de contact.
Or il existe un compte sur %{application_name} avec cet email." + label_select_merge_flow: Ce compte %{email} vous appartient-il ? + title_fill_in_password: Pour les fusionner, entrez votre mot de passe + button_merge: Fusionner les comptes + title_fill_in_email: Donnez-nous alors le mail que %{application_name} utilisera pour vous contacter + button_use_this_email: Utiliser ce mail + link_confirm_by_email: Confirmer mon compte par email + flash: + confirmation_mail_sent: "Nous venons de vous envoyer le mail de confirmation, veuillez cliquer sur le lien contenu dans ce mail pour fusionner vos comptes" + invalid_password: "Mauvais mot de passe" + connection_done: "Les comptes FranceConnect et %{application_name} sont à présent fusionnés" + merger_token_expired: "Le délai pour fusionner les comptes FranceConnect et %{application_name} est expirée. Veuillez recommencer la procédure pour vous fusionner les comptes." diff --git a/spec/controllers/france_connect/particulier_controller_spec.rb b/spec/controllers/france_connect/particulier_controller_spec.rb index b2a7c0bc9..8509be57f 100644 --- a/spec/controllers/france_connect/particulier_controller_spec.rb +++ b/spec/controllers/france_connect/particulier_controller_spec.rb @@ -267,6 +267,7 @@ describe FranceConnect::ParticulierController, type: :controller do expect(fci.user).to eq(user) expect(fci.merge_token).to be_nil expect(controller.current_user).to eq(user) + expect(flash[:notice]).to eq("Les comptes FranceConnect et #{APPLICATION_NAME} sont à présent fusionnés") end context 'but the targeted user is an instructeur' do From a91960bd4f4a8bf48c5638fa857875a931a51669 Mon Sep 17 00:00:00 2001 From: kara Diaby Date: Wed, 10 Nov 2021 17:05:55 +0100 Subject: [PATCH 07/13] add hidden by user column on dossiers --- db/migrate/20211110095853_add_hide_by_user_at_on_dossiers.rb | 5 +++++ db/schema.rb | 1 + 2 files changed, 6 insertions(+) create mode 100644 db/migrate/20211110095853_add_hide_by_user_at_on_dossiers.rb diff --git a/db/migrate/20211110095853_add_hide_by_user_at_on_dossiers.rb b/db/migrate/20211110095853_add_hide_by_user_at_on_dossiers.rb new file mode 100644 index 000000000..9e7aa12b5 --- /dev/null +++ b/db/migrate/20211110095853_add_hide_by_user_at_on_dossiers.rb @@ -0,0 +1,5 @@ +class AddHideByUserAtOnDossiers < ActiveRecord::Migration[6.1] + def change + add_column :dossiers, :hidden_by_user_at, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index c4625c91d..748e91ff5 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -324,6 +324,7 @@ ActiveRecord::Schema.define(version: 2021_11_24_111429) do t.bigint "dossier_transfer_id" t.datetime "identity_updated_at" t.datetime "depose_at" + t.datetime "hidden_by_user_at" t.index ["archived"], name: "index_dossiers_on_archived" t.index ["dossier_transfer_id"], name: "index_dossiers_on_dossier_transfer_id" t.index ["groupe_instructeur_id"], name: "index_dossiers_on_groupe_instructeur_id" From eaac293da3866230cb77e5120f0cc706ea00552a Mon Sep 17 00:00:00 2001 From: kara Diaby Date: Wed, 10 Nov 2021 17:36:24 +0100 Subject: [PATCH 08/13] =?UTF-8?q?add=20a=20new=20tab=20trait=C3=A9s=20on?= =?UTF-8?q?=20user=20dossiers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/controllers/users/dossiers_controller.rb | 21 +++++++++++----- app/models/dossier.rb | 8 +++++++ app/views/users/dossiers/index.html.haml | 25 +++++++++++++------- config/locales/en.yml | 8 +++---- config/locales/fr.yml | 13 ++++++---- 5 files changed, 53 insertions(+), 22 deletions(-) diff --git a/app/controllers/users/dossiers_controller.rb b/app/controllers/users/dossiers_controller.rb index a3ca43124..a5e6cfc9d 100644 --- a/app/controllers/users/dossiers_controller.rb +++ b/app/controllers/users/dossiers_controller.rb @@ -16,7 +16,8 @@ module Users before_action :store_user_location!, only: :new def index - @user_dossiers = current_user.dossiers.includes(:procedure).order_by_updated_at.page(page) + @user_dossiers = current_user.dossiers.includes(:procedure).not_termine.order_by_updated_at.page(page) + @dossiers_traites = current_user.dossiers.includes(:procedure).termine.not_hidden_by_user.order_by_updated_at.page(page) @dossiers_invites = current_user.dossiers_invites.includes(:procedure).order_by_updated_at.page(page) @dossiers_supprimes = current_user.deleted_dossiers.order_by_updated_at.page(page) @dossier_transfers = DossierTransfer @@ -25,7 +26,7 @@ module Users .where(email: current_user.email) .page(page) @dossiers_close_to_expiration = current_user.dossiers.close_to_expiration.page(page) - @statut = statut(@user_dossiers, @dossiers_invites, @dossiers_supprimes, @dossier_transfers, @dossiers_close_to_expiration, params[:statut]) + @statut = statut(@user_dossiers, @dossiers_traites, @dossiers_invites, @dossiers_supprimes, @dossier_transfers, @dossiers_close_to_expiration, params[:statut]) end def show @@ -287,14 +288,22 @@ module Users @transfer = DossierTransfer.new(dossiers: current_user.dossiers) end + def hide_dossier + dossier = current_user.dossiers.includes(:user, procedure: :administrateurs).find(params[:id]) + dossier.update(hidden_by_user_at: Time.zone.now) + flash.notice = t('users.dossiers.ask_deletion.deleted_dossier') + redirect_to dossiers_path + end + private # if the status tab is filled, then this tab # else first filled tab - # else mes-dossiers - def statut(mes_dossiers, dossiers_invites, dossiers_supprimes, dossier_transfers, dossiers_close_to_expiration, params_statut) + # else en-cours + def statut(mes_dossiers, dossiers_traites, dossiers_invites, dossiers_supprimes, dossier_transfers, dossiers_close_to_expiration, params_statut) tabs = { - 'mes-dossiers' => mes_dossiers.present?, + 'en-cours' => mes_dossiers.present?, + 'traites' => dossiers_traites.present?, 'dossiers-invites' => dossiers_invites.present?, 'dossiers-supprimes' => dossiers_supprimes.present?, 'dossiers-transferes' => dossier_transfers.present?, @@ -306,7 +315,7 @@ module Users tabs .filter { |_tab, filled| filled } .map { |tab, _| tab } - .first || 'mes-dossiers' + .first || 'en-cours' end end diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 12dd9b761..b4144c366 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -16,6 +16,7 @@ # en_instruction_at :datetime # groupe_instructeur_updated_at :datetime # hidden_at :datetime +# hidden_by_user_at :datetime # identity_updated_at :datetime # last_avis_updated_at :datetime # last_champ_private_updated_at :datetime @@ -206,9 +207,11 @@ class Dossier < ApplicationRecord scope :state_en_construction_ou_instruction, -> { where(state: EN_CONSTRUCTION_OU_INSTRUCTION) } scope :state_instruction_commencee, -> { where(state: INSTRUCTION_COMMENCEE) } scope :state_termine, -> { where(state: TERMINE) } + scope :state_not_termine, -> { where.not(state: TERMINE) } scope :archived, -> { where(archived: true) } scope :not_archived, -> { where(archived: false) } + scope :not_hidden_by_user, -> { where(hidden_by_user_at: nil) } scope :order_by_updated_at, -> (order = :desc) { order(updated_at: order) } scope :order_by_created_at, -> (order = :asc) { order(en_construction_at: order, created_at: order, id: order) } @@ -229,6 +232,7 @@ class Dossier < ApplicationRecord scope :en_construction, -> { not_archived.state_en_construction } scope :en_instruction, -> { not_archived.state_en_instruction } scope :termine, -> { not_archived.state_termine } + scope :not_termine, -> { state_not_termine } scope :processed_in_month, -> (month) do state_termine .joins(:traitements) @@ -1091,6 +1095,10 @@ class Dossier < ApplicationRecord end end + def hidden_by_user? + self.hidden_by_user_at.present? + end + private def create_missing_traitemets diff --git a/app/views/users/dossiers/index.html.haml b/app/views/users/dossiers/index.html.haml index bebc753a8..2129ef487 100644 --- a/app/views/users/dossiers/index.html.haml +++ b/app/views/users/dossiers/index.html.haml @@ -15,25 +15,31 @@ - else %h1.page-title= t('views.users.dossiers.index.dossiers') %ul.tabs - - if @user_dossiers.count > 0 - = tab_item(t('pluralize.mes_dossiers', count: @user_dossiers.count), - dossiers_path(statut: 'mes-dossiers'), - active: @statut == 'mes-dossiers', + - if @user_dossiers.present? + = tab_item(t('pluralize.en_cours', count: @user_dossiers.count), + dossiers_path(statut: 'en-cours'), + active: @statut == 'en-cours', badge: number_with_html_delimiter(@user_dossiers.count)) - - if @dossiers_invites.count > 0 + - if @dossiers_traites.present? + = tab_item(t('pluralize.traites', count: @dossiers_traites.count), + dossiers_path(statut: 'traites'), + active: @statut == 'traites', + badge: number_with_html_delimiter(@dossiers_traites.count)) + + - if @dossiers_invites.present? = tab_item(t('pluralize.dossiers_invites', count: @dossiers_invites.count), dossiers_path(statut: 'dossiers-invites'), active: @statut == 'dossiers-invites', badge: number_with_html_delimiter(@dossiers_invites.count)) - - if @dossiers_supprimes.count > 0 + - if @dossiers_supprimes.present? = tab_item(t('pluralize.dossiers_supprimes', count: @dossiers_supprimes.count), dossiers_path(statut: 'dossiers-supprimes'), active: @statut == 'dossiers-supprimes', badge: number_with_html_delimiter(@dossiers_supprimes.count)) - - if @dossier_transfers.count > 0 + - if @dossier_transfers.present? = tab_item(t('pluralize.dossiers_transferes', count: @dossier_transfers.count), dossiers_path(statut: 'dossiers-transferes'), active: @statut == 'dossiers-transferes', @@ -46,9 +52,12 @@ badge: number_with_html_delimiter(@dossiers_close_to_expiration.count)) .container - - if @statut == "mes-dossiers" + - if @statut == "en-cours" = render partial: "dossiers_list", locals: { dossiers: @user_dossiers } + - if @statut == "traites" + = render partial: "dossiers_list", locals: { dossiers: @dossiers_traites } + - if @statut == "dossiers-invites" = render partial: "dossiers_list", locals: { dossiers: @dossiers_invites } diff --git a/config/locales/en.yml b/config/locales/en.yml index 713f9a6b9..eda57e614 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -326,10 +326,10 @@ en: zero: archived one: archived other: archived - mes_dossiers: - zero: my file - one: my file - other: my files + en_cours: + zero: in progress + one: in progress + other: in progres dossiers_invites: zero: guest file one: guest file diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 7851bb7b4..cc8c3a00b 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -188,6 +188,7 @@ fr: start_other_dossier: "Commencer un autre dossier" delete_dossier: "Supprimer le dossier" transfer_dossier: "Transferer le dossier" + cacher_dossier: 'Cacher le dossier' edit_draft: "Modifier le brouillon" actions: "Actions" sessions: @@ -334,10 +335,14 @@ fr: zero: archivé one: archivé other: archivés - mes_dossiers: - zero: mon dossier - one: mon dossier - other: mes dossiers + en_cours: + zero: en cours + one: en cours + other: en cours + traites: + zero: traité + one: traité + other: traités dossiers_invites: zero: dossier invité one: dossier invité From 06540c1b932b732bcc39daf3667c393d31fc620c Mon Sep 17 00:00:00 2001 From: kara Diaby Date: Wed, 10 Nov 2021 17:36:42 +0100 Subject: [PATCH 09/13] tests --- .../users/dossiers_controller_spec.rb | 30 ++++++++++++++----- .../_dossier_actions.html.haml_spec.rb | 10 ------- .../users/dossiers/index.html.haml_spec.rb | 18 ++++++++--- 3 files changed, 37 insertions(+), 21 deletions(-) diff --git a/spec/controllers/users/dossiers_controller_spec.rb b/spec/controllers/users/dossiers_controller_spec.rb index 75a0dcfb3..b9a60f226 100644 --- a/spec/controllers/users/dossiers_controller_spec.rb +++ b/spec/controllers/users/dossiers_controller_spec.rb @@ -793,14 +793,14 @@ describe Users::DossiersController, type: :controller do context 'when the user does not have any dossiers' do before { get(:index) } - it { expect(assigns(:statut)).to eq('mes-dossiers') } + it { expect(assigns(:statut)).to eq('en-cours') } end context 'when the user only have its own dossiers' do let!(:own_dossier) { create(:dossier, user: user) } before { get(:index) } - it { expect(assigns(:statut)).to eq('mes-dossiers') } + it { expect(assigns(:statut)).to eq('en-cours') } it { expect(assigns(:user_dossiers)).to match([own_dossier]) } end @@ -813,14 +813,16 @@ describe Users::DossiersController, type: :controller do it { expect(assigns(:dossiers_invites)).to match([invite.dossier]) } end - context 'when the user has both' do + context 'when the user has dossiers invites, own and traites' do + let!(:procedure) { create(:procedure, :published) } let!(:own_dossier) { create(:dossier, user: user) } + let!(:own_dossier2) { create(:dossier, user: user, state: "accepte", procedure: procedure) } let!(:invite) { create(:invite, dossier: create(:dossier), user: user) } context 'and there is no statut param' do before { get(:index) } - it { expect(assigns(:statut)).to eq('mes-dossiers') } + it { expect(assigns(:statut)).to eq('en-cours') } end context 'and there is "dossiers-invites" param' do @@ -829,10 +831,24 @@ describe Users::DossiersController, type: :controller do it { expect(assigns(:statut)).to eq('dossiers-invites') } end - context 'and there is "mes-dossiers" param' do - before { get(:index, params: { statut: 'mes-dossiers' }) } + context 'and there is "en-cours" param' do + before { get(:index, params: { statut: 'en-cours' }) } - it { expect(assigns(:statut)).to eq('mes-dossiers') } + it { expect(assigns(:statut)).to eq('en-cours') } + end + + context 'and there is "traites" param' do + before { get(:index, params: { statut: 'traites' }) } + + it { expect(assigns(:statut)).to eq('traites') } + end + + context 'and the traité dossier has been hidden by user' do + before do + own_dossier2.update!(hidden_by_user_at: Time.zone.now) + get(:index, params: { statut: 'traites' }) + end + it { expect(assigns(:statut)).to eq('en-cours') } end end diff --git a/spec/views/users/dossiers/_dossier_actions.html.haml_spec.rb b/spec/views/users/dossiers/_dossier_actions.html.haml_spec.rb index c4512b41e..0859ea60f 100644 --- a/spec/views/users/dossiers/_dossier_actions.html.haml_spec.rb +++ b/spec/views/users/dossiers/_dossier_actions.html.haml_spec.rb @@ -18,14 +18,4 @@ describe 'users/dossiers/dossier_actions.html.haml', type: :view do let(:procedure) { create(:procedure, :closed) } it { is_expected.not_to have_link('Commencer un autre dossier') } end - - context 'when there are no actions to display' do - let(:procedure) { create(:procedure, :closed) } - let(:dossier) { create(:dossier, :accepte, procedure: procedure) } - let(:user) { create(:user) } - - it 'doesn’t render the menu at all' do - expect(subject).not_to have_selector('.dropdown') - end - end end diff --git a/spec/views/users/dossiers/index.html.haml_spec.rb b/spec/views/users/dossiers/index.html.haml_spec.rb index 2764bb99c..ce5d4c7f1 100644 --- a/spec/views/users/dossiers/index.html.haml_spec.rb +++ b/spec/views/users/dossiers/index.html.haml_spec.rb @@ -2,9 +2,10 @@ describe 'users/dossiers/index.html.haml', type: :view do let(:user) { create(:user) } let(:dossier_brouillon) { create(:dossier, state: Dossier.states.fetch(:brouillon), user: user) } let(:dossier_en_construction) { create(:dossier, state: Dossier.states.fetch(:en_construction), user: user) } - let(:user_dossiers) { [dossier_brouillon, dossier_en_construction] } + let(:dossier_termine) { create(:dossier, state: Dossier.states.fetch(:accepte), user: user) } + let(:user_dossiers) { [dossier_brouillon, dossier_en_construction, dossier_termine] } let(:dossiers_invites) { [] } - let(:statut) { 'mes-dossiers' } + let(:statut) { 'en-cours' } before do allow(view).to receive(:new_demarche_url).and_return('#') @@ -12,6 +13,7 @@ describe 'users/dossiers/index.html.haml', type: :view do assign(:user_dossiers, Kaminari.paginate_array(user_dossiers).page(1)) assign(:dossiers_invites, Kaminari.paginate_array(dossiers_invites).page(1)) assign(:dossiers_supprimes, Kaminari.paginate_array(user_dossiers).page(1)) + assign(:dossiers_traites, Kaminari.paginate_array(user_dossiers).page(1)) assign(:dossier_transfers, Kaminari.paginate_array([]).page(1)) assign(:dossiers_close_to_expiration, Kaminari.paginate_array([]).page(1)) assign(:statut, statut) @@ -19,7 +21,7 @@ describe 'users/dossiers/index.html.haml', type: :view do end it 'affiche la liste des dossiers' do - expect(rendered).to have_selector('.dossiers-table tbody tr', count: 2) + expect(rendered).to have_selector('.dossiers-table tbody tr', count: 3) end it 'affiche les informations des dossiers' do @@ -67,8 +69,16 @@ describe 'users/dossiers/index.html.haml', type: :view do it 'affiche la barre d’onglets' do expect(rendered).to have_selector('ul.tabs') - expect(rendered).to have_selector('ul.tabs li', count: 3) + expect(rendered).to have_selector('ul.tabs li', count: 4) expect(rendered).to have_selector('ul.tabs li.active', count: 1) end end + + context 'where there is a traite dossier' do + let(:dossiers_traites) { create_list(:dossier, 1) } + + it "displays the hide by user at button" do + expect(rendered).to have_text("Supprimer le dossier") + end + end end From fef7f7923756dc80510803b8098f09fa05cd3130 Mon Sep 17 00:00:00 2001 From: Kara Diaby Date: Wed, 24 Nov 2021 10:34:48 +0100 Subject: [PATCH 10/13] add route --- config/routes.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/config/routes.rb b/config/routes.rb index 51117675b..30df3ec00 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -268,6 +268,7 @@ Rails.application.routes.draw do get 'messagerie' post 'commentaire' => 'dossiers#create_commentaire' post 'ask_deletion' + patch 'hide_dossier' get 'attestation' get 'transferer', to: 'dossiers#transferer' end From 24ba7b66334a28d917b4fc669c6dfc24951add15 Mon Sep 17 00:00:00 2001 From: Kara Diaby Date: Wed, 24 Nov 2021 10:35:19 +0100 Subject: [PATCH 11/13] modify dossier projection service --- app/services/dossier_projection_service.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/app/services/dossier_projection_service.rb b/app/services/dossier_projection_service.rb index c88c5a7e9..bb03493ba 100644 --- a/app/services/dossier_projection_service.rb +++ b/app/services/dossier_projection_service.rb @@ -1,5 +1,5 @@ class DossierProjectionService - class DossierProjection < Struct.new(:dossier_id, :state, :archived, :columns) + class DossierProjection < Struct.new(:dossier_id, :state, :archived, :hidden_by_user_at, :columns) end TABLE = 'table' @@ -20,8 +20,9 @@ class DossierProjectionService def self.project(dossiers_ids, fields) state_field = { TABLE => 'self', COLUMN => 'state' } archived_field = { TABLE => 'self', COLUMN => 'archived' } + hidden_by_user_at_field = { TABLE => 'self', COLUMN => 'hidden_by_user_at' } - ([state_field, archived_field] + fields) # the view needs state and archived dossier attributes + ([state_field, archived_field, hidden_by_user_at_field] + fields) # the view needs state and archived dossier attributes .each { |f| f[:id_value_h] = {} } .group_by { |f| f[TABLE] } # one query per table .each do |table, fields| @@ -45,7 +46,7 @@ class DossierProjectionService .pluck(:id, *fields.map { |f| f[COLUMN].to_sym }) .each do |id, *columns| fields.zip(columns).each do |field, value| - if [state_field, archived_field].include?(field) + if [state_field, archived_field, hidden_by_user_at_field].include?(field) field[:id_value_h][id] = value else field[:id_value_h][id] = value&.strftime('%d/%m/%Y') # other fields are datetime @@ -98,6 +99,7 @@ class DossierProjectionService dossier_id, state_field[:id_value_h][dossier_id], archived_field[:id_value_h][dossier_id], + hidden_by_user_at_field[:id_value_h][dossier_id], fields.map { |f| f[:id_value_h][dossier_id] } ) end From 2103f09be125f1ef5273d3b84b7864d99b09313d Mon Sep 17 00:00:00 2001 From: Kara Diaby Date: Wed, 24 Nov 2021 10:35:34 +0100 Subject: [PATCH 12/13] layout --- app/assets/stylesheets/dossiers_table.scss | 8 ++++++++ app/views/commencer/show.html.haml | 2 +- app/views/instructeurs/procedures/show.html.haml | 7 ++++--- app/views/users/dossiers/_dossier_actions.html.haml | 10 +++++++++- 4 files changed, 22 insertions(+), 5 deletions(-) diff --git a/app/assets/stylesheets/dossiers_table.scss b/app/assets/stylesheets/dossiers_table.scss index 32fec296e..7d836033c 100644 --- a/app/assets/stylesheets/dossiers_table.scss +++ b/app/assets/stylesheets/dossiers_table.scss @@ -83,3 +83,11 @@ width: 200px; } } + +.file-hidden-by-user { + background-color: rgba(242, 137, 0, 0.6); + + &:hover { + background-color: rgba(242, 137, 0, 0.6) !important; + } +} diff --git a/app/views/commencer/show.html.haml b/app/views/commencer/show.html.haml index 4eb4840a5..28d78643a 100644 --- a/app/views/commencer/show.html.haml +++ b/app/views/commencer/show.html.haml @@ -11,7 +11,7 @@ = link_to t('views.shared.account.already_user'), commencer_sign_in_path(path: @procedure.path), class: ['button large expand'] - else - - dossiers = current_user.dossiers.where(revision: @revision.draft? ? @revision : @procedure.revisions.where.not(id: @procedure.draft_revision_id)) + - dossiers = current_user.dossiers.where(hidden_by_user_at: nil, revision: @revision.draft? ? @revision : @procedure.revisions.where.not(id: @procedure.draft_revision_id)) - drafts = dossiers.merge(Dossier.state_brouillon) - not_drafts = dossiers.merge(Dossier.state_not_brouillon) diff --git a/app/views/instructeurs/procedures/show.html.haml b/app/views/instructeurs/procedures/show.html.haml index 15d32e1fb..1ab6d2f62 100644 --- a/app/views/instructeurs/procedures/show.html.haml +++ b/app/views/instructeurs/procedures/show.html.haml @@ -100,8 +100,7 @@ %tbody - @projected_dossiers.each do |p| - path = instructeur_dossier_path(@procedure, p.dossier_id) - - %tr + %tr{ class: [p.hidden_by_user_at.present? && "file-hidden-by-user"] } %td.folder-col %a.cell-link{ href: path } %span.icon.folder @@ -113,7 +112,9 @@ - p.columns.each do |column| %td - %a.cell-link{ href: path }= column + %a.cell-link{ href: path } + = column + = "- #{t('views.instructeurs.dossiers.deleted_by_user')}" if p.hidden_by_user_at.present? %td.status-col %a.cell-link{ href: path }= status_badge(p.state) diff --git a/app/views/users/dossiers/_dossier_actions.html.haml b/app/views/users/dossiers/_dossier_actions.html.haml index 27f39af37..6e510222d 100644 --- a/app/views/users/dossiers/_dossier_actions.html.haml +++ b/app/views/users/dossiers/_dossier_actions.html.haml @@ -2,7 +2,8 @@ - has_delete_action = dossier.can_be_deleted_by_user? - has_new_dossier_action = dossier.procedure.accepts_new_dossiers? - has_transfer_action = dossier.user == current_user -- has_actions = has_edit_action || has_delete_action || has_new_dossier_action || has_transfer_action +- has_hide_action = dossier.termine? && dossier.hidden_by_user_at.nil? +- has_actions = has_edit_action || has_delete_action || has_new_dossier_action || has_transfer_action || has_hide_action - if has_actions .dropdown.user-dossier-actions @@ -44,3 +45,10 @@ %span.icon.delete .dropdown-description = t('views.users.dossiers.dossier_action.delete_dossier') + - if has_hide_action + %li + = link_to hide_dossier_dossier_path(dossier), method: :patch do + %span.icon.delete + .dropdown-description + = t('views.users.dossiers.dossier_action.hide_dossier') + From 67d331e788b6a56db7d1888ea2c400ba691fd6ca Mon Sep 17 00:00:00 2001 From: Kara Diaby Date: Wed, 24 Nov 2021 10:35:50 +0100 Subject: [PATCH 13/13] modify locales fr and en --- config/locales/en.yml | 10 +++++++++- config/locales/fr.yml | 5 ++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index eda57e614..875000695 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -132,6 +132,9 @@ en: form: "Form" edit_siret: "Edit SIRET" edit_identity: "Edit identity data" + instructeurs: + dossiers: + deleted_by_user: "File deleted by user" users: dossiers: autosave: @@ -191,6 +194,7 @@ en: edit_dossier: "Edit the file" start_other_dossier: "Start an other file" delete_dossier: "Delete the file" + hide_dossier: "Delete from your screen" transfer_dossier: "Transfer the file" edit_draft: "Edit the draft" actions: "Actions" @@ -329,7 +333,11 @@ en: en_cours: zero: in progress one: in progress - other: in progres + other: in progress + traites: + zero: finished + one: finished + other: finished dossiers_invites: zero: guest file one: guest file diff --git a/config/locales/fr.yml b/config/locales/fr.yml index cc8c3a00b..5196e8aab 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -128,6 +128,9 @@ fr: form: "Formulaire" edit_siret: "Modifier le SIRET" edit_identity: "Modifier l’identité" + instructeurs: + dossiers: + deleted_by_user: "Dossier supprimé par l'usager" users: dossiers: autosave: @@ -187,8 +190,8 @@ fr: edit_dossier: "Modifier le dossier" start_other_dossier: "Commencer un autre dossier" delete_dossier: "Supprimer le dossier" + hide_dossier: "Supprimer de votre interface" transfer_dossier: "Transferer le dossier" - cacher_dossier: 'Cacher le dossier' edit_draft: "Modifier le brouillon" actions: "Actions" sessions: