Improve performances

* Charger le modal de projets à la demande en appelant la méthode render_select_projects_modal_by_ajax + optimiser le code afin d'accélérer les pages * Remplace utilisation de méthode include? sur un array par utilisation les opérations d'intersection+ différence dans la page projet/settings * Ajouter le test de méthode render_project_nested_lists
nanego · Sep 14, 2023 · 376eb7a · 376eb7a
1 parent 3b9fb44
commit 376eb7a
Show file tree

Hide file tree

Showing 10 changed files with 257 additions and 87 deletions.
diff --git a/app/controllers/issue_templates_controller.rb b/app/controllers/issue_templates_controller.rb
@@ -153,6 +153,28 @@ def add_repeatable_group
     render partial: "issues/sections/add_repeatable_group"
   end
 
+  def render_select_projects_modal_by_ajax
+
+    @issue_template = params[:template_id].present? ? IssueTemplate.find(params[:template_id]) : IssueTemplate.new
+
+    vals_allowed_target = Rails.env.test? && params[:format] == 'js' ? JSON.parse(params[:allowed_projects]) : params[:allowed_projects].permit!.to_h.values
+    # convert to int
+    @allowed_target_projects_attributes_array = vals_allowed_target.map do |id, name, status, lft, rgt|
+      [id.to_i, name, status.to_i, lft.to_i, rgt.to_i]
+    end
+
+    if params[:project_ids]
+      project_ids = params[:project_ids].split(',')
+      vals_template_projects = (project_ids.present? ? Project.find(project_ids).pluck(:id, :name, :status, :lft, :rgt) : [])
+    end
+
+    # convert to int
+    @template_projects_attributes_array = vals_template_projects.map do |id, name, status, lft, rgt|
+      [id.to_i, name, status.to_i, lft.to_i, rgt.to_i]
+    end
+
+    render json: { html: render_to_string(partial: 'modal_select_projects') }
+  end
   private
 
   def find_project

diff --git a/app/views/issue_templates/_form.html.erb b/app/views/issue_templates/_form.html.erb
@@ -58,47 +58,42 @@
 
           <div class="splitcontentright">
             <div class="select_projects" id="select_projects">
-              <%= f.select :template_project_ids, project_tree_options(@issue_template.allowed_target_projects | @issue_template.template_projects, :selected => @issue_template.template_projects), { :required => true },
-                           { :multiple => true, :size => 10, style: "display:none;" } %>
               <%
-                nb_other_projects = 0
-                nb_visible_projects = 0
                 template_projects = @issue_template.template_projects
                 allowed_projects = @issue_template.allowed_target_projects
+
+                # Here, We use map instead of select('id','name') because of in the failure validation case for a non persisted template
+                # :Associations::CollectionProxy select('id','name') will return []
+                visible_projects = (allowed_projects.map(&:name) & template_projects.map(&:name))
+                other_projects = (template_projects.map(&:name) - allowed_projects.map(&:name))
+
               %>
+              <%= f.select :template_project_ids, project_tree_options(allowed_projects | template_projects, :selected => template_projects), { :required => true },
+                           { :multiple => true, :size => 10, style: "display: none;" } %>
               <span id="my_projects">
-                <% template_projects.each do |project| %>
-                  <% if allowed_projects.include?(project) %>
-                    <%= content_tag("span", project.name.gsub(/ /, "&nbsp;").html_safe, class: "list_templates_projects_names") %>
-                    <% nb_visible_projects += 1 %>
-                  <% else %>
-                    <% nb_other_projects += 1 %>
-                  <% end %>
+                <% visible_projects.each do |project| %>
+
+                    <%= content_tag("span", project.gsub(/ /, "&nbsp;").html_safe, class: "list_templates_projects_names") %>
+
                 <% end %>
               </span>
-              <% if nb_other_projects > 0 %>
+              <% if other_projects.count > 0 %>
                 <span class="list_templates_other_projects">
