Merge pull request #10778 from colinux/sharerd-controller-nav-bar-profile

UX: améliore la détection du type de profile utilisé pour la nav bar

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 @@ class ApplicationController < ActionController::Base
     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

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

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

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

app/controllers/release_notes_controller.rb

@@ -23,8 +23,6 @@ class ReleaseNotesController < ApplicationController
     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

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

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
 

app/views/layouts/_account_dropdown.haml

@@ -5,6 +5,7 @@
       - if dossier.present? && dossier&.france_connected_with_one_identity?
         %span
            via FranceConnect
+      - 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

app/views/layouts/_header.haml

@@ -1,5 +1,5 @@
--# We can't use &. because the controller may not implement #nav_bar_profile
+-# 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) || :guest
+- 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?
@@ -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 }
 
-            - if is_expert_context
+            - elsif is_expert_context
+              = render partial: 'layouts/search_dossiers_form', locals: { context: :expert }
+
+            - elsif params[:controller] == 'recherche'
               = render partial: 'layouts/search_dossiers_form'
 
     = render SwitchDomainBannerComponent.new(user: current_user)

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

spec/controllers/recherche_controller_spec.rb

@@ -201,12 +201,43 @@ describe RechercheController, type: :controller do
     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