Joined "confirm publish" view, fixed bug for items that cant be publi…

…shed

app/views/assets/_asset_checkbox_row.html.erb

@@ -16,24 +16,25 @@
     end
   end
 
+  itemid="#{item.class.name}_#{item.id}"
 
   toggle ||=false
-  cb_parent_selector ||="div\##{item.class.name}_#{item.id}.split_button_parent"
+  cb_parent_selector ||="div\##{itemid}.split_button_parent"
   show_managers=true
-  managed_by_selector ||="div\##{item.class.name}_#{item.id}.managed_by_list"
+  managed_by_selector ||="div\##{itemid}.managed_by_list"
   show_permissions=true
-  permissions_selector ||="div\##{item.class.name}_#{item.id}.permission_list"
+  permissions_selector ||="div\##{itemid}.permission_list"
 
 -%>
-<div class="isa-tree <%= html_classes -%> <%= tree_class %> _<%= item.id %>">
+<div class="isa-tree <%= html_classes -%> <%= tree_class %> <%= itemid %>">
   <% if toggle %>
     <%= render :partial => 'assets/isa-tree-toggle', locals: { cb_parent_selector: cb_parent_selector } -%>
   <% end %>
   <% if can_view %>
     <div  style="display: inline; margin-right:1em">
       <%= render :partial => 'general/split_button_checkbox',
                  locals: { checkbox_id: publishing_item_param(item),
-                           checkbox_class: "#{item.class.name}_#{item.id}",
+                           checkbox_class: itemid,
                            checked: checked,
                            checkbox_text: "",
                            published: published,
@@ -52,7 +53,7 @@
     <div  style="display: inline; margin-right:1em">
       <%= render :partial => 'general/split_button_checkbox',
                  locals: { checkbox_id: publishing_item_param(item),
-                           checkbox_class: "#{item.class.name}_#{item.id}",
+                           checkbox_class: itemid,
                            not_visible: true,
                            toggle: toggle,
                            cb_parent_selector: cb_parent_selector} -%>
@@ -69,7 +70,7 @@
         data-managed_by_selector='<%= managed_by_selector %>'>
       <span class="glyphicon glyphicon-user" aria-hidden="true" style="position:relative; top:2px;" ></span>
     </a>
-    <div class="managed_by_list" id="<%="#{item.class.name}_#{item.id}"-%>" style="display:none;margin-left:5em">
+    <div class="managed_by_list" id="<%= itemid -%>" style="display:none;margin-left:5em">
       <strong>Manageable by:</strong> <%= item.managers.empty? ? "<span class='none_text'>None</span>".html_safe : item.managers.collect { |m| link_to(h(m.title), m) }.join(", ").html_safe -%>
     </div>
   <% end %>

app/views/assets/publishing/publish_final_confirmation.html.erb

@@ -1,21 +1,74 @@
 <%= show_title "Confirm publishing" -%>
-<p>
-  You are about the publish the following items.
-</p>
-<%= form_tag :action => :publish do %>
-  <ul class="publishing_options">
-    <% @items_for_publishing.each do |item| %>
-      <%= hidden_field_tag publishing_item_param(item), 1 %>
+
+<% unless @items_for_immediate_publishing.empty? -%>
+  <h2>The following items will be published:</h2>
+  <div class="alert alert-info">
+    <p> Select "Confirm" to make the following item(s) immediately accessible to the public. </p>
+  </div>
+  <ul class="publishing_options" id="publish_immediately" style="margin-left: 2em">
+    <% @items_for_immediate_publishing.each do |item| %>
       <li class="type_and_title">
         <%= text_for_resource item -%>: <%= link_to item.title, item -%>
       </li>
     <% end %>
   </ul>
+<% end -%>
+
+<% unless @waiting_for_publish_items.empty? -%>
+  <h2>The following items require approval:</h2>
+  <div class="alert alert-warning">
+    <p>
+      One or more of the items to be published are associated with a <%= t('project') -%>
+      that has chosen to use the protection of the <%= t('asset_gatekeeper').downcase %>.
+      The <%= t('asset_gatekeeper').downcase %> is a person that needs to approve items before they are finally published.
+    </p>
+    <p>
+      When you click "Confirm" an email will be sent to that person, and they will either approve or reject the publication.
+      Once this has happened, you will be notified back via an email.
+    </p>
+  </div>
+  <ul class="publishing_options" id="waiting_approval" style="margin-left: 2em">
+    <% @waiting_for_publish_items.each do |item| %>
+      <li class="type_and_title"><%= text_for_resource item -%>
+        :&nbsp;<%= link_to item.title, item, :target => "_blank" -%></li>
+      <ul><li class="secondary">Approval required from: <%= item.asset_gatekeepers.collect { |m| link_to(m.title, m) }.join(" or ").html_safe -%></li></ul>
+      <br/>
+  <% end %>
+  </ul>
+<% end -%>
+
+<% unless @items_cannot_publish.blank? -%>
+  <h2>The following items cannot be published:</h2>
+  <div class="alert alert-danger">
+    <p> One or more of the items you selected cannot be published.</p>
+    <p>
+      It is likely that the item(s) are associated with a <%= t('project') -%>
+      that has chosen to use the protection of the <%= t('asset_gatekeeper').downcase %>,
+      and an existing request is either waiting approval or it has been rejected.
+      Visit the item(s) page to confirm this.
+    </p>
+    <p>If you think this is not the case, please contact your system's administrator.</p>
+  </div>
+  <ul class="publishing_options" id="cannot_publish" style="margin-left: 2em">
+    <% @items_cannot_publish.each do |item| %>
+      <li class="type_and_title"><%= text_for_resource item -%>
+        :&nbsp;<%= link_to item.title, item, :target => "_blank" -%></li>
+      <br/>
+    <% end %>
+  </ul>
+<% end -%>
+
+<%= form_tag :action => :publish do %>
+  <div style="display:none;">
+    <% @items_for_publishing.each do |item| %>
+      <%= hidden_field_tag publishing_item_param(item), 1 %>
+      <br/>
+    <% end %>
+  </div>
   <br/>
 
   <% resource = (controller_name == 'people') ? current_user.person : @asset %>
-
-  <%= submit_tag "Confirm",data: { disable_with: 'Confirm' }, :class => 'btn btn-primary' -%>
+  <%= submit_tag "Confirm", data: { disable_with: 'Confirm' }, :class => 'btn btn-primary' -%>
   or
   <%= cancel_button(resource) %>
 <% end %>

lib/seek/publishing/publishing_common.rb

@@ -37,12 +37,8 @@ def publish_related_items
       def check_gatekeeper_required
         @waiting_for_publish_items = @items_for_publishing.select { |item| item.gatekeeper_required? && !User.current_user.person.is_asset_gatekeeper_of?(item) }
         @items_for_immediate_publishing = @items_for_publishing - @waiting_for_publish_items
-        unless @waiting_for_publish_items.empty?
-          respond_to do |format|
-            format.html { render template: 'assets/publishing/waiting_approval_list' }
-          end
-        else
-          publish_final_confirmation
+        respond_to do |format|
+          format.html { render template: 'assets/publishing/publish_final_confirmation' }
         end
       end
 
@@ -53,12 +49,6 @@ def update_sharing_policies(*args)
         super
       end
 
-      def publish_final_confirmation
-        respond_to do |format|
-          format.html { render template: 'assets/publishing/publish_final_confirmation' }
-        end
-      end
-
       def publish
         publish_requested_items
 
@@ -169,6 +159,7 @@ def set_investigations
       # sets the @items_for_publishing based on the :publish param, and filtered by whether than can_publish?
       def set_items_for_publishing
         @items_for_publishing = resolve_publish_params(params[:publish]).select(&:can_publish?)
+        @items_cannot_publish = resolve_publish_params(params[:publish]) - @items_for_publishing
       end
 
       # sets the @items_for_publishing based on the :publish param, and filtered by whether than can_publish? OR contains_publishable_items?

test/functional/publishing/batch_publishing_test.rb

@@ -73,17 +73,70 @@ def setup
     end
   end
 
-  test 'do not have not-publishable items in batch_publishing_preview' do
+  test 'Cannot select already-published items in batch_publishing_preview' do
     published_item = FactoryBot.create(:data_file,
-                             contributor: User.current_user.person,
-                             policy: FactoryBot.create(:public_policy))
-    assert !published_item.can_publish?, 'This data file must not be publishable for the test to succeed'
+                                       contributor: User.current_user.person,
+                                       policy: FactoryBot.create(:public_policy))
+    assert published_item.is_published?
 
     get :batch_publishing_preview, params: { id: User.current_user.person.id }
     assert_response :success
 
