From 0309e0f6b2759d51158d2524c6da090ea988aa94 Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Wed, 11 Sep 2024 12:09:21 +0200 Subject: [PATCH 1/4] refactor(profile): generalize profile from referrer or for user --- app/controllers/application_controller.rb | 12 +---- .../concerns/nav_bar_profile_concern.rb | 44 +++++++++++++++++++ app/controllers/errors_controller.rb | 2 - app/controllers/release_notes_controller.rb | 2 - app/views/layouts/_header.haml | 4 +- 5 files changed, 47 insertions(+), 17 deletions(-) create mode 100644 app/controllers/concerns/nav_bar_profile_concern.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index c4d597cfd51..afaf4876259 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -2,6 +2,7 @@ class ApplicationController < ActionController::Base include TrustedDeviceConcern + include NavBarProfileConcern include Pundit::Authorization include Devise::StoreLocationExtension include ApplicationController::LongLivedAuthenticityToken @@ -422,17 +423,6 @@ def set_customizable_view_path prepend_view_path "app/custom_views" end - def try_nav_bar_profile_from_referrer - # detect context from referer, simple (no detection when refreshing the page) - params = Rails.application.routes.recognize_path(request&.referer) - - controller_class = "#{params[:controller].camelize}Controller".safe_constantize - return if controller_class.nil? - - controller_instance = controller_class.new - controller_instance.try(:nav_bar_profile) - end - def cast_bool(value) ActiveRecord::Type::Boolean.new.deserialize(value) end diff --git a/app/controllers/concerns/nav_bar_profile_concern.rb b/app/controllers/concerns/nav_bar_profile_concern.rb new file mode 100644 index 00000000000..4c856480f98 --- /dev/null +++ b/app/controllers/concerns/nav_bar_profile_concern.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +module NavBarProfileConcern + extend ActiveSupport::Concern + + included do + # Override this method on controller basis for more precise context or custom logic + def nav_bar_profile + end + + def fallback_nav_bar_profile + return :guest if current_user.blank? + + nav_bar_profile_from_referrer || default_nav_bar_profile_for_user + end + + private + + # Shared controllers (search, errors, release notes…) don't have specific context + # Simple attempt to try to re-use the profile from the previous page + # so user does'not feel lost. + def nav_bar_profile_from_referrer + # detect context from referer, simple (no detection when refreshing the page) + params = Rails.application.routes.recognize_path(request&.referer) + + controller_class = "#{params[:controller].camelize}Controller".safe_constantize + return if controller_class.nil? + + controller_instance = controller_class.new + controller_instance.try(:nav_bar_profile) + end + + # Fallback for shared controllers from user account + # to the more relevant profile. + def default_nav_bar_profile_for_user + return :gestionnaire if current_user.gestionnaire? + return :administrateur if current_user.administrateur? + return :instructeur if current_user.instructeur? + return :expert if current_user.expert? + + :user + end + end +end diff --git a/app/controllers/errors_controller.rb b/app/controllers/errors_controller.rb index be1b685651f..e44b5ee8b0c 100644 --- a/app/controllers/errors_controller.rb +++ b/app/controllers/errors_controller.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true class ErrorsController < ApplicationController - def nav_bar_profile = try_nav_bar_profile_from_referrer - rescue_from Exception do # catch any error, except errors triggered by middlewares outside controller (like warden middleware) render file: Rails.public_path.join('500.html'), layout: false, status: :internal_server_error diff --git a/app/controllers/release_notes_controller.rb b/app/controllers/release_notes_controller.rb index 494c338fb18..cb5c08c1e0a 100644 --- a/app/controllers/release_notes_controller.rb +++ b/app/controllers/release_notes_controller.rb @@ -23,8 +23,6 @@ def index render "scrollable_list" if params[:page].present? end - def nav_bar_profile = try_nav_bar_profile_from_referrer - private def touch_default_categories_seen_at diff --git a/app/views/layouts/_header.haml b/app/views/layouts/_header.haml index ca22f2acc0a..3bc89efd58b 100644 --- a/app/views/layouts/_header.haml +++ b/app/views/layouts/_header.haml @@ -1,5 +1,5 @@ --# We can't use &. because the controller may not implement #nav_bar_profile -- nav_bar_profile = controller.try(:nav_bar_profile) || :guest +-# We can't use &. or as helper methods because the controllers from view specs does not implement these methods +- nav_bar_profile = controller.try(:nav_bar_profile) || controller.try(:fallback_nav_bar_profile) || :guest - dossier = controller.try(:dossier_for_help) - procedure = controller.try(:procedure_for_help) - is_instructeur_context = nav_bar_profile == :instructeur && instructeur_signed_in? From 16e1daac9b5c626a1ac36cc18d446f062d235fd7 Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Wed, 11 Sep 2024 15:13:16 +0200 Subject: [PATCH 2/4] fix(recherche): respects nav bar profile context --- app/controllers/recherche_controller.rb | 12 ++++++ app/views/layouts/_header.haml | 10 ++--- .../layouts/_search_dossiers_form.html.haml | 1 + spec/controllers/recherche_controller_spec.rb | 37 +++++++++++++++++-- 4 files changed, 52 insertions(+), 8 deletions(-) diff --git a/app/controllers/recherche_controller.rb b/app/controllers/recherche_controller.rb index 73b9df44837..98d68479232 100644 --- a/app/controllers/recherche_controller.rb +++ b/app/controllers/recherche_controller.rb @@ -9,6 +9,18 @@ class RechercheController < ApplicationController { "table" => 'procedure', "column" => 'procedure_id' } ] + def nav_bar_profile + return super if request.blank? # Controller introspection does not contains params/request, see NavBarProfileConcern + + context_params = params[:context]&.to_sym + case context_params + when :instructeur, :expert + context_params + else + :user + end + end + def index @search_terms = search_terms @dossiers_count = 0 diff --git a/app/views/layouts/_header.haml b/app/views/layouts/_header.haml index 3bc89efd58b..6b1e1ab47f4 100644 --- a/app/views/layouts/_header.haml +++ b/app/views/layouts/_header.haml @@ -61,13 +61,13 @@ %li= render partial: 'layouts/locale_dropdown' - - if params[:controller] == 'recherche' - = render partial: 'layouts/search_dossiers_form' - - if is_instructeur_context - = render partial: 'layouts/search_dossiers_form' + = render partial: 'layouts/search_dossiers_form', locals: { context: :instructeur } + + - elsif is_expert_context + = render partial: 'layouts/search_dossiers_form', locals: { context: :expert } - - if is_expert_context + - elsif params[:controller] == 'recherche' = render partial: 'layouts/search_dossiers_form' = render SwitchDomainBannerComponent.new(user: current_user) diff --git a/app/views/layouts/_search_dossiers_form.html.haml b/app/views/layouts/_search_dossiers_form.html.haml index 9596161633e..3ae8dc46d90 100644 --- a/app/views/layouts/_search_dossiers_form.html.haml +++ b/app/views/layouts/_search_dossiers_form.html.haml @@ -3,6 +3,7 @@ %button.fr-btn--close.fr-btn{ "aria-controls" => "search-modal", :title => t('close_modal', scope: [:layouts, :header]) }= t('close_modal', scope: [:layouts, :header]) #search-473.fr-search-bar.fr-search-bar--lg = form_tag recherche_index_path, method: :get, :role => "search", class: "flex width-100" do + = hidden_field_tag :context, local_assigns[:context] = label_tag "q", t('views.users.dossiers.search.search_file'), class: 'sr-only' = text_field_tag "q", "#{@search_terms if @search_terms.present?}", placeholder: t('views.users.dossiers.search.search_file'), class: "fr-input" %button.fr-btn diff --git a/spec/controllers/recherche_controller_spec.rb b/spec/controllers/recherche_controller_spec.rb index feceb68d157..0c155fa4842 100644 --- a/spec/controllers/recherche_controller_spec.rb +++ b/spec/controllers/recherche_controller_spec.rb @@ -201,12 +201,43 @@ context 'with no query param it does not crash' do subject { get :index, params: {} } - it { is_expected.to have_http_status(200) } - it 'returns 0 dossier' do - subject + expect(subject).to have_http_status(200) expect(assigns(:projected_dossiers).count).to eq(0) end end + + context 'nav bar profile in user context' do + subject { get(:index, params: {}).body } + render_views + + it 'define user nav' do + expect(subject).to include "Mes dossiers" + expect(subject).to include "usager" + end + end + + context 'nav bar profile in instructeur context' do + subject { get(:index, params: { context: :instructeur }).body } + render_views + + it 'define instructeur nav' do + expect(subject).to include "Démarches" + expect(subject).to include "instructeur" + expect(subject).not_to include "Mes dossiers" + end + end + + context 'nav bar profile in expert context' do + before { user.create_expert } + subject { get(:index, params: { context: :expert }).body } + render_views + + it 'define expert nav' do + expect(subject).to include "Avis" + expect(subject).to include "expert" + expect(subject).not_to include "Mes dossiers" + end + end end end From d5661869546228b89737e117122bd4424342f517 Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Wed, 11 Sep 2024 15:13:32 +0200 Subject: [PATCH 3/4] fix: coherent nav bar profile for super admins --- app/controllers/super_admins/sessions_controller.rb | 1 + app/controllers/super_admins_controller.rb | 2 ++ 2 files changed, 3 insertions(+) diff --git a/app/controllers/super_admins/sessions_controller.rb b/app/controllers/super_admins/sessions_controller.rb index 80113311503..14ac5149e64 100644 --- a/app/controllers/super_admins/sessions_controller.rb +++ b/app/controllers/super_admins/sessions_controller.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true class SuperAdmins::SessionsController < Devise::SessionsController + def nav_bar_profile = :superadmin end diff --git a/app/controllers/super_admins_controller.rb b/app/controllers/super_admins_controller.rb index 4b166c52561..d6767332086 100644 --- a/app/controllers/super_admins_controller.rb +++ b/app/controllers/super_admins_controller.rb @@ -3,6 +3,8 @@ class SuperAdminsController < ApplicationController before_action :authenticate_super_admin! + def nav_bar_profile = :superadmin + def edit_otp end From f6744adbdeb63628359471c2a006afa3f49fd499 Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Wed, 11 Sep 2024 18:22:25 +0200 Subject: [PATCH 4/4] fix: never show 'guest' profile name it's just a hack to tell we don't known the actual profile --- app/views/layouts/_account_dropdown.haml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/views/layouts/_account_dropdown.haml b/app/views/layouts/_account_dropdown.haml index 39beaf8cedd..a9279b58f2a 100644 --- a/app/views/layouts/_account_dropdown.haml +++ b/app/views/layouts/_account_dropdown.haml @@ -5,8 +5,9 @@ - if dossier.present? && dossier&.france_connected_with_one_identity? %span  via FranceConnect - %span{ class: "fr-badge fr-badge--sm #{color_by_role(nav_bar_profile)}" } - = t("layouts.#{nav_bar_profile}") + - if nav_bar_profile != :guest # don't confuse user with unknown profile + %span{ class: "fr-badge fr-badge--sm #{color_by_role(nav_bar_profile)}" } + = t("layouts.#{nav_bar_profile}") #account.fr-collapse.fr-menu %ul.fr-menu__list.max-content - if multiple_devise_profile_connect?