-                  <% if nb_visible_projects > 0 %>
+                  <% if visible_projects.count > 0 %>
                     <span class="and_x_other_projects"><%= l("support.array.sentence_connector") %></span>
-                    <%= nb_other_projects %>
-                    <span class="and_x_other_projects"><%= nb_other_projects > 1 ? l("other").pluralize : l("other") %></span>
+                    <%= other_projects.count %>
+                    <span class="and_x_other_projects"><%= other_projects.count > 1 ? l("other").pluralize : l("other") %></span>
                   <% else %>
                     <span class="and_x_other_projects" style="display: none;"><%= l("support.array.sentence_connector") %></span>
-                    <%= nb_other_projects %>
-                    <span class="and_x_other_projects" style="display: none;"><%= nb_other_projects > 1 ? l("other").pluralize : l("other") %></span>
+                    <%= other_projects.count %>
+                    <span class="and_x_other_projects" style="display: none;"><%= other_projects.count > 1 ? l("other").pluralize : l("other") %></span>
                   <% end %>
-                  <%= nb_other_projects > 1 ? l("project").pluralize : l("project") %>
+                  <%= other_projects.count > 1 ? l("project").pluralize : l("project") %>
                 </span>
               <% end %>
             </div>
 
-            <%= link_to l("modify_projects"), '#', id: "link_update_project_list", :onclick => 'showModal("ajax-modal", "1000px");$("#button_apply_projects").focus();' %>
-
-            <script type="text/javascript">
-                $(document).ready(function () {
-                    $('#ajax-modal').html('<%= escape_javascript(render :partial => 'issue_templates/modal_select_projects') %>');
-                });
-            </script>
+            <%= link_to l("modify_projects"), '#', id: "link_update_project_list" %>
 
           </div>
 
@@ -223,7 +218,7 @@
 
         <% if Redmine::Plugin.installed?(:redmine_multiprojects_issue) %>
           <p id="projects_form">
-            <% select_options = project_tree_options_for_select((@issue_template.allowed_target_projects | @issue_template.secondary_projects), :selected => @issue_template.secondary_projects) %>
+            <% select_options = project_tree_options_for_select((allowed_projects | @issue_template.secondary_projects), :selected => @issue_template.secondary_projects) %>
             <%= f.select :secondary_project_ids,
                          select_options,
                          { :required => true, :label => l("related_projects") },
@@ -263,6 +258,32 @@
         $('#issue-template-form').bind('submit', function () {
             $(this).find(':input').removeAttr('disabled');
         });
+
+        function buildSelectprojectsModal(id){
+          var allowed_projects = <%= allowed_projects.sort_by(&:lft).pluck(:id, :name, :status, :lft, :rgt).to_json.html_safe %>
+
+          var project_ids = $('#issue_template_template_project_ids').val();
+          if (project_ids.length == 0) {
+            project_ids = ''
+          }
+
+          $.ajax({
+            url: "<%= render_select_projects_modal_by_ajax_path(template_id: @issue_template.id) %>",
+            type: 'POST',
+            data : { allowed_projects : allowed_projects, project_ids : project_ids },
+            success: function(response) {
+              $('#ajax-indicator').removeClass('bottom');
+              if(response){
+                $('#ajax-modal').html(response.html);
+                showModal("ajax-modal", "1000px");
+                $("#button_apply_projects").focus();
+              }
+            },error: function(response) {
+              $('#ajax-indicator').removeClass('bottom');
+              console.error(response);
+            },
+          });
+        }
     </script>
 
     <%= render 'valid_similar_existing_templates' if @issue_template.new_record? %>
@@ -274,3 +295,12 @@
   </div>
 
 </div>
