From 59b0f3961d96481531c08de9e68efa43134ab1f6 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Wed, 9 Mar 2022 10:25:48 +0100 Subject: [PATCH 1/4] refactor(dossier): improuve dossiers_count_summary --- app/models/instructeur.rb | 32 +++++++-------- .../procedures/_download_dossiers.html.haml | 41 +++++++++---------- .../procedures/download_export.js.erb | 4 +- .../instructeurs/procedures/show.html.haml | 3 +- 4 files changed, 41 insertions(+), 39 deletions(-) diff --git a/app/models/instructeur.rb b/app/models/instructeur.rb index 9c5d07a2b..ed870b3e3 100644 --- a/app/models/instructeur.rb +++ b/app/models/instructeur.rb @@ -231,14 +231,13 @@ class Instructeur < ApplicationRecord def dossiers_count_summary(groupe_instructeur_ids) query = <<~EOF SELECT - COUNT(DISTINCT dossiers.id) FILTER (where not archived AND NOT (dossiers.hidden_by_user_at IS NOT NULL AND state = 'en_construction') AND dossiers.state in ('en_construction', 'en_instruction') AND follows.id IS NULL) AS a_suivre, - COUNT(DISTINCT dossiers.id) FILTER (where not archived AND dossiers.state in ('en_construction', 'en_instruction') AND follows.instructeur_id = :instructeur_id) AS suivis, - COUNT(DISTINCT dossiers.id) FILTER (where not archived AND dossiers.state in ('accepte', 'refuse', 'sans_suite')) AS traites, - COUNT(DISTINCT dossiers.id) FILTER (where not archived AND NOT (dossiers.hidden_by_user_at IS NOT NULL AND state = 'en_construction') AND NOT (dossiers.hidden_by_administration_at IS NOT NULL)) AS tous, - COUNT(DISTINCT dossiers.id) FILTER (where not archived AND (dossiers.hidden_by_administration_at IS NOT NULL AND dossiers.state in ('accepte', 'refuse', 'sans_suite') )) AS supprimes_recemment, - COUNT(DISTINCT dossiers.id) FILTER (where archived) AS archives, - COUNT(DISTINCT dossiers.id) FILTER (where - procedures.procedure_expires_when_termine_enabled + COUNT(DISTINCT dossiers.id) FILTER (where dossiers.hidden_by_administration_at IS NULL AND not archived AND dossiers.state in ('en_construction', 'en_instruction') AND follows.id IS NULL) AS a_suivre, + COUNT(DISTINCT dossiers.id) FILTER (where dossiers.hidden_by_administration_at IS NULL AND not archived AND dossiers.state in ('en_construction', 'en_instruction') AND follows.instructeur_id = :instructeur_id) AS suivis, + COUNT(DISTINCT dossiers.id) FILTER (where dossiers.hidden_by_administration_at IS NULL AND not archived AND dossiers.state in ('accepte', 'refuse', 'sans_suite')) AS traites, + COUNT(DISTINCT dossiers.id) FILTER (where dossiers.hidden_by_administration_at IS NULL AND not archived) AS tous, + COUNT(DISTINCT dossiers.id) FILTER (where dossiers.hidden_by_administration_at IS NULL AND archived) AS archives, + COUNT(DISTINCT dossiers.id) FILTER (where dossiers.hidden_by_administration_at IS NOT NULL AND not archived AND dossiers.state in ('accepte', 'refuse', 'sans_suite')) AS supprimes_recemment, + COUNT(DISTINCT dossiers.id) FILTER (where dossiers.hidden_by_administration_at IS NULL AND procedures.procedure_expires_when_termine_enabled AND ( dossiers.state in ('accepte', 'refuse', 'sans_suite') AND dossiers.processed_at + dossiers.conservation_extension + (procedures.duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now @@ -247,17 +246,18 @@ class Instructeur < ApplicationRecord AND dossiers.en_construction_at + dossiers.conservation_extension + (duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now ) ) AS expirant - FROM "dossiers" - INNER JOIN "procedure_revisions" - ON "procedure_revisions"."id" = "dossiers"."revision_id" - INNER JOIN "procedures" - ON "procedures"."id" = "procedure_revisions"."procedure_id" + FROM dossiers + INNER JOIN procedure_revisions + ON procedure_revisions.id = dossiers.revision_id + INNER JOIN procedures + ON procedures.id = procedure_revisions.procedure_id LEFT OUTER JOIN follows ON follows.dossier_id = dossiers.id AND follows.unfollowed_at IS NULL - WHERE "dossiers"."hidden_at" IS NULL - AND "dossiers"."state" != 'brouillon' - AND "dossiers"."groupe_instructeur_id" in (:groupe_instructeur_ids) + WHERE dossiers.state != 'brouillon' + AND dossiers.groupe_instructeur_id in (:groupe_instructeur_ids) + AND dossiers.hidden_at IS NULL + AND NOT (dossiers.hidden_by_user_at IS NOT NULL AND dossiers.state = 'en_construction') EOF sanitized_query = ActiveRecord::Base.sanitize_sql([ diff --git a/app/views/instructeurs/procedures/_download_dossiers.html.haml b/app/views/instructeurs/procedures/_download_dossiers.html.haml index 454cfd7e6..9b813cd3a 100644 --- a/app/views/instructeurs/procedures/_download_dossiers.html.haml +++ b/app/views/instructeurs/procedures/_download_dossiers.html.haml @@ -1,21 +1,20 @@ -- if dossier_count > 0 - %span.dropdown - %button.button.dropdown-button{ 'aria-expanded' => 'false', 'aria-controls' => 'download-menu' } - Télécharger tous les dossiers - #download-menu.dropdown-content.fade-in-down{ style: 'width: 450px' } - %ul.dropdown-items - - exports_list(exports).each do |(format, time_span_type, export)| - %li - - if export.nil? - = link_to t("#{time_span_type}_#{format}_html", scope: [:instructeurs, :procedure, :export_stale]), download_export_instructeur_procedure_path(procedure, time_span_type: time_span_type, export_format: format), remote: true - - elsif export.ready? - = link_to t("export_#{time_span_type}_ready_html", export_time: time_ago_in_words(export.updated_at), export_format: ".#{format}", scope: [:instructeurs, :procedure]), export.file.service_url, target: "_blank", rel: "noopener" - - if export.old? - = button_to download_export_instructeur_procedure_path(procedure, export_format: format, time_span_type: time_span_type, force_export: true), class: "button small", style: "padding-right: 2px", title: t("#{time_span_type}_short", export_format: ".#{format}", scope: [:instructeurs, :procedure, :export_stale]), remote: true, method: :get, params: { export_format: format, time_span_type: time_span_type, force_export: true } do - .icon.retry - - else - %span{ 'data-export-poll-url': download_export_instructeur_procedure_path(procedure, export_format: format, no_progress_notification: true) } - = t("export_#{time_span_type}_pending_html", export_time: time_ago_in_words(export.created_at), export_format: ".#{format}", scope: [:instructeurs, :procedure]) - - if procedure.feature_enabled?(:archive_zip_globale) - %li - = link_to t(:download_archive, scope: [:instructeurs, :procedure]), instructeur_archives_path(procedure) +%span.dropdown + %button.button.dropdown-button{ 'aria-expanded' => 'false', 'aria-controls' => 'download-menu' } + Télécharger tous les dossiers + #download-menu.dropdown-content.fade-in-down{ style: 'width: 450px' } + %ul.dropdown-items + - exports_list(exports).each do |(format, time_span_type, export)| + %li + - if export.nil? + = link_to t("#{time_span_type}_#{format}_html", scope: [:instructeurs, :procedure, :export_stale]), download_export_instructeur_procedure_path(procedure, time_span_type: time_span_type, export_format: format), remote: true + - elsif export.ready? + = link_to t("export_#{time_span_type}_ready_html", export_time: time_ago_in_words(export.updated_at), export_format: ".#{format}", scope: [:instructeurs, :procedure]), export.file.service_url, target: "_blank", rel: "noopener" + - if export.old? + = button_to download_export_instructeur_procedure_path(procedure, export_format: format, time_span_type: time_span_type, force_export: true), class: "button small", style: "padding-right: 2px", title: t("#{time_span_type}_short", export_format: ".#{format}", scope: [:instructeurs, :procedure, :export_stale]), remote: true, method: :get, params: { export_format: format, time_span_type: time_span_type, force_export: true } do + .icon.retry + - else + %span{ 'data-export-poll-url': download_export_instructeur_procedure_path(procedure, export_format: format, no_progress_notification: true) } + = t("export_#{time_span_type}_pending_html", export_time: time_ago_in_words(export.created_at), export_format: ".#{format}", scope: [:instructeurs, :procedure]) + - if procedure.feature_enabled?(:archive_zip_globale) + %li + = link_to t(:download_archive, scope: [:instructeurs, :procedure]), instructeur_archives_path(procedure) diff --git a/app/views/instructeurs/procedures/download_export.js.erb b/app/views/instructeurs/procedures/download_export.js.erb index c7da3afca..aab5dbf9f 100644 --- a/app/views/instructeurs/procedures/download_export.js.erb +++ b/app/views/instructeurs/procedures/download_export.js.erb @@ -1,4 +1,6 @@ -<%= render_to_element('.procedure-actions', partial: "download_dossiers", locals: { procedure: @procedure, exports: @exports, dossier_count: @dossier_count }) %> +<% if @can_download_dossiers %> + <%= render_to_element('.procedure-actions', partial: "download_dossiers", locals: { procedure: @procedure, exports: @exports }) %> +<% end %> <% @exports.each do |format, exports| %> <% exports.each do |time_span_type, export| %> diff --git a/app/views/instructeurs/procedures/show.html.haml b/app/views/instructeurs/procedures/show.html.haml index 6734bd8de..334d0b4bb 100644 --- a/app/views/instructeurs/procedures/show.html.haml +++ b/app/views/instructeurs/procedures/show.html.haml @@ -10,7 +10,8 @@ = render partial: 'header', locals: { procedure: @procedure, statut: @statut } .procedure-actions - = render partial: "download_dossiers", locals: { procedure: @procedure, exports: @exports, dossier_count: @tous_count + @archives_count } + - if @can_download_dossiers + = render partial: "download_dossiers", locals: { procedure: @procedure, exports: @exports } .container.flex= render partial: "tabs", locals: { procedure: @procedure, statut: @statut, a_suivre_count: @a_suivre_count, From 54b559364a3b2a89e5e345efdc1befd283ab4715 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Wed, 9 Mar 2022 10:27:43 +0100 Subject: [PATCH 2/4] feat(dossier): replace discarded with visible_by_administration --- .../groupe_instructeurs_controller.rb | 4 +- .../instructeurs/dossiers_controller.rb | 3 +- .../instructeurs/procedures_controller.rb | 36 +++-- app/controllers/invites_controller.rb | 2 +- .../manager/dossiers_controller.rb | 4 +- app/controllers/users/dossiers_controller.rb | 40 +++-- app/graphql/types/query_type.rb | 4 +- app/mailers/avis_mailer.rb | 2 +- app/models/avis.rb | 4 +- app/models/champ.rb | 2 +- app/models/deleted_dossier.rb | 2 + app/models/dossier.rb | 147 ++++++------------ app/models/dossier_operation_log.rb | 5 +- app/models/dossier_transfer.rb | 6 +- app/models/instructeur.rb | 3 +- app/models/invite.rb | 9 +- app/models/procedure.rb | 6 +- app/models/user.rb | 26 +++- app/services/dossier_search_service.rb | 6 +- app/views/manager/dossiers/show.html.erb | 2 +- 20 files changed, 149 insertions(+), 164 deletions(-) diff --git a/app/controllers/administrateurs/groupe_instructeurs_controller.rb b/app/controllers/administrateurs/groupe_instructeurs_controller.rb index 1ffbb406b..80b651dab 100644 --- a/app/controllers/administrateurs/groupe_instructeurs_controller.rb +++ b/app/controllers/administrateurs/groupe_instructeurs_controller.rb @@ -63,7 +63,7 @@ module Administrateurs end def destroy - if !groupe_instructeur.dossiers.with_discarded.empty? + if !groupe_instructeur.dossiers.empty? flash[:alert] = "Impossible de supprimer un groupe avec des dossiers. Il faut le réaffecter avant" elsif procedure.groupe_instructeurs.one? flash[:alert] = "Suppression impossible : il doit y avoir au moins un groupe instructeur sur chaque procédure" @@ -95,7 +95,7 @@ module Administrateurs def reaffecter target_group = procedure.groupe_instructeurs.find(params[:target_group]) reaffecter_bulk_messages(target_group) - groupe_instructeur.dossiers.with_discarded.find_each do |dossier| + groupe_instructeur.dossiers.find_each do |dossier| dossier.assign_to_groupe_instructeur(target_group, current_administrateur) end diff --git a/app/controllers/instructeurs/dossiers_controller.rb b/app/controllers/instructeurs/dossiers_controller.rb index 78a4c3db7..39d7554f6 100644 --- a/app/controllers/instructeurs/dossiers_controller.rb +++ b/app/controllers/instructeurs/dossiers_controller.rb @@ -228,7 +228,7 @@ module Instructeurs def delete_dossier if dossier.termine? - dossier.discard_and_keep_track!(current_instructeur, :instructeur_request) + dossier.hide_and_keep_track!(current_instructeur, :instructeur_request) flash.notice = t('instructeurs.dossiers.deleted_by_instructeur') redirect_to instructeur_procedure_path(procedure) else @@ -242,6 +242,7 @@ module Instructeurs def dossier @dossier ||= current_instructeur .dossiers + .visible_by_administration .includes(champs: :type_de_champ) .find(params[:dossier_id]) end diff --git a/app/controllers/instructeurs/procedures_controller.rb b/app/controllers/instructeurs/procedures_controller.rb index 921699168..25e7b3ad3 100644 --- a/app/controllers/instructeurs/procedures_controller.rb +++ b/app/controllers/instructeurs/procedures_controller.rb @@ -13,19 +13,21 @@ module Instructeurs .order(closed_at: :desc, unpublished_at: :desc, published_at: :desc, created_at: :desc) dossiers = current_instructeur.dossiers.joins(:groupe_instructeur) - @dossiers_count_per_procedure = dossiers.all_state.visible_by_administration.group('groupe_instructeurs.procedure_id').reorder(nil).count - @dossiers_a_suivre_count_per_procedure = dossiers.without_followers.en_cours.visible_by_administration.group('groupe_instructeurs.procedure_id').reorder(nil).count - @dossiers_archived_count_per_procedure = dossiers.archived.group('groupe_instructeurs.procedure_id').count - @dossiers_termines_count_per_procedure = dossiers.termine.visible_by_administration.group('groupe_instructeurs.procedure_id').reorder(nil).count - @dossiers_expirant_count_per_procedure = dossiers.termine_or_en_construction_close_to_expiration.group('groupe_instructeurs.procedure_id').count + dossiers_visibles = dossiers.visible_by_administration + @dossiers_count_per_procedure = dossiers_visibles.all_state.group('groupe_instructeurs.procedure_id').reorder(nil).count + @dossiers_a_suivre_count_per_procedure = dossiers_visibles.without_followers.en_cours.group('groupe_instructeurs.procedure_id').reorder(nil).count + @dossiers_archived_count_per_procedure = dossiers_visibles.archived.group('groupe_instructeurs.procedure_id').count + @dossiers_termines_count_per_procedure = dossiers_visibles.termine.group('groupe_instructeurs.procedure_id').reorder(nil).count + @dossiers_expirant_count_per_procedure = dossiers_visibles.termine_or_en_construction_close_to_expiration.group('groupe_instructeurs.procedure_id').count @dossiers_supprimes_recemment_count_per_procedure = dossiers.hidden_by_administration.group('groupe_instructeurs.procedure_id').reorder(nil).count - groupe_ids = current_instructeur.groupe_instructeurs.pluck(:id) + groupe_ids = current_instructeur.groupe_instructeurs.pluck(:id) @followed_dossiers_count_per_procedure = current_instructeur .followed_dossiers .joins(:groupe_instructeur) .en_cours .where(groupe_instructeur_id: groupe_ids) + .visible_by_administration .group('groupe_instructeurs.procedure_id') .reorder(nil) .count @@ -56,27 +58,27 @@ module Instructeurs @a_suivre_count, @suivis_count, @traites_count, @tous_count, @supprimes_recemment_count, @archives_count, @expirant_count = current_instructeur .dossiers_count_summary(groupe_instructeur_ids) .fetch_values('a_suivre', 'suivis', 'traites', 'tous', 'supprimes_recemment', 'archives', 'expirant') + @can_download_dossiers = (@tous_count + @archives_count) > 0 - dossiers_visibles = Dossier - .where(groupe_instructeur_id: groupe_instructeur_ids) + dossiers = Dossier.where(groupe_instructeur_id: groupe_instructeur_ids) + dossiers_visibles = dossiers.visible_by_administration @a_suivre_dossiers = dossiers_visibles .without_followers .en_cours - .visible_by_administration @followed_dossiers = current_instructeur .followed_dossiers - .where(groupe_instructeur_id: groupe_instructeur_ids) .en_cours + .merge(dossiers_visibles) @followed_dossiers_id = @followed_dossiers.pluck(:id) - @termines_dossiers = dossiers_visibles.termine.visible_by_administration - @all_state_dossiers = dossiers_visibles.all_state.visible_by_administration - @supprimes_recemment_dossiers = dossiers_visibles.termine.hidden_by_administration + @termines_dossiers = dossiers_visibles.termine + @all_state_dossiers = dossiers_visibles.all_state @archived_dossiers = dossiers_visibles.archived @expirant_dossiers = dossiers_visibles.termine_or_en_construction_close_to_expiration + @supprimes_recemment_dossiers = dossiers.hidden_by_administration.termine @dossiers = case statut when 'a-suivre' @@ -178,10 +180,10 @@ module Instructeurs .groupe_instructeurs .where(procedure: procedure) - @dossier_count = current_instructeur - .dossiers_count_summary(groupe_instructeur_ids) - .fetch_values('tous', 'archives') - .sum + @can_download_dossiers = current_instructeur + .dossiers + .visible_by_administration + .exists?(groupe_instructeur_id: groupe_instructeur_ids) export = Export.find_or_create_export(export_format, time_span_type, groupe_instructeurs) diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb index 54ffb87ef..fac22655b 100644 --- a/app/controllers/invites_controller.rb +++ b/app/controllers/invites_controller.rb @@ -4,7 +4,7 @@ class InvitesController < ApplicationController def create email = params[:invite_email].downcase - dossier = current_user.dossiers.find(params[:dossier_id]) + dossier = current_user.dossiers.visible_by_user.find(params[:dossier_id]) invite = Invite.create( dossier: dossier, diff --git a/app/controllers/manager/dossiers_controller.rb b/app/controllers/manager/dossiers_controller.rb index 84d2dda86..98fb23705 100644 --- a/app/controllers/manager/dossiers_controller.rb +++ b/app/controllers/manager/dossiers_controller.rb @@ -9,10 +9,10 @@ module Manager def scoped_resource if unfiltered_list? # Don't display discarded dossiers in the unfiltered list… - Dossier.kept + Dossier.visible_by_administration else # … but allow them to be searched and displayed. - Dossier.with_discarded + Dossier end end diff --git a/app/controllers/users/dossiers_controller.rb b/app/controllers/users/dossiers_controller.rb index c18215983..f352bc4d2 100644 --- a/app/controllers/users/dossiers_controller.rb +++ b/app/controllers/users/dossiers_controller.rb @@ -5,10 +5,12 @@ module Users layout 'procedure_context', only: [:identite, :update_identite, :siret, :update_siret] ACTIONS_ALLOWED_TO_ANY_USER = [:index, :recherche, :new, :transferer_all] - ACTIONS_ALLOWED_TO_OWNER_OR_INVITE = [:show, :demande, :messagerie, :brouillon, :update_brouillon, :modifier, :update, :create_commentaire, :restore] + ACTIONS_ALLOWED_TO_OWNER_OR_INVITE = [:show, :demande, :messagerie, :brouillon, :update_brouillon, :modifier, :update, :create_commentaire] + ACTIONS_ALLOWED_TO_OWNER_OR_INVITE_HIDDEN = [:restore] - before_action :ensure_ownership!, except: ACTIONS_ALLOWED_TO_ANY_USER + ACTIONS_ALLOWED_TO_OWNER_OR_INVITE + before_action :ensure_ownership!, except: ACTIONS_ALLOWED_TO_ANY_USER + ACTIONS_ALLOWED_TO_OWNER_OR_INVITE + ACTIONS_ALLOWED_TO_OWNER_OR_INVITE_HIDDEN before_action :ensure_ownership_or_invitation!, only: ACTIONS_ALLOWED_TO_OWNER_OR_INVITE + before_action :ensure_ownership_or_invitation_hidden!, only: ACTIONS_ALLOWED_TO_OWNER_OR_INVITE_HIDDEN before_action :ensure_dossier_can_be_updated, only: [:update_identite, :update_brouillon, :modifier, :update] before_action :forbid_invite_submission!, only: [:update_brouillon] before_action :forbid_closed_submission!, only: [:update_brouillon] @@ -16,17 +18,20 @@ module Users before_action :store_user_location!, only: :new def index - @user_dossiers = current_user.dossiers.includes(:procedure).state_not_termine.visible_by_user.order_by_updated_at.page(page) - @dossiers_traites = current_user.dossiers.includes(:procedure).state_termine.visible_by_user.order_by_updated_at.page(page) - @dossiers_invites = current_user.dossiers_invites.includes(:procedure).order_by_updated_at.page(page) - @dossiers_supprimes_recemment = current_user.dossiers.hidden_by_user.order_by_updated_at.page(page) + dossiers = Dossier.includes(:procedure).order_by_updated_at.page(page) + dossiers_visibles = dossiers.visible_by_user + + @user_dossiers = current_user.dossiers.state_not_termine.merge(dossiers_visibles) + @dossiers_traites = current_user.dossiers.state_termine.merge(dossiers_visibles) + @dossiers_close_to_expiration = current_user.dossiers.close_to_expiration.merge(dossiers_visibles) + @dossiers_invites = current_user.dossiers_invites.merge(dossiers_visibles) + @dossiers_supprimes_recemment = current_user.dossiers.hidden_by_user.merge(dossiers) @dossiers_supprimes_definitivement = current_user.deleted_dossiers.order_by_updated_at.page(page) @dossier_transfers = DossierTransfer .includes(dossiers: :user) .with_dossiers .where(email: current_user.email) .page(page) - @dossiers_close_to_expiration = current_user.dossiers.close_to_expiration.page(page) @statut = statut(@user_dossiers, @dossiers_traites, @dossiers_invites, @dossiers_supprimes_recemment, @dossiers_supprimes_definitivement, @dossier_transfers, @dossiers_close_to_expiration, params[:statut]) end @@ -214,9 +219,8 @@ module Users end def delete_dossier - dossier = current_user.dossiers.includes(:user, procedure: :administrateurs).find(params[:id]) if dossier.can_be_deleted_by_user? - dossier.discard_and_keep_track!(current_user, :user_request) + dossier.hide_and_keep_track!(current_user, :user_request) flash.notice = t('users.dossiers.ask_deletion.soft_deleted_dossier') redirect_to dossiers_path else @@ -277,7 +281,7 @@ module Users def dossier_for_help dossier_id = params[:id] || params[:dossier_id] - @dossier || (dossier_id.present? && Dossier.find_by(id: dossier_id.to_i)) + @dossier || (dossier_id.present? && Dossier.visible_by_user.find_by(id: dossier_id.to_i)) end def transferer @@ -289,7 +293,7 @@ module Users end def restore - dossier.restore(current_user) + hidden_dossier.restore(current_user) flash.notice = t('users.dossiers.restore') redirect_to dossiers_path end @@ -355,11 +359,15 @@ module Users end def dossier - @dossier ||= Dossier.find(params[:id] || params[:dossier_id]) + @dossier ||= Dossier.visible_by_user.find(params[:id] || params[:dossier_id]) + end + + def hidden_dossier + @hidden_dossier ||= Dossier.hidden_by_user.find(params[:id] || params[:dossier_id]) end def dossier_with_champs - Dossier.with_champs.find(params[:id]) + Dossier.with_champs.visible_by_user.find(params[:id]) end def should_change_groupe_instructeur? @@ -436,6 +444,12 @@ module Users end end + def ensure_ownership_or_invitation_hidden! + if !current_user.owns_or_invite?(hidden_dossier) + forbidden! + end + end + def forbid_invite_submission! if passage_en_construction? && !current_user.owns?(dossier) forbidden! diff --git a/app/graphql/types/query_type.rb b/app/graphql/types/query_type.rb index cdcd2c56d..d6c6405b6 100644 --- a/app/graphql/types/query_type.rb +++ b/app/graphql/types/query_type.rb @@ -20,9 +20,9 @@ module Types def dossier(number:) if context.internal_use? - Dossier.state_not_brouillon.with_discarded.for_api_v2.find(number) - else Dossier.state_not_brouillon.for_api_v2.find(number) + else + Dossier.visible_by_administration.for_api_v2.find(number) end rescue => e raise GraphQL::ExecutionError.new(e.message, extensions: { code: :not_found }) diff --git a/app/mailers/avis_mailer.rb b/app/mailers/avis_mailer.rb index 63d8b470c..d21053e5a 100644 --- a/app/mailers/avis_mailer.rb +++ b/app/mailers/avis_mailer.rb @@ -5,7 +5,7 @@ class AvisMailer < ApplicationMailer layout 'mailers/layout' def avis_invitation(avis) - if avis.dossier.present? + if avis.dossier.visible_by_administration? @avis = avis email = @avis.expert&.email subject = "Donnez votre avis sur le dossier nº #{@avis.dossier.id} (#{@avis.dossier.procedure.libelle})" diff --git a/app/models/avis.rb b/app/models/avis.rb index 93cf9e41f..ad3976091 100644 --- a/app/models/avis.rb +++ b/app/models/avis.rb @@ -48,8 +48,8 @@ class Avis < ApplicationRecord scope :for_dossier, -> (dossier_id) { where(dossier_id: dossier_id) } scope :by_latest, -> { order(updated_at: :desc) } scope :updated_since?, -> (date) { where('avis.updated_at > ?', date) } - scope :discarded_termine_expired, -> { unscope(:joins).where(dossier: Dossier.discarded_termine_expired) } - scope :discarded_en_construction_expired, -> { unscope(:joins).where(dossier: Dossier.discarded_en_construction_expired) } + scope :termine_expired, -> { unscope(:joins).where(dossier: Dossier.termine_expired) } + scope :en_construction_expired, -> { unscope(:joins).where(dossier: Dossier.en_construction_expired) } scope :not_hidden_by_administration, -> { where(dossiers: { hidden_by_administration_at: nil }) } # The form allows subtmitting avis requests to several emails at once, # hence this virtual attribute. diff --git a/app/models/champ.rb b/app/models/champ.rb index aa093ccc5..b2aee8e41 100644 --- a/app/models/champ.rb +++ b/app/models/champ.rb @@ -20,7 +20,7 @@ # type_de_champ_id :integer # class Champ < ApplicationRecord - belongs_to :dossier, -> { with_discarded }, inverse_of: false, touch: true, optional: false + belongs_to :dossier, inverse_of: false, touch: true, optional: false belongs_to :type_de_champ, inverse_of: :champ, optional: false belongs_to :parent, class_name: 'Champ', optional: true has_one_attached :piece_justificative_file diff --git a/app/models/deleted_dossier.rb b/app/models/deleted_dossier.rb index 7ba5c6603..103e2a6ea 100644 --- a/app/models/deleted_dossier.rb +++ b/app/models/deleted_dossier.rb @@ -30,6 +30,8 @@ class DeletedDossier < ApplicationRecord } def self.create_from_dossier(dossier, reason) + return if !dossier.log_operations? + # We have some bad data because of partially deleted dossiers in the past. # For now use find_or_create_by! to avoid errors. create_with( diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 44c0b4c10..08f093c22 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -42,10 +42,6 @@ class Dossier < ApplicationRecord include DossierFilteringConcern include DossierRebaseConcern - include Discard::Model - self.discard_column = :hidden_at - default_scope -> { kept } - enum state: { brouillon: 'brouillon', en_construction: 'en_construction', @@ -200,6 +196,7 @@ class Dossier < ApplicationRecord scope :state_brouillon, -> { where(state: states.fetch(:brouillon)) } scope :state_not_brouillon, -> { where.not(state: states.fetch(:brouillon)) } scope :state_en_construction, -> { where(state: states.fetch(:en_construction)) } + scope :state_not_en_construction, -> { where.not(state: states.fetch(:en_construction)) } scope :state_en_instruction, -> { where(state: states.fetch(:en_instruction)) } scope :state_en_construction_ou_instruction, -> { where(state: EN_CONSTRUCTION_OU_INSTRUCTION) } scope :state_instruction_commencee, -> { where(state: INSTRUCTION_COMMENCEE) } @@ -211,7 +208,11 @@ class Dossier < ApplicationRecord scope :hidden_by_user, -> { where.not(hidden_by_user_at: nil) } scope :hidden_by_administration, -> { where.not(hidden_by_administration_at: nil) } scope :visible_by_user, -> { where(hidden_by_user_at: nil) } - scope :visible_by_administration, -> { where("hidden_by_administration_at IS NULL AND NOT (hidden_by_user_at IS NOT NULL AND dossiers.state = 'en_construction')") } + scope :visible_by_administration, -> { + state_not_brouillon + .where(hidden_by_administration_at: nil) + .merge(visible_by_user.or(state_not_en_construction)) + } scope :order_by_updated_at, -> (order = :desc) { order(updated_at: order) } scope :order_by_created_at, -> (order = :asc) { order(depose_at: order, created_at: order, id: order) } @@ -349,27 +350,11 @@ class Dossier < ApplicationRecord scope :without_en_construction_expiration_notice_sent, -> { where(en_construction_close_to_expiration_notice_sent_at: nil) } scope :without_termine_expiration_notice_sent, -> { where(termine_close_to_expiration_notice_sent_at: nil) } - scope :discarded_expired, -> { discarded.where('dossiers.hidden_at < ?', 1.week.ago) } - scope :discarded_by_user_expired, -> { discarded.where('dossiers.hidden_by_user_at < ?', 1.week.ago) } - scope :discarded_by_administration_expired, -> { discarded.where('dossiers.hidden_by_administration_at < ?', 1.week.ago) } - scope :discarded_brouillon_expired, -> do - with_discarded - .state_brouillon - .discarded_expired - .or(state_brouillon.discarded_by_user_expired) - end - scope :discarded_en_construction_expired, -> do - with_discarded - .state_en_construction - .discarded_expired - .or(state_en_construction.discarded_by_user_expired) - end - scope :discarded_termine_expired, -> do - with_discarded - .state_termine - .discarded_expired - .or(state_termine.discarded_by_user_expired.discarded_by_administration_expired) - end + scope :deleted_by_user_expired, -> { where('dossiers.hidden_by_user_at < ?', 1.week.ago) } + scope :deleted_by_administration_expired, -> { where('dossiers.hidden_by_administration_at < ?', 1.week.ago) } + scope :en_brouillon_expired_to_delete, -> { state_brouillon.deleted_by_user_expired } + scope :en_construction_expired_to_delete, -> { state_en_construction.deleted_by_user_expired } + scope :termine_expired_to_delete, -> { state_termine.deleted_by_user_expired.deleted_by_administration_expired } scope :brouillon_near_procedure_closing_date, -> do # select users who have submitted dossier for the given 'procedures.id' @@ -537,8 +522,8 @@ class Dossier < ApplicationRecord brouillon? || en_construction? || termine? end - def can_be_hidden_by_user? - en_construction? || termine? + def can_be_deleted_by_administration?(reason) + termine? || reason == :procedure_removed end def messagerie_available? @@ -686,10 +671,6 @@ class Dossier < ApplicationRecord !procedure.brouillon? && !brouillon? end - def keep_track_on_deletion? - !procedure.brouillon? && !brouillon? - end - def hidden_by_user? hidden_by_user_at.present? end @@ -698,8 +679,16 @@ class Dossier < ApplicationRecord hidden_by_administration_at.present? end - def deleted_by_instructeur_and_user? - termine? && hidden_by_administration? && hidden_by_user? + def hidden_for_administration? + hidden_by_administration? || (hidden_by_user? && en_construction?) || brouillon? + end + + def visible_by_administration? + !hidden_for_administration? + end + + def hidden_for_administration_and_user? + hidden_for_administration? && hidden_by_user? end def expose_legacy_carto_api? @@ -738,11 +727,9 @@ class Dossier < ApplicationRecord def expired_keep_track_and_destroy! transaction do - if keep_track_on_deletion? - DeletedDossier.create_from_dossier(self, :expired) - dossier_operation_logs.destroy_all - log_automatic_dossier_operation(:supprimer, self) - end + DeletedDossier.create_from_dossier(self, :expired) + dossier_operation_logs.destroy_all + log_automatic_dossier_operation(:supprimer, self) destroy! end true @@ -758,37 +745,20 @@ class Dossier < ApplicationRecord author.is_a?(Instructeur) || author.is_a?(Administrateur) || author.is_a?(SuperAdmin) end - def restore_dossier_and_destroy_deleted_dossier(author) - if deleted_dossier.present? - deleted_dossier&.destroy! - end - - log_dossier_operation(author, :restaurer, self) - end - - def discard_and_keep_track!(author, reason) - if termine? && author_is_administration(author) - update(hidden_by_administration_at: Time.zone.now, hidden_by_reason: reason) - end - - if can_be_hidden_by_user? && author_is_user(author) - update(hidden_by_user_at: Time.zone.now, dossier_transfer_id: nil, hidden_by_reason: reason) - end - + def hide_and_keep_track!(author, reason) transaction do - if deleted_by_instructeur_and_user? || en_construction? || brouillon? - if keep_track_on_deletion? - log_dossier_operation(author, :supprimer, self) - end - - if !(en_construction? && author_is_user(author)) - discard! - end + if author_is_administration(author) && can_be_deleted_by_administration?(reason) + update(hidden_by_administration_at: Time.zone.now, hidden_by_reason: reason) + elsif author_is_user(author) && can_be_deleted_by_user? + update(hidden_by_user_at: Time.zone.now, dossier_transfer_id: nil, hidden_by_reason: reason) + else + raise "Unauthorized dossier hide attempt Dossier##{id} by #{author} for reason #{reason}" end + + log_dossier_operation(author, :supprimer, self) end - if en_construction? - update(hidden_by_reason: reason) + if en_construction? && !hidden_by_administration? administration_emails = followers_instructeurs.present? ? followers_instructeurs.map(&:email) : procedure.administrateurs.map(&:email) administration_emails.each do |email| DossierMailer.notify_en_construction_deletion_to_administration(self, email).deliver_later @@ -797,30 +767,18 @@ class Dossier < ApplicationRecord end def restore(author) - if discarded? - transaction do - if author_is_administration(author) && hidden_by_administration? - update(hidden_by_administration_at: nil) - end - - if undiscard && keep_track_on_deletion? - restore_dossier_and_destroy_deleted_dossier(author) - end - end - elsif author_is_user(author) && hidden_by_user? - transaction do - update(hidden_by_user_at: nil) - !hidden_by_administration? && update(hidden_by_reason: nil) - - if en_construction? - restore_dossier_and_destroy_deleted_dossier(author) - end - end - elsif author_is_administration(author) && hidden_by_administration? - transaction do + transaction do + if author_is_administration(author) update(hidden_by_administration_at: nil) - !hidden_by_user? && update(hidden_by_reason: nil) + elsif author_is_user(author) + update(hidden_by_user_at: nil) end + + if !hidden_by_user? && !hidden_by_administration? + update(hidden_by_reason: nil) + end + + log_dossier_operation(author, :restaurer, self) end end @@ -1152,19 +1110,16 @@ class Dossier < ApplicationRecord def purge_discarded transaction do - if keep_track_on_deletion? - DeletedDossier.create_from_dossier(self, hidden_by_reason) - end - + DeletedDossier.create_from_dossier(self, hidden_by_reason) dossier_operation_logs.not_deletion.destroy_all destroy end end def self.purge_discarded - discarded_brouillon_expired.find_each(&:purge_discarded) - discarded_en_construction_expired.find_each(&:purge_discarded) - discarded_termine_expired.find_each(&:purge_discarded) + en_brouillon_expired_to_delete.find_each(&:purge_discarded) + en_construction_expired_to_delete.find_each(&:purge_discarded) + termine_expired_to_delete.find_each(&:purge_discarded) end private @@ -1243,7 +1198,7 @@ class Dossier < ApplicationRecord followers_instructeurs.each do |instructeur| if instructeur.groupe_instructeurs.exclude?(groupe_instructeur) instructeur.unfollow(self) - if kept? + if visible_by_administration? DossierMailer.notify_groupe_instructeur_changed(instructeur, self).deliver_later end end diff --git a/app/models/dossier_operation_log.rb b/app/models/dossier_operation_log.rb index 316b54647..9a348680f 100644 --- a/app/models/dossier_operation_log.rb +++ b/app/models/dossier_operation_log.rb @@ -37,8 +37,9 @@ class DossierOperationLog < ApplicationRecord belongs_to :bill_signature, optional: true scope :not_deletion, -> { where.not(operation: operations.fetch(:supprimer)) } - scope :discarded_en_construction_expired, -> { where(dossier: Dossier.discarded_en_construction_expired).not_deletion } - scope :discarded_termine_expired, -> { where(dossier: Dossier.discarded_termine_expired).not_deletion } + scope :brouillon_expired, -> { where(dossier: Dossier.brouillon_expired).not_deletion } + scope :en_construction_expired, -> { where(dossier: Dossier.en_construction_expired).not_deletion } + scope :termine_expired, -> { where(dossier: Dossier.termine_expired).not_deletion } def self.create_and_serialize(params) dossier = params.fetch(:dossier) diff --git a/app/models/dossier_transfer.rb b/app/models/dossier_transfer.rb index d2a6a3ac8..4ae5f88b3 100644 --- a/app/models/dossier_transfer.rb +++ b/app/models/dossier_transfer.rb @@ -14,7 +14,7 @@ class DossierTransfer < ApplicationRecord scope :pending, -> { where('created_at > ?', (Time.zone.now - EXPIRATION_LIMIT)) } scope :stale, -> { where('created_at < ?', (Time.zone.now - EXPIRATION_LIMIT)) } - scope :with_dossiers, -> { joins(:dossiers) } + scope :with_dossiers, -> { joins(:dossiers).merge(Dossier.visible_by_user) } after_create_commit :send_notification @@ -48,7 +48,7 @@ class DossierTransfer < ApplicationRecord def destroy_and_nullify transaction do # Rails cascading is not working with default scopes. Doing nullify cascade manually. - dossiers.with_discarded.update_all(dossier_transfer_id: nil) + dossiers.update_all(dossier_transfer_id: nil) destroy end end @@ -56,7 +56,7 @@ class DossierTransfer < ApplicationRecord def self.destroy_stale transaction do # Rails cascading is not working with default scopes. Doing nullify cascade manually. - Dossier.with_discarded.where(transfer: stale).update_all(dossier_transfer_id: nil) + Dossier.where(transfer: stale).update_all(dossier_transfer_id: nil) stale.destroy_all end end diff --git a/app/models/instructeur.rb b/app/models/instructeur.rb index ed870b3e3..6ab359dae 100644 --- a/app/models/instructeur.rb +++ b/app/models/instructeur.rb @@ -256,8 +256,7 @@ class Instructeur < ApplicationRecord AND follows.unfollowed_at IS NULL WHERE dossiers.state != 'brouillon' AND dossiers.groupe_instructeur_id in (:groupe_instructeur_ids) - AND dossiers.hidden_at IS NULL - AND NOT (dossiers.hidden_by_user_at IS NOT NULL AND dossiers.state = 'en_construction') + AND (dossiers.hidden_by_user_at IS NULL OR dossiers.state != 'en_construction') EOF sanitized_query = ActiveRecord::Base.sanitize_sql([ diff --git a/app/models/invite.rb b/app/models/invite.rb index 5c4d4c1f5..28d1e3038 100644 --- a/app/models/invite.rb +++ b/app/models/invite.rb @@ -26,14 +26,9 @@ class Invite < ApplicationRecord validates :email, format: { with: Devise.email_regexp, message: "n'est pas valide" }, allow_nil: true - # #1619 When an administrateur deletes a `Procedure`, its `hidden_at` field, and - # the `hidden_at` field of its `Dossier`s, get set, effectively removing the Procedure - # and Dossier from their respective `default_scope`s. - # Therefore, we also remove `Invite`s for such effectively deleted `Dossier`s - # from their default scope. - scope :kept, -> { joins(:dossier).merge(Dossier.kept) } + scope :with_dossiers, -> { joins(:dossier).merge(Dossier.visible_by_user) } - default_scope { kept } + default_scope { with_dossiers } def send_notification if self.user.present? diff --git a/app/models/procedure.rb b/app/models/procedure.rb index a7cc0e1a9..dcc5bf8f9 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -682,15 +682,15 @@ class Procedure < ApplicationRecord close! end - dossiers.termine.visible_by_administration.each do |dossier| - dossier.discard_and_keep_track!(author, :procedure_removed) + dossiers.visible_by_administration.each do |dossier| + dossier.hide_and_keep_track!(author, :procedure_removed) end discard! end def purge_discarded - if !dossiers.with_discarded.exists? + if dossiers.empty? destroy end end diff --git a/app/models/user.rb b/app/models/user.rb index 0e88dba7e..4da9b3382 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -190,20 +190,34 @@ class User < ApplicationRecord end transaction do - Invite.where(dossier: dossiers.with_discarded).destroy_all - dossiers.state_en_construction.each do |dossier| - dossier.discard_and_keep_track!(administration, :user_removed) + # delete invites + Invite.where(dossier: dossiers).destroy_all + + # delete dossiers brouillon + dossiers.state_brouillon.each do |dossier| + dossier.hide_and_keep_track!(dossier.user, :user_removed) + end + dossiers.state_brouillon.find_each(&:purge_discarded) + + # delete dossiers en_construction + dossiers.state_en_construction.each do |dossier| + dossier.hide_and_keep_track!(dossier.user, :user_removed) + end + dossiers.state_en_construction.find_each(&:purge_discarded) + + # delete dossiers terminé + dossiers.state_termine.each do |dossier| + dossier.hide_and_keep_track!(dossier.user, :user_removed) end - DossierOperationLog.where(dossier: dossiers.with_discarded.discarded).not_deletion.destroy_all - dossiers.with_discarded.discarded.destroy_all dossiers.update_all(deleted_user_email_never_send: email, user_id: nil, dossier_transfer_id: nil) + destroy! end end def merge(old_user) transaction do - old_user.dossiers.with_discarded.update_all(user_id: id) + old_user.dossiers.update_all(user_id: id) old_user.invites.update_all(user_id: id) old_user.merge_logs.update_all(user_id: id) diff --git a/app/services/dossier_search_service.rb b/app/services/dossier_search_service.rb index a3f95a3f5..4d898839c 100644 --- a/app/services/dossier_search_service.rb +++ b/app/services/dossier_search_service.rb @@ -18,7 +18,7 @@ class DossierSearchService def self.dossier_by_exact_id(dossiers, search_terms) id = search_terms.to_i if id != 0 && id_compatible?(id) # Sometimes instructeur is searching dossiers with a big number (ex: SIRET), ActiveRecord can't deal with them and throws ActiveModel::RangeError. id_compatible? prevents this. - dossiers.where(id: id).ids + dossiers.visible_by_administration.where(id: id).ids else [] end @@ -29,6 +29,7 @@ class DossierSearchService ts_query = "to_tsquery('french', #{Dossier.connection.quote(to_tsquery(search_terms))})" dossiers + .visible_by_administration .where("#{ts_vector} @@ #{ts_query}") .order(Arel.sql("COALESCE(ts_rank(#{ts_vector}, #{ts_query}), 0) DESC")) .pluck('id') @@ -40,6 +41,7 @@ class DossierSearchService ts_query = "to_tsquery('french', #{Dossier.connection.quote(to_tsquery(search_terms))})" dossiers + .visible_by_user .where("#{ts_vector} @@ #{ts_query}") .order(Arel.sql("COALESCE(ts_rank(#{ts_vector}, #{ts_query}), 0) DESC")) end @@ -47,7 +49,7 @@ class DossierSearchService def self.dossier_by_exact_id_for_user(search_terms, user) id = search_terms.to_i if id != 0 && id_compatible?(id) # Sometimes user is searching dossiers with a big number (ex: SIRET), ActiveRecord can't deal with them and throws ActiveModel::RangeError. id_compatible? prevents this. - Dossier.where(id: user.dossiers.where(id: id) + user.dossiers_invites.where(id: id)).distinct + Dossier.where(id: user.dossiers.visible_by_user.where(id: id) + user.dossiers_invites.visible_by_user.where(id: id)).distinct else Dossier.none end diff --git a/app/views/manager/dossiers/show.html.erb b/app/views/manager/dossiers/show.html.erb index 2e7454290..423182570 100644 --- a/app/views/manager/dossiers/show.html.erb +++ b/app/views/manager/dossiers/show.html.erb @@ -22,7 +22,7 @@ as well as a link to its edit page.