From 5efe05f524ff25b69691c8c539ce74087de57980 Mon Sep 17 00:00:00 2001 From: fherreazcue Date: Thu, 7 Sep 2023 17:19:25 +0100 Subject: [PATCH] Joined "confirm publish" view, fixed bug for items that cant be published --- app/views/assets/_asset_checkbox_row.html.erb | 15 ++-- .../publish_final_confirmation.html.erb | 71 ++++++++++++++++--- .../publishing/waiting_approval_list.html.erb | 48 ------------- lib/seek/publishing/publishing_common.rb | 15 +--- .../publishing/batch_publishing_test.rb | 65 +++++++++++++++-- .../publishing/single_publishing_test.rb | 4 +- 6 files changed, 134 insertions(+), 84 deletions(-) delete mode 100644 app/views/assets/publishing/waiting_approval_list.html.erb diff --git a/app/views/assets/_asset_checkbox_row.html.erb b/app/views/assets/_asset_checkbox_row.html.erb index 1edfb7021a..c209c2ec83 100644 --- a/app/views/assets/_asset_checkbox_row.html.erb +++ b/app/views/assets/_asset_checkbox_row.html.erb @@ -16,16 +16,17 @@ 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" -%> -
+
<% if toggle %> <%= render :partial => 'assets/isa-tree-toggle', locals: { cb_parent_selector: cb_parent_selector } -%> <% end %> @@ -33,7 +34,7 @@
<%= 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 @@
<%= 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 %>'> -
" style="display:none;margin-left:5em"> + <% end %> diff --git a/app/views/assets/publishing/publish_final_confirmation.html.erb b/app/views/assets/publishing/publish_final_confirmation.html.erb index fc24050d67..a4130628ff 100644 --- a/app/views/assets/publishing/publish_final_confirmation.html.erb +++ b/app/views/assets/publishing/publish_final_confirmation.html.erb @@ -1,21 +1,74 @@ <%= show_title "Confirm publishing" -%> -

- You are about the publish the following items. -

-<%= form_tag :action => :publish do %> -
    - <% @items_for_publishing.each do |item| %> - <%= hidden_field_tag publishing_item_param(item), 1 %> + +<% unless @items_for_immediate_publishing.empty? -%> +

    The following items will be published:

    +
    +

    Select "Confirm" to make the following item(s) immediately accessible to the public.

    +
    +
      + <% @items_for_immediate_publishing.each do |item| %>
    • <%= text_for_resource item -%>: <%= link_to item.title, item -%>
      • <% end %>
    +<% end -%> + +<% unless @waiting_for_publish_items.empty? -%> +

    The following items require approval:

    +
    +

    + 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. +

    +

    + 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. +

    +
    +
      + <% @waiting_for_publish_items.each do |item| %> +
    • <%= text_for_resource item -%> + : <%= link_to item.title, item, :target => "_blank" -%>
      • +
      • Approval required from: <%= item.asset_gatekeepers.collect { |m| link_to(m.title, m) }.join(" or ").html_safe -%>
      +
      + <% end %> +
    +<% end -%> + +<% unless @items_cannot_publish.blank? -%> +

    The following items cannot be published:

    +
    +

    One or more of the items you selected cannot be published.

    +

    + 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. +

    +

    If you think this is not the case, please contact your system's administrator.

    +
    +
      + <% @items_cannot_publish.each do |item| %> +
    • <%= text_for_resource item -%> + : <%= link_to item.title, item, :target => "_blank" -%>
      • +
      + <% end %> +
    +<% end -%> + +<%= form_tag :action => :publish do %> +
    + <% @items_for_publishing.each do |item| %> + <%= hidden_field_tag publishing_item_param(item), 1 %> +
    + <% end %> +

    <% 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 %> diff --git a/app/views/assets/publishing/waiting_approval_list.html.erb b/app/views/assets/publishing/waiting_approval_list.html.erb deleted file mode 100644 index 531339fcc4..0000000000 --- a/app/views/assets/publishing/waiting_approval_list.html.erb +++ /dev/null @@ -1,48 +0,0 @@ -<%= show_title "One #{t('asset_gatekeeper').downcase } will need to approve the items to be published" -%> - -
    -

    - 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. -

    -

    - The affected items, and the related <%= t('asset_gatekeeper').downcase %>, are listed below. When you Submit 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. -

    -
    -

    The following items require approval from one of the <%= t('asset_gatekeeper').downcase.pluralize %>:

    -
      - <% @waiting_for_publish_items.each do |item| %> -
    • <%= text_for_resource item -%> - : <%= link_to item.title, item, :target => "_blank" -%>
      • -
    • Approval required from: <%= item.asset_gatekeepers.collect { |m| link_to(m.title, m) }.join(" or ").html_safe -%>
      • -
      - <% end %> -
    -<% unless @items_for_immediate_publishing.empty? -%> -

    The following items will be published immediately and do not require approval from the <%= t('asset_gatekeeper').downcase %>:

    -
      - <% @items_for_immediate_publishing.each do |item| %> -
    • - <%= text_for_resource item -%>: <%= link_to item.title, item -%> -
      • - <% end %> -
    -<% end -%> - -<%= form_tag :action => :publish do %> -
    - <% @items_for_publishing.each do |item| %> - <%= hidden_field_tag publishing_item_param(item), 1 %> -
    - <% end %> -
    -
    - - <% resource = (controller_name == 'people') ? current_user.person : @asset %> - <%= submit_tag "Confirm", data: { disable_with: 'Confirm' }, :class => 'btn btn-primary' -%> - or - <%= cancel_button(resource) %> -<% end %> diff --git a/lib/seek/publishing/publishing_common.rb b/lib/seek/publishing/publishing_common.rb index 7e323cb394..57104602be 100644 --- a/lib/seek/publishing/publishing_common.rb +++ b/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? diff --git a/test/functional/publishing/batch_publishing_test.rb b/test/functional/publishing/batch_publishing_test.rb index d8e4476a82..ff98852ca9 100644 --- a/test/functional/publishing/batch_publishing_test.rb +++ b/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 diff --git a/test/functional/publishing/single_publishing_test.rb b/test/functional/publishing/single_publishing_test.rb index 0943ba4a52..dca13e6a99 100644 --- a/test/functional/publishing/single_publishing_test.rb +++ b/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