+<script>
+  $(document).ready(function() {
+    $("#link_update_project_list").click(function (e) {
+      e.preventDefault();
+      buildSelectprojectsModal();
+      $('#ajax-indicator').show();
+    });
+  });
+</script>
diff --git a/app/views/issue_templates/_list_template.html.erb b/app/views/issue_templates/_list_template.html.erb
@@ -21,33 +21,33 @@
   <td class="template_projects">
     <div>
       <%
-        nb_other_projects = 0
-        nb_visible_projects = 0
-        template_projects = issue_template.template_projects.all
-        allowed_projects = issue_template.allowed_target_projects
+        template_projects = issue_template.template_projects.all.select('id','name')
+        allowed_projects = issue_template.allowed_target_projects.select('id','name')
+
+        visible_projects = (allowed_projects & template_projects).to_a
+        other_projects =  (template_projects - allowed_projects).to_a
+
+        nb_other_projects = other_projects.count
+        nb_visible_projects = visible_projects.count
       %>
-      <% template_projects.each do |project| %>
-        <% if allowed_projects.include?(project) %>
+
+      <% visible_projects.each do |project| %>
           <%= link_to project.name.gsub(/ /, "&nbsp;").html_safe,
                       "#",
-                      class: "project_id_#{project.id} list_templates_projects_names #{@project && @project == project ? "current" : ""}",
+                      class: "project_id_#{project.id} list_templates_projects_names #{@project && @project.id == project.id ? "current" : ""}",
                       :onclick => "$('.list_templates_projects_names').removeClass('current');$(\".project_id_#{project.id}\").toggleClass('current')"
           %>
-          <% nb_visible_projects += 1 %>
-        <% else %>
-          <% nb_other_projects += 1 %>
-        <% end %>
       <% end %>
       <% if nb_other_projects > 0 %>
-                <span class="list_templates_other_projects">
-                  <% if nb_visible_projects > 0 %>
-                    <%= l("support.array.sentence_connector") %>
-                    <%= pluralize(nb_other_projects, l("other")) %>
-                    <%= nb_other_projects > 1 ? l("project").pluralize : l("project") %>
-                  <% else %>
-                    <%= pluralize(nb_other_projects, l("project")) %>
-                  <% end %>
-                </span>
+          <span class="list_templates_other_projects">
+            <% if nb_visible_projects > 0 %>
+              <%= l("support.array.sentence_connector") %>
+              <%= pluralize(nb_other_projects, l("other")) %>
+              <%= nb_other_projects > 1 ? l("project").pluralize : l("project") %>
+            <% else %>
+              <%= pluralize(nb_other_projects, l("project")) %>
+            <% end %>
+          </span>
       <% end %>
     </div>
   </td>

diff --git a/app/views/issue_templates/_modal_select_projects.html.erb b/app/views/issue_templates/_modal_select_projects.html.erb
@@ -1,7 +1,7 @@
-<% template_projects = @issue_template.template_projects
-   allowed_projects = @issue_template.allowed_target_projects.includes(:custom_values)
+<% 
    custom_fields = CustomField.select("id, name").where("type = ? AND name IN (?)", "ProjectCustomField", Setting["plugin_redmine_templates"]['custom_fields'])
-   concerned_projects_ids = (allowed_projects | template_projects).map(&:id)
+
+   concerned_projects_ids = (@allowed_target_projects_attributes_array | @template_projects_attributes_array).map{ |el| el[0] }
 
    options_for_selects = {}
    custom_fields.each do |field|
@@ -34,22 +34,25 @@
 
    end
 
-   nested_projects_list = render_project_nested_lists(allowed_projects | template_projects) do |project|
+   nested_projects_list = render_project_nested_lists_by_attributes_for_template(@allowed_target_projects_attributes_array | @template_projects_attributes_array) do |project|
+      # Project is these informations [:id, :name, :status, :lft, :rgt] 
+      project_id = project[0]
+      project_name = project[1]
      custom_fields_data = {}
-     if allowed_projects.include?(project)
+     if @allowed_target_projects_attributes_array.include?(project)
        custom_fields.each do |f|
