Render batch change results in partial instead of flash

Avoid some exception use

app/views/assets/sharing/batch_sharing_permission_preview.html.erb

@@ -3,6 +3,7 @@
   @policy_params = params[:policy_attributes]
 
 %>
+<%= render partial: 'sharing/batch_change_results' %>
 
 <%= show_title "Change permissions of items related to #{link_to(h(current_user.person.name),current_user.person)} in batch".html_safe -%>
 

app/views/isa_studies/_buttons.html.erb

@@ -21,6 +21,10 @@ exportToExcel||="sampleExportExcel()"
   function loadBatchPermission(dtName, e){
     const sample_ids = window[dtName].selectedSampleIds()
     let publish_list = {'Sample': {}}
+    if (!sample_ids.length) {
+        alert('Please select at least one sample.');
+        return;
+    }
     for(let id of sample_ids){ publish_list['Sample'][id]='1' }
 
     $j.ajax({

app/views/sharing/_batch_change_results.html.erb

@@ -0,0 +1,36 @@
+<% return unless @items_for_sharing %>
+<% items_text = 'item'.pluralize(@items_for_sharing.size) %>
+<% if @success&.any? || @gatekeeper_required&.any? %>
+  <div id="notice_flash" class="fade in alert alert-success alert-dismissable" role="alert">
+    <% if @success.any? %>
+      The sharing policies for your selected <%= items_text -%> were successfully updated:
+      <ul id="ok">
+        <% @success.each do |item| %>
+          <li><%= item.title %></li>
+        <% end %>
+      </ul>
+    <% end %>
+
+    <% if @gatekeeper_required.any? %>
+      Publishing the following <%= items_text -%> requires approval from a gatekeeper:
+      <ul id="gk">
+        <% @gatekeeper_required.each do |item| %>
+          <li><%= item.title %></li>
+        <% end %>
+      </ul>
+      The <%= items_text -%> will not be published until approved.
+    <% end %>
+  </div>
+<% elsif @error&.any? %>
+  <div id="notice_flash" class="fade in alert alert-danger alert-dismissable" role="alert">
+    The sharing policies for your selected <%= items_text -%> were not successfully updated:
+    <ul>
+      <% @error.each do |item| %>
+        <li>
+          <%= item.title %><br>
+          The reason: <%= item.errors.full_messages.join(', ') %>
+        </li>
+      <% end %>
+    </ul>
+  </div>
+<% end %>

app/views/single_pages/sample_batch_sharing_permissions_changed.html.erb

@@ -1,17 +1,3 @@
-<%
-  flash_exists = flash[:error] || flash[:notice]
-%>
-
-<% if flash_exists %>
-  <div id="notice_flash" class="fade in alert alert-<%= flash[:notice] ? "success" : "danger" %> alert-dismissable" role="alert">
-    <% if flash[:notice] %>
-      <%= flash[:notice]%>
-    <% else%>
-      <%= flash[:error]%>
-      <% flash[:error]=nil %>
-    <% end %>
-  </div>
-    <a id="hide-permissions-changed" class=" btn btn-default" onclick="$j('#change-batch-permission-modal').modal('hide')">Ok</a>
-<% end %>
-
+<%= render partial: 'sharing/batch_change_results' %>
 
+<a id="hide-permissions-changed" class=" btn btn-default" onclick="$j('#change-batch-permission-modal').modal('hide')">OK</a>

lib/seek/publishing/publishing_common.rb

@@ -142,16 +142,14 @@ def set_investigations
         @assets.each do |type, klass|
           next if %w[Investigation Study Assay].include? type
           klass.each do |asset|
-            if asset.investigations.empty?
+            if !asset.respond_to?(:investigations) || asset.investigations.empty?
               @assets_not_in_isa.push(asset)
             else
               asset.investigations.each do |inv|
                 next if @investigations.include?(inv)
                 @investigations.push(inv)
               end
             end
-            rescue NoMethodError
-              @assets_not_in_isa.push(asset)
           end
         end
       end

lib/seek/sharing/sharing_common.rb

@@ -37,35 +37,23 @@ def batch_sharing_permission_preview
       def batch_sharing_permission_changed
         @items_for_sharing = resolve_sharing_params(params[:publish])
         @batch_sharing_permission_changed = true
-        notice_count = 0
-        gatekeeper_count = 0
-        error_count = 0
-        flash[:notice] = "The sharing policies for your selected #{"item".pluralize(@items_for_sharing.size)} were successfully updated:<ul id=\"ok\"> "
-        gatekeeper_flash = "Publishing the following #{"item".pluralize(@items_for_sharing.size)} requires approval from a gatekeeper:<ul id=\"gk\"> "
-        flash[:error] = "The sharing policies for your selected #{"item".pluralize(@items_for_sharing.size)} were not successfully updated:<ul> "
+        @success = []
+        @gatekeeper_required = []
+        @error = []
         @items_for_sharing.each do |item|
           item.policy.update_with_bulk_sharing_policy(policy_params) if policy_params.present?
-          begin
-            item.save!
+          if item.save
             request_publish_approval_batch_sharing(item)  # Has to go before log_publishing_batch_sharing(item)
             log_publishing_batch_sharing(item)
             if item.is_waiting_approval? && is_gatekeeper_approval_required?(item)
-              gatekeeper_count += 1
-              gatekeeper_flash += "<li>#{item.title}</li>"
+              @gatekeeper_required << item
             else
-              notice_count += 1
-              flash[:notice] += "<li>#{item.title}</li>"
+              @success << item
             end
-          rescue Exception => e
-            error_count += 1
-            flash[:error] += "<li>#{item.title}<br>"
-            flash[:error] += "The reason:  #{e.message}</li>"
+          else
+            @error << item
           end
         end
-        flash[:notice] = notice_count == 0 ? "": (flash[:notice]+"</ul>").html_safe
-        flash[:notice] += gatekeeper_count == 0 ? "": (gatekeeper_flash+"</ul>"+"The #{"item".pluralize(gatekeeper_count)} will not be published until approved.").html_safe
-        flash.now[:notice] = (notice_count+gatekeeper_count) == 0 ? nil: flash[:notice].html_safe
-        flash.now[:error] = error_count == 0 ? nil: (flash[:error]+"</ul>").html_safe
         if params[:single_page]
           render 'single_pages/sample_batch_sharing_permissions_changed', { layout: false }
         else