Merge pull request #6620 from betagouv/main

2021-11-05-01

.ruby-version

@@ -1 +1 @@
-2.7.1
+2.7.3

CONTRIBUTING.md

@@ -32,13 +32,11 @@ Une fois votre PR approuvée, elle sera intégrée dans la base de code principa
 
 Nous mettons en production au minimum une fois par semaine (et généralement plus) : vos changements seront disponibles en production sur [demarches-simplifiees.fr](https://www.demarches-simplifiees.fr) quelques jours après.
 
-
-# Héberger demarches-simplifiees.fr
+## Héberger demarches-simplifiees.fr
 
 demarches-simplifiees.fr est **compliqué à héberger**. Parmi les problématiques que nous rencontrons :
 
 - **Sécurité et confidentialité des données** : par nature, demarches-simplifiees.fr est appelé à traiter des natures de données qui peuvent présenter des caractéristiqus plus ou moins sensibles. La sécurité de l’infrastructure doit être contrôlée et certifiée pour garantir la confidentialité des données. Cela implique par exemple une démarche de mise en conformité avec le [Référentiel Général de Sécurité](https://www.ssi.gouv.fr/entreprise/reglementation/confiance-numerique/le-referentiel-general-de-securite-rgs/), qui est un processus assez lourd.
-
   C’est également valable pour le stockage des pièces-jointes, qui peuvent la aussi présenter des particularités et des sensibilités dont la confidentialité doit être garantie.
 - **Utilisation de services externes** : demarches-simplifiees.fr s’interconnecte à de nombreux services externes : des APIs (API Entreprise, API Carto, la Base Adresse Nationale, etc.) – mais aussi des services pour le stockage externe des pièces-jointes, l’analyse anti-virus ou l’envoi des emails. Le fonctionnement de demarches-simplifiees.fr dépend de la disponibilité de ces services externes.
 - **Mises à jour** : le schéma de la base de données change régulièrement. Nous codons également des scripts pour harmoniser les anciennes données. Parfois des modifications ponctuelles sont effectuées sur des démarches anciennes, pour les mettre en conformité avec de nouvelles règles métiers. Nous maintenons également les dépendances logicielles utilisées – notamment en réagissant rapidement lorsqu’une faille de sécurité est signalée. Ces mises à jour fréquentes en production sont indispensables au bon fonctionnement de l’outil.
@@ -48,3 +46,12 @@ Si vous souhaitez adapter demarches-simplifiees.fr à votre besoin, nous vous re
 Dans le cas où vous envisagez d’héberger une instance de demarches-simplifiees.fr vous-même, nous n'avons malheureusement pas les moyens de vous accompagner, ni d’assurer de support technique concernant votre installation.
 
 Toutefois, certains acteurs (le ministère des armées, l’administration autonome en Polynésie française) ont déployé des instances séparées. Nous proposons aux personnes intéressées de les mettre en relation avec ces acteurs existants, afin de disposer d’un retour d’expérience, et de bénéficier de leur retour.
+
+## Bonnes pratiques de code
+
+Votre contribution sera plus rapidement traitée si elle s'appuie sur nos habitudes de développement.
+
+Nous travaillons à rendre explicites un maximum de nos pratiques de dev, aussi il est chaudement recommandé
+de prendre connaissance des [bonnes pratiques de code](doc/Contributions/Pratiques-de-dev.md).
+
+Merci de votre intérêt pour le projet !

Gemfile.lock

@@ -239,8 +239,8 @@ GEM
     erubis (2.7.0)
     et-orbi (1.2.4)
       tzinfo
-    ethon (0.12.0)
-      ffi (>= 1.3.0)
+    ethon (0.15.0)
+      ffi (>= 1.15.0)
     excon (0.79.0)
     execjs (2.7.0)
     factory_bot (6.1.0)
@@ -258,7 +258,7 @@ GEM
     faraday-excon (1.1.0)
     faraday-net_http (1.0.1)
     faraday-net_http_persistent (1.1.0)
-    ffi (1.15.0)
+    ffi (1.15.4)
     flipper (0.20.3)
     flipper-active_record (0.20.3)
       activerecord (>= 5.0, < 7)

app/controllers/users/profil_controller.rb

@@ -1,6 +1,9 @@
 module Users
   class ProfilController < UserController
+    before_action :ensure_update_email_is_authorized, only: :update_email
+
     def show
+      @waiting_merge_emails = waiting_merge_emails
       @waiting_transfers = current_user.dossiers.joins(:transfer).group('dossier_transfers.email').count.to_a
     end
 
@@ -11,13 +14,16 @@ module Users
     end
 
     def update_email
-      if current_user.instructeur? && !target_email_allowed?
-        flash.alert = t('.email_not_allowed', contact_email: CONTACT_EMAIL, requested_email: requested_email)
-      elsif current_user.update(update_email_params)
+      requested_user = User.find_by(email: requested_email)
+
+      if requested_user.present?
+        current_user.ask_for_merge(requested_user)
+        current_user.update(unconfirmed_email: nil)
+
         flash.notice = t('devise.registrations.update_needs_confirmation')
-      elsif current_user.errors&.details&.dig(:email)&.any? { |e| e[:error] == :taken }
-        UserMailer.account_already_taken(current_user, requested_email).deliver_later
-        # avoid leaking information about whether an account with this email exists or not
+      elsif current_user.update(update_email_params)
+        current_user.update(requested_merge_into: nil)
+
         flash.notice = t('devise.registrations.update_needs_confirmation')
       else
         flash.alert = current_user.errors.full_messages
@@ -32,8 +38,39 @@ module Users
       redirect_to profil_path
     end
 
+    def accept_merge
+      users_requesting_merge.each { |user| current_user.merge(user) }
+      users_requesting_merge.update_all(requested_merge_into_id: nil)
+
+      flash.notice = "Vous avez absorbé le compte #{waiting_merge_emails.join(', ')}"
+      redirect_to profil_path
+    end
+
+    def refuse_merge
+      users = users_requesting_merge
+      users.update_all(requested_merge_into_id: nil)
+
+      flash.notice = 'La fusion a été refusé'
+      redirect_to profil_path
+    end
+
     private
 
+    def waiting_merge_emails
+      users_requesting_merge.pluck(:email)
+    end
+
+    def users_requesting_merge
+      @requesting_merge ||= current_user.requested_merge_from
+    end
+
+    def ensure_update_email_is_authorized
+      if current_user.instructeur? && !target_email_allowed?
+        flash.alert = t('users.profil.ensure_update_email_is_authorized.email_not_allowed', contact_email: CONTACT_EMAIL, requested_email: requested_email)
+        redirect_to profil_path
+      end
+    end
+
     def update_email_params
       params.require(:user).permit(:email)
     end

app/javascript/components/ComboCommunesSearch.jsx

@@ -1,9 +1,10 @@
-import React from 'react';
+import React, { useState, useMemo } from 'react';
 import { QueryClientProvider } from 'react-query';
 import { matchSorter } from 'match-sorter';
 
 import ComboSearch from './ComboSearch';
 import { queryClient } from './shared/queryClient';
+import { ComboDepartementsSearch } from './ComboDepartementsSearch';
 
 // Avoid hiding similar matches for precise queries (like "Sainte Marie")
 function searchResultsLimit(term) {
@@ -36,20 +37,71 @@ function expandResultsWithMultiplePostalCodes(term, results) {
   return expandedResults;
 }
 
+const placeholderDepartements = [
+  ['63 – Puy-de-Dôme', 'Clermont-Ferrand'],
+  ['77 – Seine-et-Marne', 'Melun'],
+  ['22 – Côtes d’Armor', 'Saint-Brieuc'],
+  ['47 – Lot-et-Garonne', 'Agen']
+];
+const [placeholderDepartement, placeholderCommune] =
+  placeholderDepartements[
+    Math.floor(Math.random() * (placeholderDepartements.length - 1))
+  ];
+
 function ComboCommunesSearch(params) {
+  const [departementCode, setDepartementCode] = useState();
+  const inputId = useMemo(
+    () =>
+      document.querySelector(`input[data-uuid="${params.hiddenFieldId}"]`)?.id,
+    [params.hiddenFieldId]
+  );
+  const departementDescribedBy = `${inputId}_departement_notice`;
+  const communeDescribedBy = `${inputId}_commune_notice`;
+
   return (
     <QueryClientProvider client={queryClient}>
-      <ComboSearch
-        required={params.mandatory}
-        hiddenFieldId={params.hiddenFieldId}
-        scope="communes"
-        minimumInputLength={2}
-        transformResult={({ code, nom, codesPostaux }) => [
-          code,
-          `${nom} (${codesPostaux[0]})`
-        ]}
-        transformResults={expandResultsWithMultiplePostalCodes}
-      />
+      <div>
+        <div className="notice" id={departementDescribedBy}>
+          <p>
+            Choisissez le département dans lequel se situe la commune. Vous
+            pouvez entrer le nom ou le code.
+          </p>
+        </div>
+        <ComboDepartementsSearch
+          inputId={!departementCode ? inputId : null}
+          aria-describedby={departementDescribedBy}
+          placeholder={placeholderDepartement}
+          mandatory={params.mandatory}
+          onChange={(_, result) => {
+            setDepartementCode(result?.code);
+          }}
+        />
+      </div>
+      {departementCode ? (
+        <div>
+          <div className="notice" id={communeDescribedBy}>
+            <p>
+              Choisissez la commune. Vous pouver entre le nom ou le code postal.
+            </p>
+          </div>
+          <ComboSearch
+            autoFocus
+            inputId={inputId}
+            aria-describedby={communeDescribedBy}
+            placeholder={placeholderCommune}
+            required={params.mandatory}
+            hiddenFieldId={params.hiddenFieldId}
+            scope="communes"
+            scopeExtra={departementCode}
+            minimumInputLength={2}
+            transformResult={({ code, nom, codesPostaux }) => [
+              code,
+              `${nom} (${codesPostaux[0]})`
+            ]}
+            transformResults={expandResultsWithMultiplePostalCodes}
+          />
+        </div>
+      ) : null}
     </QueryClientProvider>
   );
 }

app/javascript/components/ComboDepartementsSearch.jsx

@@ -16,19 +16,27 @@ function expandResultsWithForeignDepartement(term, results) {
   ];
 }
 
-function ComboDepartementsSearch(params) {
+export function ComboDepartementsSearch(params) {
+  return (
+    <ComboSearch
+      {...params}
+      scope="departements"
+      minimumInputLength={1}
+      transformResult={({ code, nom }) => [code, `${code} - ${nom}`]}
+      transformResults={expandResultsWithForeignDepartement}
+    />
+  );
+}
+
+function ComboDepartementsSearchDefault(params) {
   return (
     <QueryClientProvider client={queryClient}>
-      <ComboSearch
+      <ComboDepartementsSearch
         required={params.mandatory}
         hiddenFieldId={params.hiddenFieldId}
-        scope="departements"
-        minimumInputLength={1}
-        transformResult={({ code, nom }) => [code, `${code} - ${nom}`]}
-        transformResults={expandResultsWithForeignDepartement}
       />
     </QueryClientProvider>
   );
 }
 
-export default ComboDepartementsSearch;
+export default ComboDepartementsSearchDefault;

app/javascript/components/ComboSearch.jsx

@@ -1,4 +1,10 @@
-import React, { useState, useMemo, useCallback, useRef } from 'react';
+import React, {
+  useState,
+  useMemo,
+  useCallback,
+  useRef,
+  useEffect
+} from 'react';
 import { useDebounce } from 'use-debounce';
 import { useQuery } from 'react-query';
 import PropTypes from 'prop-types';
@@ -19,22 +25,25 @@ function defaultTransformResults(_, results) {
 }
 
 function ComboSearch({
-  placeholder,
-  required,
   hiddenFieldId,
   onChange,
   scope,
+  inputId,
+  scopeExtra,
   minimumInputLength,
   transformResult,
   allowInputValues = false,
   transformResults = defaultTransformResults,
-  className
+  ...props
 }) {
-  const label = scope;
   const hiddenValueField = useMemo(
     () => document.querySelector(`input[data-uuid="${hiddenFieldId}"]`),
     [hiddenFieldId]
   );
+  const comboInputId = useMemo(
+    () => hiddenValueField?.id || inputId,
+    [inputId, hiddenValueField]
+  );
   const hiddenIdField = useMemo(
     () =>
       document.querySelector(
@@ -81,15 +90,18 @@ function ComboSearch({
   const handleOnChange = useCallback(
     ({ target: { value } }) => {
       setValue(value);
-      if (value.length >= minimumInputLength) {
+      if (!value) {
+        setExternalId('');
+        setExternalValue('');
+        if (onChange) {
+          onChange(null);
+        }
+      } else if (value.length >= minimumInputLength) {
         setSearchTerm(value.trim());
         if (allowInputValues) {
           setExternalId('');
           setExternalValue(value);
         }
-      } else if (!value) {
-        setExternalId('');
-        setExternalValue('');
       }
     },
     [minimumInputLength]
@@ -102,11 +114,14 @@ function ComboSearch({
     awaitFormSubmit.done();
   }, []);
 
-  const { isSuccess, data } = useQuery([scope, debouncedSearchTerm], {
-    enabled: !!debouncedSearchTerm,
-    notifyOnStatusChange: false,
-    refetchOnMount: false
-  });
+  const { isSuccess, data } = useQuery(
+    [scope, debouncedSearchTerm, scopeExtra],
+    {
+      enabled: !!debouncedSearchTerm,
+      notifyOnStatusChange: false,
+      refetchOnMount: false
+    }
+  );
   const results = isSuccess ? transformResults(debouncedSearchTerm, data) : [];
 
   const onBlur = useCallback(() => {
@@ -118,15 +133,20 @@ function ComboSearch({
     }
   }, [data]);
 
+  useEffect(() => {
+    document
+      .querySelector(`#${comboInputId}[type="hidden"]`)
+      ?.removeAttribute('id');
+  }, [comboInputId]);
+
   return (
-    <Combobox aria-label={label} onSelect={handleOnSelect}>
+    <Combobox onSelect={handleOnSelect}>
       <ComboboxInput
-        className={className}
-        placeholder={placeholder}
+        {...props}
+        id={comboInputId}
         onChange={handleOnChange}
         onBlur={onBlur}
         value={value}
-        required={required}
       />
       {isSuccess && (
         <ComboboxPopover className="shadow-popup">
@@ -157,8 +177,6 @@ function ComboSearch({
 }
 
 ComboSearch.propTypes = {
-  placeholder: PropTypes.string,
-  required: PropTypes.bool,
   hiddenFieldId: PropTypes.string,
   scope: PropTypes.string,
   minimumInputLength: PropTypes.number,
@@ -166,7 +184,8 @@ ComboSearch.propTypes = {
   transformResults: PropTypes.func,
   allowInputValues: PropTypes.bool,
   onChange: PropTypes.func,
-  className: PropTypes.string
+  inputId: PropTypes.string,
+  scopeExtra: PropTypes.string
 };
 
 export default ComboSearch;

app/javascript/components/shared/queryClient.js

@@ -26,17 +26,21 @@ export const queryClient = new QueryClient({
   }
 });
 
-function buildURL(scope, term) {
+function buildURL(scope, term, extra) {
   term = encodeURIComponent(term.replace(/\(|\)/g, ''));
   if (scope === 'adresse') {
     return `${api_adresse_url}/search?q=${term}&limit=${API_ADRESSE_QUERY_LIMIT}`;
   } else if (scope === 'annuaire-education') {
     return `${api_education_url}/search?dataset=fr-en-annuaire-education&q=${term}&rows=${API_EDUCATION_QUERY_LIMIT}`;
   } else if (scope === 'communes') {
+    const limit = `limit=${API_GEO_COMMUNES_QUERY_LIMIT}`;
+    const url = extra
+      ? `${api_geo_url}/communes?codeDepartement=${extra}&${limit}&`
+      : `${api_geo_url}/communes?${limit}&`;
     if (isNumeric(term)) {
-      return `${api_geo_url}/communes?codePostal=${term}&limit=${API_GEO_COMMUNES_QUERY_LIMIT}`;
+      return `${url}codePostal=${term}`;
     }
-    return `${api_geo_url}/communes?nom=${term}&boost=population&limit=${API_GEO_COMMUNES_QUERY_LIMIT}`;
+    return `${url}nom=${term}&boost=population`;
   } else if (isNumeric(term)) {
     const code = term.padStart(2, '0');
     return `${api_geo_url}/${scope}?code=${code}&limit=${API_GEO_QUERY_LIMIT}`;
@@ -53,12 +57,12 @@ function buildOptions() {
   return [{}, null];
 }
 
-async function defaultQueryFn({ queryKey: [scope, term] }) {
+async function defaultQueryFn({ queryKey: [scope, term, extra] }) {
   if (scope == 'pays') {
     return matchSorter(await getPays(), term, { keys: ['label'] });
   }
 
-  const url = buildURL(scope, term);
+  const url = buildURL(scope, term, extra);
   const [options, controller] = buildOptions();
   const promise = fetch(url, options).then((response) => {
     if (response.ok) {

app/jobs/virus_scanner_job.rb

@@ -1,15 +1,8 @@
 class VirusScannerJob < ApplicationJob
-  class FileNotAnalyzedYetError < StandardError
-  end
-
-  queue_as :active_storage_analysis
-
   # If by the time the job runs the blob has been deleted, ignore the error
   discard_on ActiveRecord::RecordNotFound
   # If the file is deleted during the scan, ignore the error
   discard_on ActiveStorage::FileNotFoundError
-  # If the file is not analyzed yet, retry later (to avoid clobbering metadata)
-  retry_on FileNotAnalyzedYetError, wait: :exponentially_longer, attempts: 10
   # If for some reason the file appears invalid, retry for a while
   retry_on(ActiveStorage::IntegrityError, attempts: 5, wait: 5.seconds) do |job, _error|
     blob = job.arguments.first
@@ -23,7 +16,6 @@ class VirusScannerJob < ApplicationJob
   end
 
   def perform(blob)
-    if !blob.analyzed? then raise FileNotAnalyzedYetError end
     if blob.virus_scanner.done? then return end
 
     metadata = extract_metadata_via_virus_scanner(blob)

app/lib/active_job/retry_on_transient_errors.rb

@@ -5,7 +5,8 @@ module ActiveJob::RetryOnTransientErrors
     Excon::Error::InternalServerError,
     Excon::Error::GatewayTimeout,
     Excon::Error::BadRequest,
-    Excon::Error::Socket
+    Excon::Error::Socket,
+    ActiveRecord::StaleObjectError
   ]
 
   included do

app/mailers/user_mailer.rb

@@ -12,10 +12,10 @@ class UserMailer < ApplicationMailer
     mail(to: user.email, subject: @subject, procedure: @procedure)
   end
 
-  def account_already_taken(user, requested_email)
+  def ask_for_merge(user, requested_email)
     @user = user
     @requested_email = requested_email
-    @subject = "Changement d’adresse email"
+    @subject = "Fusion de compte"
 
     mail(to: requested_email, subject: @subject)
   end

app/models/user.rb

@@ -28,6 +28,7 @@
 #  administrateur_id            :bigint
 #  expert_id                    :bigint
 #  instructeur_id               :bigint
+#  requested_merge_into_id      :bigint
 #
 class User < ApplicationRecord
   include EmailSanitizableConcern
@@ -48,10 +49,13 @@ class User < ApplicationRecord
   has_many :dossiers_invites, through: :invites, source: :dossier
   has_many :deleted_dossiers
   has_many :merge_logs, dependent: :destroy
+  has_many :requested_merge_from, class_name: 'User', dependent: :nullify, inverse_of: :requested_merge_into, foreign_key: :requested_merge_into_id
+
   has_one :france_connect_information, dependent: :destroy
   belongs_to :instructeur, optional: true, dependent: :destroy
   belongs_to :administrateur, optional: true, dependent: :destroy
   belongs_to :expert, optional: true, dependent: :destroy
+  belongs_to :requested_merge_into, class_name: 'User', optional: true
 
   accepts_nested_attributes_for :france_connect_information
 
@@ -218,6 +222,11 @@ class User < ApplicationRecord
     end
   end
 
+  def ask_for_merge(requested_user)
+    update(requested_merge_into: requested_user)
+    UserMailer.ask_for_merge(self, requested_user.email).deliver_later
+  end
+
   private
 
   def link_invites!

app/views/user_mailer/account_already_taken.haml

@@ -1,20 +0,0 @@
-- content_for(:title, @subject)
-
-%p
-  Bonjour,
-
-%p
-  L’utilisateur « #{@user.email} » a demandé le changement de son adresse vers « #{@requested_email} ».
-
-%p
-  Malheureusement, votre compte « #{@requested_email} » existe déjà. Nous ne pouvons pas fusionner automatiquement vos comptes.
-
-%p
-  %strong Nous ne pouvons donc pas effectuer le changement d’adresse email.
-
-%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"

app/views/user_mailer/ask_for_merge.haml

@@ -0,0 +1,22 @@
+- content_for(:title, @subject)
+
+%p
+  Bonjour,
+
+%p
+  L’utilisateur <b>« #{@user.email} »</b> a demandé la fusion de son compte avec le votre <b>« #{@requested_email} »</b>.
+
+%p
+  Si vous désirez confirmer la fusion de ces comptes :
+
+%ol
+  %li connectez-vous avec le compte #{@requested_email}
+  %li allez sur votre page profil
+  %li acceptez la fusion
+
+%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"

app/views/users/profil/show.html.haml

@@ -5,18 +5,30 @@
 #profil-page.container
   %h1 Profil
 
+  - if @waiting_merge_emails.any?
+    .card
+      .card-title Demande de fusion de comptes
+      %p
+        Acceptez-vous d’absorber le compte de
+        %span.email-address= @waiting_merge_emails.join(', ')
+
+      = link_to 'Refuser la fusion', refuse_merge_path, method: :post, class: 'button', data: { confirm: "Confirmez-vous le refus ?" }
+      = link_to 'Accepter la fusion', accept_merge_path, method: :post, class: 'button', data: { confirm: "Confirmez-vous la fusion des comptes ?" }
+
   .card
     .card-title Coordonnées
     %p
       Votre email est actuellement
       %span.email-address= current_user.email
-    - if current_user.unconfirmed_email.present?
-      .card.warning
-        .card-title
-          Changement en attente :
-          %span.email-address= current_user.unconfirmed_email
-        %p
-          Pour finaliser votre changement d’adresse, vérifiez vos emails et cliquez sur le lien de confirmation.
+
+    - waiting_email = current_user.unconfirmed_email || current_user.requested_merge_into&.email
+
+    - if waiting_email.present?
+      %p.mb-4
+        Changement en attente :
+        %span.email-address= waiting_email
+        %br
+        Pour finaliser votre changement d’adresse, vérifiez vos emails et cliquez sur le lien de confirmation.
 
     - if current_user.instructeur?
       %p.mb-4

config/initializers/lograge.rb

@@ -14,6 +14,7 @@ Rails.application.configure do
         user_email: event.payload[:user_email],
         user_roles: event.payload[:user_roles],
         user_agent: event.payload[:user_agent],
+        graphql_operation: event.payload[:graphql_operation],
         browser: event.payload[:browser],
         browser_version: event.payload[:browser_version],
         platform: event.payload[:platform]

config/locales/views/users/profil/fr.yml

@@ -21,7 +21,7 @@ fr:
           <br>
           Si ce n'est pas votre cas, contactez le support&nbsp;:
           <a href="mailto:%{contact_email}">%{contact_email}</a>
-      update_email:
+      ensure_update_email_is_authorized:
         email_not_allowed: "L’email %{requested_email} ne peut être utilisé, contactez le support : <a href='mailto:%{contact_email}'>%{contact_email}</a>"
       transfer_all_dossiers:
         new_transfer: 

config/routes.rb

@@ -280,6 +280,8 @@ Rails.application.routes.draw do
     get 'renew-api-token' => redirect('/profil')
     patch 'update_email' => 'profil#update_email'
     post 'transfer_all_dossiers' => 'profil#transfer_all_dossiers'
+    post 'accept_merge' => 'profil#accept_merge'
+    post 'refuse_merge' => 'profil#refuse_merge'
   end
 
   #
@@ -467,6 +469,7 @@ Rails.application.routes.draw do
       get 'regions' => 'api_geo_test#regions'
       get 'communes' => 'api_geo_test#communes'
       get 'departements' => 'api_geo_test#departements'
+      get 'departements/:code/communes' => 'api_geo_test#communes'
     end
   end
 

db/migrate/20211026082232_add_requested_merge_into_column_to_users.rb

@@ -0,0 +1,5 @@
+class AddRequestedMergeIntoColumnToUsers < ActiveRecord::Migration[6.1]
+  def change
+    add_reference :users, :requested_merge_into, foreign_key: { to_table: :users }, null: true, index: true
+  end
+end

db/migrate/20211104102349_add_lock_version_to_active_storage_blobs.rb

@@ -0,0 +1,5 @@
+class AddLockVersionToActiveStorageBlobs < ActiveRecord::Migration[6.1]
+  def change
+    add_column :active_storage_blobs, :lock_version, :integer
+  end
+end

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: 2021_10_26_131800) do
+ActiveRecord::Schema.define(version: 2021_11_04_102349) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -45,6 +45,7 @@ ActiveRecord::Schema.define(version: 2021_10_26_131800) do
     t.string "checksum", null: false
     t.datetime "created_at", null: false
     t.string "service_name", null: false
+    t.integer "lock_version"
     t.index ["key"], name: "index_active_storage_blobs_on_key", unique: true
   end
 
@@ -794,11 +795,13 @@ ActiveRecord::Schema.define(version: 2021_10_26_131800) do
     t.bigint "administrateur_id"
     t.bigint "expert_id"
     t.string "locale"
+    t.bigint "requested_merge_into_id"
     t.index ["administrateur_id"], name: "index_users_on_administrateur_id"
     t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true
     t.index ["email"], name: "index_users_on_email", unique: true
     t.index ["expert_id"], name: "index_users_on_expert_id"
     t.index ["instructeur_id"], name: "index_users_on_instructeur_id"
+    t.index ["requested_merge_into_id"], name: "index_users_on_requested_merge_into_id"
     t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true
     t.index ["unlock_token"], name: "index_users_on_unlock_token", unique: true
   end
@@ -868,5 +871,6 @@ ActiveRecord::Schema.define(version: 2021_10_26_131800) do
   add_foreign_key "users", "administrateurs"
   add_foreign_key "users", "experts"
   add_foreign_key "users", "instructeurs"
+  add_foreign_key "users", "users", column: "requested_merge_into_id"
   add_foreign_key "without_continuation_mails", "procedures"
 end

doc/Contributions/Pratiques-de-dev.md

@@ -0,0 +1,103 @@
+# Bonnes pratiques de développement
+
+## Branches du projet
+
+- `main`, branche qui contient le code du site tel qu'il est [en intégration](https://dev.demarches-simplifiees.fr/)
+- `production`, branche qui contient le code du site tel qu'il est [en production](https://www.demarches-simplifiees.fr)
+
+## Cycle de développement
+
+1. Pour chaque ensemble de modifications, créer une branche associée
+  (sur le repo principal si vous avez les droits, sinon sur un fork personnel).
+2. Une fois les modifications faites, créer une Pull Request sur GitHub.
+3. Si un·e relectrice·eur ne s'est pas manifesté·e au bout de 48 h, relancer en mettant un message dans la PR.
+4. Quand la Pull Request a été relue et validée :
+    1. le contributeur rebase la branche par rapport à `main` (manuellement, ou en mettant un commentaire "/rebase" dans la PR),
+    2. le mainteneur merge la PR.
+
+## Pour une première contribution
+
+Si c’est votre première contribution, commencez par une toute petite Pull Request (PR), par exemple de nettoyage,
+pour vous faire la main sur le processus.
+
+## Bonnes pratiques : sur le code
+
+### Tests
+
+- Le code doit être testé, la PR doit contenir les tests (toute PR sans test sera a priori rejetée).
+
+### Code mort / code propre
+
+- Une contribution ne devrait pas comporter de code mort (enlever le code commenté ou jamais appelé).
+
+### Injection de dépendance
+
+D'une manière générale, nous préférons avoir des controlleurs verbeux mais explicites.
+Afin d'éviter de trop alourdir les contrôleurs et les modèles, déjà bien chargés, nous mettons parfois en place des services pour centraliser une partie cohérente des traitements (voir [DossierProjectionService](https://github.com/betagouv/demarches-simplifiees.fr/blob/92f463bc039200b98908dc5c09366844b0e1d593/app/services/dossier_projection_service.rb), [PieceJustificativeService](https://github.com/betagouv/demarches-simplifiees.fr/blob/92f463bc039200b98908dc5c09366844b0e1d593/app/services/pieces_justificatives_service.rb))
+
+- Toute injection de dépendance doit être utilisée (sinon ne pas la coder)
+- Il est demandé d'éviter l'injection de dépendance dans les constructeurs.
+  (Une exception pourrait être prise en compte si deux implémentations différentes, hors tests, sont injectées.)
+- On mock directement les dépendances concernées (parce que ruby, c'est magique)
+
+```ruby
+# Non
+def initializer(http_service)
+ @http_service = http_service
+end
+
+def get
+  @http_service.get(url)
+end
+
+# Oui
+def get
+  Typhoeus.get(url)
+end
+
+# spec
+
+expect(Service).to receive(:do_stuff).and_return(true)
+## ou même
+expect_any_instance_of(Procedure).to receive(:procedure_overview).and_return(procedure_overview)
+```
+
+## Bonnes pratiques : sur les PR
+
+- Toujours mettre un message décrivant l'objet de la PR
+- Si la PR concerne des changements visuels, mettre une capture d'écran
+- Faites de petites PR. Si la PR est trop grosse (> 400 lignes), découpez-la en plusieurs PR. Chaque petite PR doit
+  essayer d'apporter de la valeur au client, d'apporter une petite fonctionnalité.
+- Le relecteur d'une PR peut prendre la main (après l'avoir demandé) pour faire les modifs de formes
+- Les modifications de formes non détectées automatiquement sont optionnelles
+- Néanmoins, les commits de nettoyage sont autorisés au sein de ces petites PRs
+- Les remarques sur l’amélioration du code ne concernant pas directement la PR sont optionnelles
+
+En date du 2021-10-19, voici une PR servant d'exemple :
+
+* [#6519 ETQ Super Admin je veux changer le mail d'un instructeur](https://github.com/betagouv/demarches-simplifiees.fr/pull/6519)
+
+## Bonnes pratiques : sur les branches
+
+- Donner un nom descriptif à ses branches
+- Faire une branche par "sujet", ne pas faire de branches trop lourdes
+
+**Attention** : Ne **pas utiliser le bouton "Update branch"** de GitHub.
+
+Ce bouton merge `main` dans la feature branch – ce qui casse l'historique semi-linéraire. (Nous, ce qu'on voudrait, c'est rebaser).
+À la place, rebaser manuellement la feature-branch sur `main` (ou mettre un commentaire "/rebase" dans la PR).
+
+## Bonnes pratiques : sur les commits
+
+- Les messages de commit sont écrits en anglais
+- Faire des petits commits, les plus unitaires possible, homogènes et en essayant de ne pas mélanger les sujets.
+- Les commits correctifs sont à "fixup-er" dans les commits qu'ils corrigent
+- Séparer les modifications relatives à du nettoyage dans un commit séparé, voire une PR séparée
+- Dans le cas où un commit corrige un bug ou implémente une feature, mentionner dans le message de commit le numéro de l'issue avec `Fix #XXXX` ou `Ref #XXXX`
+
+Exemple d'une série de commits :
+
+- un commit pour du renommage,
+- un commit pour un ajout de méthode + test,
+- un commit pour l'interface utilisateur
+

spec/controllers/users/profil_controller_spec.rb

@@ -49,12 +49,16 @@ describe Users::ProfilController, type: :controller do
 
   describe 'PATCH #update_email' do
     context 'when everything is fine' do
+      let(:previous_request) { create(:user) }
+
       before do
+        user.update(requested_merge_into: previous_request)
         patch :update_email, params: { user: { email: 'loulou@lou.com' } }
         user.reload
       end
 
       it { expect(user.unconfirmed_email).to eq('loulou@lou.com') }
+      it { expect(user.requested_merge_into).to be_nil }
       it { expect(response).to redirect_to(profil_path) }
       it { expect(flash.notice).to eq(I18n.t('devise.registrations.update_needs_confirmation')) }
     end
@@ -63,16 +67,21 @@ describe Users::ProfilController, type: :controller do
       let(:existing_user) { create(:user) }
 
       before do
+        user.update(unconfirmed_email: 'unconfirmed@mail.com')
+
+        expect_any_instance_of(User).to receive(:ask_for_merge).with(existing_user)
+
         perform_enqueued_jobs do
           patch :update_email, params: { user: { email: existing_user.email } }
         end
         user.reload
       end
 
-      it { expect(user.unconfirmed_email).to be_nil }
-      it { expect(ActionMailer::Base.deliveries.last.to).to eq([existing_user.email]) }
-      it { expect(response).to redirect_to(profil_path) }
-      it { expect(flash.notice).to eq(I18n.t('devise.registrations.update_needs_confirmation')) }
+      it 'launches the merge process' do
+        expect(user.unconfirmed_email).to be_nil
+        expect(response).to redirect_to(profil_path)
+        expect(flash.notice).to eq(I18n.t('devise.registrations.update_needs_confirmation'))
+      end
     end
 
     context 'when the mail is incorrect' do
@@ -126,4 +135,38 @@ describe Users::ProfilController, type: :controller do
       expect(flash.notice).to eq("Le transfert de 3 dossiers à #{next_owner} est en cours")
     end
   end
+
+  context 'POST #accept_merge' do
+    let!(:requesting_user) { create(:user, requested_merge_into: user) }
+
+    subject { post :accept_merge }
+
+    it 'merges the account' do
+      expect_any_instance_of(User).to receive(:merge)
+
+      subject
+      requesting_user.reload
+
+      expect(requesting_user.requested_merge_into).to be_nil
+      expect(flash.notice).to include('Vous avez absorbé')
+      expect(response).to redirect_to(profil_path)
+    end
+  end
+
+  context 'POST #refuse_merge' do
+    let!(:requesting_user) { create(:user, requested_merge_into: user) }
+
+    subject { post :refuse_merge }
+
+    it 'merges the account' do
+      expect_any_instance_of(User).not_to receive(:merge)
+
+      subject
+      requesting_user.reload
+
+      expect(requesting_user.requested_merge_into).to be_nil
+      expect(flash.notice).to include('La fusion a été refusé')
+      expect(response).to redirect_to(profil_path)
+    end
+  end
 end

spec/jobs/virus_scanner_job_spec.rb

@@ -7,57 +7,67 @@ describe VirusScannerJob, type: :job do
     VirusScannerJob.perform_now(blob)
   end
 
-  context "when the blob is not analyzed yet" do
-    it "retries the job later" do
-      expect { subject }.to have_enqueued_job(VirusScannerJob)
+  context "when the virus scan launch before rails analyze" do
+    before do
+      allow(ClamavService).to receive(:safe_file?).and_return(true)
+      subject
+      blob.analyze
+    end
+    it { expect(blob.virus_scanner.safe?).to be_truthy }
+    it { expect(blob.analyzed?).to be_truthy }
+    it { expect(blob.lock_version).to eq(2) }
+  end
+
+  context "should raise ActiveRecord::StaleObjectError" do
+    let(:blob_2) { ActiveStorage::Blob.find(blob.id) }
+    before do
+      blob_2.metadata[:virus_scan_result] = "infected"
+      blob.metadata[:virus_scan_result] = "safe"
+      blob.save
+    end
+
+    it { expect { blob_2.save }.to raise_error(ActiveRecord::StaleObjectError) }
+  end
+
+  context "when there is an integrity error" do
+    before do
+      blob.update_column('checksum', 'integrity error')
+
+      assert_performed_jobs(5) do
+        VirusScannerJob.perform_later(blob)
+      end
+    end
+
+    it do
+      expect(blob.reload.virus_scanner.corrupt?).to be_truthy
     end
   end
 
-  context "when the blob has been analyzed" do
+  context "when no virus is found" do
     before do
-      blob.analyze
+      allow(ClamavService).to receive(:safe_file?).and_return(true)
+      subject
     end
 
-    context "when there is an integrity error" do
-      before do
-        blob.update_column('checksum', 'integrity error')
+    it { expect(blob.virus_scanner.safe?).to be_truthy }
+  end
 
-        assert_performed_jobs(5) do
-          VirusScannerJob.perform_later(blob)
-        end
-      end
-
-      it do
-        expect(blob.reload.virus_scanner.corrupt?).to be_truthy
-      end
+  context "when a virus is found" do
+    before do
+      allow(ClamavService).to receive(:safe_file?).and_return(false)
+      subject
     end
 
-    context "when no virus is found" do
-      before do
-        allow(ClamavService).to receive(:safe_file?).and_return(true)
-        subject
-      end
+    it { expect(blob.virus_scanner.infected?).to be_truthy }
+  end
 
-      it { expect(blob.virus_scanner.safe?).to be_truthy }
+  context "when the blob has been deleted" do
+    before do
+      ActiveStorage::Blob.find(blob.id).purge
     end
 
-    context "when a virus is found" do
-      before do
-        allow(ClamavService).to receive(:safe_file?).and_return(false)
-        subject
-      end
-
-      it { expect(blob.virus_scanner.infected?).to be_truthy }
-    end
-
-    context "when the blob has been deleted" do
-      before do
-        ActiveStorage::Blob.find(blob.id).purge
-      end
-
-      it "ignores the error" do
-        expect { subject }.not_to raise_error
-      end
+    it "ignores the error" do
+      expect { subject }.not_to raise_error
     end
   end
 end

spec/mailers/previews/user_mailer_preview.rb

@@ -8,8 +8,8 @@ class UserMailerPreview < ActionMailer::Preview
     UserMailer.new_account_warning(user, procedure)
   end
 
-  def account_already_taken
-    UserMailer.account_already_taken(user, 'dircab@territoires.gouv.fr')
+  def ask_for_merge
+    UserMailer.ask_for_merge(user, 'dircab@territoires.gouv.fr')
   end
 
   private

spec/mailers/user_mailer_spec.rb

@@ -17,10 +17,10 @@ RSpec.describe UserMailer, type: :mailer do
     end
   end
 
-  describe '.account_already_taken' do
+  describe '.ask_for_merge' do
     let(:requested_email) { 'new@exemple.fr' }
 
-    subject { described_class.account_already_taken(user, requested_email) }
+    subject { described_class.ask_for_merge(user, requested_email) }
 
     it { expect(subject.to).to eq([requested_email]) }
     it { expect(subject.body).to include(requested_email) }

spec/support/system_helpers.rb

@@ -104,13 +104,11 @@ module SystemHelpers
   end
 
   def select_combobox(champ, fill_with, value)
-    input = find("input[aria-label=\"#{champ}\"")
-    input.click
-    input.fill_in with: fill_with
+    fill_in champ, with: fill_with
     selector = "li[data-option-value=\"#{value}\"]"
     find(selector).click
     expect(page).to have_css(selector)
-    expect(page).to have_hidden_field(champ, with: value)
+    expect(page).to have_css("[type=\"hidden\"][value=\"#{value}\"]")
   end
 
   def select_multi_combobox(champ, fill_with, value)

spec/system/users/brouillon_spec.rb

@@ -32,6 +32,7 @@ describe 'The user' do
     select_combobox('pays', 'aust', 'Australie')
     select_combobox('regions', 'Ma', 'Martinique')
     select_combobox('departements', 'Ai', '02 - Aisne')
+    select_combobox('communes', 'Ai', '02 - Aisne')
     select_combobox('communes', 'Ambl', 'Ambléon (01300)')
 
     check('engagement')

spec/system/users/change_email_spec.rb

@@ -29,3 +29,36 @@ describe 'Changing an email' do
     expect(user.reload.email).to eq(new_email)
   end
 end
+
+describe 'Merging account' do
+  let(:old_user) { create(:user) }
+  let(:new_user) { create(:user) }
+
+  before do
+    login_as old_user, scope: :user
+  end
+
+  scenario 'is easy' do
+    visit '/profil'
+
+    fill_in :user_email, with: new_user.email
+
+    perform_enqueued_jobs do
+      click_button 'Changer mon adresse'
+    end
+
+    expect(page).to have_content(I18n.t('devise.registrations.update_needs_confirmation'))
+    expect(page).to have_content(old_user.email)
+    expect(page).to have_content(new_user.email)
+
+    login_as new_user, scope: :user
+    visit '/profil'
+
+    expect(page).to have_content("Acceptez-vous d’absorber le compte de #{old_user.email}")
+    click_on 'Accepter la fusion'
+
+    expect(page).not_to have_content(old_user.email)
+    expect(page).to have_content(new_user.email)
+    expect { old_user.reload }.to raise_error(ActiveRecord::RecordNotFound)
+  end
+end

yarn.lock

@@ -3901,9 +3901,9 @@ color-name@^1.0.0, color-name@~1.1.4:
   integrity sha512-dOy+3AuW3a2wNbZHIuMZpTcgjGuLU/uBL/ubcZF9OXbDo8ff4O8yVp5Bf0efS8uEoYo5q4Fx7dY9OgQGXgAsQA==
 
 color-string@^1.5.2:
-  version "1.5.3"
-  resolved "https://registry.yarnpkg.com/color-string/-/color-string-1.5.3.tgz#c9bbc5f01b58b5492f3d6857459cb6590ce204cc"
-  integrity sha512-dC2C5qeWoYkxki5UAXapdjqO672AM4vZuPGRQfO8b5HKuKGBbKWpITyDYN7TOFKvRW7kOgAn3746clDBMDJyQw==
+  version "1.6.0"
+  resolved "https://registry.yarnpkg.com/color-string/-/color-string-1.6.0.tgz#c3915f61fe267672cb7e1e064c9d692219f6c312"
+  integrity sha512-c/hGS+kRWJutUBEngKKmk4iH3sD59MBkoxVapS/0wgpCz2u7XsNloxknyvBhzwEs1IbV36D9PwqLPJ2DTu3vMA==
   dependencies:
     color-name "^1.0.0"
     simple-swizzle "^0.2.2"
@@ -12057,9 +12057,9 @@ strip-outer@^1.0.0:
     escape-string-regexp "^1.0.2"
 
 striptags@^3.0.1:
-  version "3.1.1"
-  resolved "https://registry.yarnpkg.com/striptags/-/striptags-3.1.1.tgz#c8c3e7fdd6fb4bb3a32a3b752e5b5e3e38093ebd"
-  integrity sha1-yMPn/db7S7OjKjt1LltePjgJPr0=
+  version "3.2.0"
+  resolved "https://registry.yarnpkg.com/striptags/-/striptags-3.2.0.tgz#cc74a137db2de8b0b9a370006334161f7dd67052"
+  integrity sha512-g45ZOGzHDMe2bdYMdIvdAfCQkCTDMGBazSw1ypMowwGIee7ZQ5dU0rBJ8Jqgl+jAKIv4dbeE1jscZq9wid1Tkw==
 
 style-loader@^1.3.0:
   version "1.3.0"