-         custom_fields_data.merge!(f.name.parameterize => custom_values[project.id][f.id])
+         custom_fields_data.merge!(f.name.parameterize => custom_values[project_id][f.id])
        end
      end
      content_tag('label',
                  check_box_tag(
                    'template_project_ids[]',
-                   project.id,
-                   @issue_template != nil && template_projects.include?(project),
-                   disabled: allowed_projects.include?(project) ? false : true,
-                   :class => ("inactive" unless allowed_projects.include?(project)),
+                   project_id,
+                   @issue_template != nil && @template_projects_attributes_array.include?(project),
+                   disabled: @allowed_target_projects_attributes_array.include?(project) ? false : true,
+                   :class => ("inactive" unless @allowed_target_projects_attributes_array.include?(project)),
                    data: custom_fields_data
-                 ) + ' ' + h(project.name), :class => ("inactive" unless allowed_projects.include?(project))
+                 ) + ' ' + h(project_name), :class => ("inactive" unless @allowed_target_projects_attributes_array.include?(project))
      )
    end
 %>

diff --git a/app/views/projects/_settings_issue_templates_tab.html.erb b/app/views/projects/_settings_issue_templates_tab.html.erb
@@ -71,22 +71,21 @@
             <td class="template_projects">
               <div>
                 <%
-                  nb_other_projects = 0
-                  nb_visible_projects = 0
                   template_projects_names = issue_template.template_projects.pluck(:name)
                   allowed_projects_names = issue_template.allowed_target_projects.pluck(:name)
+
+                  visible_projects = (allowed_projects_names & template_projects_names).to_a
+                  other_projects =  (template_projects_names - allowed_projects_names).to_a
+
+                  nb_other_projects = other_projects.count
+                  nb_visible_projects = visible_projects.count
                 %>
                 <% template_projects_names.each do |project_name| %>
-                  <% if allowed_projects_names.include?(project_name) %>
-                    <%= link_to project_name.gsub(/ /, "&nbsp;").html_safe,
-                                "#",
-                                class: "project_id_#{project_name.parameterize} list_templates_projects_names #{@project && @project.name == project_name ? "current" : ""}",
-                                :onclick => "$('.list_templates_projects_names').removeClass('current');$(\".project_id_#{project_name.parameterize}\").toggleClass('current')"
-                    %>
-                    <% nb_visible_projects += 1 %>
-                  <% else %>
-                    <% nb_other_projects += 1 %>
-                  <% end %>
+                  <%= link_to project_name.gsub(/ /, "&nbsp;").html_safe,
+                              "#",
+                              class: "project_id_#{project_name.parameterize} list_templates_projects_names #{@project && @project.name == project_name ? "current" : ""}",
+                              :onclick => "$('.list_templates_projects_names').removeClass('current');$(\".project_id_#{project_name.parameterize}\").toggleClass('current')"
+                  %>
                 <% end %>
                 <% if nb_other_projects > 0 %>
                   <span class="list_templates_other_projects">

diff --git a/config/routes.rb b/config/routes.rb
@@ -3,8 +3,9 @@
   match 'issue_templates/update_form', :controller => 'issue_templates', :action => 'update_form', :via => [:put, :post], :as => 'issue_template_form'
   post 'issue_templates/:id/enable' => "issue_templates#enable", :as => :enable_issue_template
   match 'issue_templates/similar_templates', :controller => 'issue_templates', :via => [:put, :post], :action => 'similar_templates'
-  post "issue_templates/add_repeatable_group" => "issue_templates#add_repeatable_group", :as => :add_repeatable_group
-
+  post "issue_templates/add_repeatable_group" => "issue_templates#add_repeatable_group", :as => :add_repeatable_group  
+  match 'issue_templates/render_select_projects_modal_by_ajax', :controller => 'issue_templates', :action => 'render_select_projects_modal_by_ajax', :via => :post,  :as => 'render_select_projects_modal_by_ajax'
+
   resources :issue_templates, except: [:show] do
     collection do
       get :custom_form