-    assert_select "input[type='checkbox'][id=?]", "publish_#{published_item.class.name}_#{published_item.id}",
-                  count: 0
+    assert_select ".isa-tree.#{published_item.class.name}_#{published_item.id}", count: 2 do
+      assert_select "input[type='checkbox']", count: 0
+    end
+  end
+
+  test 'publish_final_confirmation' do
+    gatekeeper = FactoryBot.create(:asset_gatekeeper)
+    person = User.current_user.person
+    gatekept_project = gatekeeper.projects.first
+    publishable = FactoryBot.create(:data_file, projects: [person.projects.first], contributor: person)
+    gatekept = FactoryBot.create(:data_file, projects: [gatekept_project], contributor: person)
+    waiting = FactoryBot.create(:data_file, projects: [gatekept_project], contributor: person)
+    ResourcePublishLog.add_log(ResourcePublishLog::WAITING_FOR_APPROVAL, waiting, nil, person.user)
+
+    login_as(person.user)
+    assert publishable.can_publish?
+    assert gatekept.can_publish?
+    assert gatekept.gatekeeper_required?
+    assert !waiting.can_publish?
+
+    params = { publish: {} }
+    [publishable, gatekept, waiting].each do |asset|
+      params[:publish][asset.class.name] ||= {}
+      params[:publish][asset.class.name][asset.id.to_s] = '1'
+    end
+
+    get :check_gatekeeper_required, params: params.merge(id: person.id)
+    assert_response :success
+    pp(params.merge(id: person.id))
+
+    # Publishable shown in immediate publishing section
+    assert_select 'h2', text: /The following items will be published:/, count: 1
+    assert_select 'div.alert-info', text: /immediately accessible to the public/, count: 1
+    assert_select 'ul.publishing_options#publish_immediately', count: 1 do
+      assert_select 'li.type_and_title', count: 1 do
+        assert_select 'a[href=?]', data_file_path(publishable), text: publishable.title
+      end
+    end
+    # Gatekept explains waiting approval
+    assert_select 'h2', text: /The following items require approval:/, count: 1
+    assert_select 'div.alert-warning', text: /an email will be sent to that person, and they will either approve or reject/, count: 1
+    assert_select 'ul.publishing_options#waiting_approval', count: 1 do
+      assert_select 'li.type_and_title', count: 1 do
+        assert_select 'a[href=?]', data_file_path(gatekept), text: gatekept.title
+      end
+    end
+
+    # If not-publishable warning is issued
+    assert_select 'h2', text: /The following items cannot be published:/, count: 1
+    assert_select 'div.alert-danger', text: /One or more of the items you selected cannot be published/, count: 1
+    assert_select 'ul.publishing_options#cannot_publish', count: 1 do
+      assert_select 'li.type_and_title', count: 1 do
+        assert_select 'a[href=?]', data_file_path(waiting), text: waiting.title
+      end
+    end
   end
 
   test 'do not have not_publishable_type item in batch_publishing_preview' do

test/functional/publishing/single_publishing_test.rb

@@ -164,7 +164,7 @@ def setup
 
     # split-button dropdown menu shown for tree branches
     should_have_dropdown.each do |asset|
-      assert_select "._#{asset.id}", count: 1 do
+      assert_select ".isa-tree.#{asset.class.name}_#{asset.id}", count: 1 do
         assert_select '.parent-btn-dropdown', count: 1
         assert_select '.dropdown-menu', count: 1 do
           assert_select 'li', count: 2 do
@@ -181,7 +181,7 @@ def setup
     end
     # split-button dropdown menu not shown for tree leafs
     should_not_have_dropdown.each do |asset|
-      assert_select "._#{asset.id}", count: 1 do
+      assert_select ".isa-tree.#{asset.class.name}_#{asset.id}", count: 1 do
         assert_select '.parent-btn-dropdown', count: 0
         assert_select '.dropdown-menu', count: 0
       end