diff --git a/init.rb b/init.rb
@@ -7,17 +7,18 @@
   require_dependency 'redmine_templates/issues_controller_patch'
   require_dependency 'redmine_templates/projects_controller_patch'
   require_dependency 'redmine_templates/issue_patch'
-  require_dependency 'redmine_templates/helpers/projects_helper_patch'
   require_dependency 'redmine_templates/project_query_patch'
   require_dependency 'redmine_templates/queries_helper_patch'
   require_dependency 'redmine_templates/issue_query_patch'
-  require_dependency 'redmine_templates/helpers/issues_helper_patch'
   require_dependency 'redmine_templates/tracker_patch'
   require_dependency 'redmine_templates/issue_status_patch'
   require_dependency 'redmine_templates/issue_category_patch'
   require_dependency 'redmine_templates/user_patch'
   require_dependency 'redmine_templates/issue_priority_patch'
   require_dependency 'redmine_templates/typology_patch' if Redmine::Plugin.installed?(:redmine_typologies)
+  require_dependency 'redmine_templates/helpers/projects_helper_patch'
+  require_dependency 'redmine_templates/helpers/issues_helper_patch'
+  require_dependency 'redmine_templates/helpers/application_helper_patch'
 end
 
 Redmine::Plugin.register :redmine_templates do

diff --git a/lib/redmine_templates/helpers/application_helper_patch.rb b/lib/redmine_templates/helpers/application_helper_patch.rb
@@ -0,0 +1,54 @@
+require_dependency 'application_helper'
+
+module PluginRedmineTemplates
+  module ApplicationHelperPatch
+
+    # adapted from standard method "is_descendant_of?(other)"
+    def is_descendant_of_by_attributes_for_template?(project, other_project)
+      # project, other_project are represented by array [:id, :name, :status, :lft, :rgt]
+      project_lft = project[3]
+      project_rgt = project[4]
+      other_project_lft = other_project[3]
+      other_project_rgt = other_project[4]
+
+      other_project_lft < project_lft && other_project_rgt > project_rgt
+    end
+
+    # This function simulate  render_project_nested_lists, but projects are an array of attributes not activerecord
+    def render_project_nested_lists_by_attributes_for_template(projects, &block)
+      s = +''
+      if projects.any?
+        ancestors = []
+        # in the orginal method we write projects.sort_by(&:lft).each do |project|
+        # but here the projects array is already sorted by lft before sending by ajax
+        projects.each do |project|
+          project_status = project[0]
+          project_name = project[1]
+          # use is_descendant_of_by_attributes_for_template instead of is_descendant_of
+          if ancestors.empty? || is_descendant_of_by_attributes_for_template?(project, ancestors.last)
+            s << "<ul class='projects #{ancestors.empty? ? 'root' : nil}'>\n"
+          else
+            ancestors.pop
+            s << "</li>"
+            while ancestors.any? && !is_descendant_of_by_attributes_for_template?(project, ancestors.last)
+              ancestors.pop
+              s << "</ul></li>\n"
+            end
+          end
+          classes = (ancestors.empty? ? 'root' : 'child')
+          # use the condition project_status ==  Project::STATUS_ARCHIVED instead of  project.archived?
+          classes += ' archived' if project_status == Project::STATUS_ARCHIVED
+          s << "<li class='#{classes}'><div class='#{classes}'>"
+          s << ERB::Util.h(block_given? ? capture(project, &block) : project_name)
+          s << "</div>\n"
+          ancestors << project
+        end
+        s << ("</li></ul>\n" * ancestors.size)
+      end
+      s.html_safe
+    end
+  end
+end
+
+ApplicationHelper.prepend PluginRedmineTemplates::ApplicationHelperPatch
+ActionView::Base.prepend ApplicationHelper