Pass along the object's admin_set_id to the param sanitization to ens…

…ure that permissions params are returned or stripped based upon the active workflow for the admin set. Without this the batch edit of permissions silently fails because it doesn't see any params to operate on.

This commit also fixes the params for the visibility tests based upon browser testing.  The line reading the visibility from the bare param could be removed since it isn't used so I marked it as deprecated.

Fix visibility and use actor to correctly handle params

Make permissions collapsable like description tab

.rubocop_fixme.yml

@@ -6,6 +6,7 @@ Metrics/ClassLength:
   Exclude:
     - 'app/controllers/hyrax/dashboard/collections_controller.rb'
     - 'app/controllers/hyrax/admin/admin_sets_controller.rb'
+    - 'app/controllers/hyrax/batch_edits_controller.rb'
     - 'app/controllers/hyrax/downloads_controller.rb'
     - 'app/controllers/hyrax/file_sets_controller.rb'
     - 'app/forms/hyrax/forms/permission_template_form.rb'
@@ -83,6 +84,7 @@ RSpec/ExampleLength:
     - 'spec/actors/hyrax/actors/file_set_actor_spec.rb'
     - 'spec/actors/hyrax/actors/generic_work_actor_spec.rb'
     - 'spec/controllers/hyrax/api/items_controller_spec.rb'
+    - 'spec/controllers/hyrax/batch_edits_controller_spec.rb'
     - 'spec/controllers/hyrax/batch_uploads_controller_spec.rb'
     - 'spec/controllers/hyrax/file_sets_controller_spec.rb'
     - 'spec/controllers/hyrax/generic_works_controller_spec.rb'

app/assets/javascripts/hyrax/batch_edit.js

@@ -159,7 +159,8 @@ function batch_edit_init () {
         setTimeout(ajaxManager.runNow(), 100);
     }
 
-    $("#permissions_save").click(runSave);
+    $("#permissions_visibility_save").click(runSave);
+    $("#permissions_roles_save").click(runSave);
     $(".field-save").click(runSave);
 }
 

app/controllers/hyrax/batch_edits_controller.rb

@@ -46,9 +46,16 @@ def destroy_collection
     end
 
     def update_document(obj)
-      obj.attributes = work_params
+      interpret_visiblity_params(obj)
+      obj.attributes = work_params(admin_set_id: obj.admin_set_id).except(*visibility_params)
       obj.date_modified = Time.current.ctime
-      obj.visibility = params[:visibility]
+
+      if params[:visibility]
+        Deprecation.warn(self, "visibility is not submitted by the Hyrax UI and is deprecated " \
+                               "for removal in Hyrax 3.0. Please use " \
+                               "#{form_class.model_name.param_key}[visibility] instead.")
+        obj.visibility = params[:visibility]
+      end
 
       InheritPermissionsJob.perform_now(obj)
       VisibilityCopyJob.perform_now(obj)
@@ -94,9 +101,28 @@ def terms
         form_class.terms
       end
 
-      def work_params
+      def work_params(extra_params = {})
         work_params = params[form_class.model_name.param_key] || ActionController::Parameters.new
-        form_class.model_attributes(work_params)
+        form_class.model_attributes(work_params.merge(extra_params))
+      end
+
+      def interpret_visiblity_params(obj)
+        stack = ActionDispatch::MiddlewareStack.new.tap do |middleware|
+          middleware.use Hyrax::Actors::InterpretVisibilityActor
+        end
+        env = Hyrax::Actors::Environment.new(obj, current_ability, work_params(admin_set_id: obj.admin_set_id))
+        last_actor = Hyrax::Actors::Terminator.new
+        stack.build(last_actor).update(env)
+      end
+
+      def visibility_params
+        ['visibility',
+         'lease_expiration_date',
+         'visibility_during_lease',
+         'visibility_after_lease',
+         'embargo_release_date',
+         'visibility_during_embargo',
+         'visibility_after_embargo']
       end
 
       def redirect_to_return_controller

app/views/hyrax/batch_edits/edit.html.erb

@@ -50,25 +50,65 @@
         </div><!-- #descriptions_display -->
 
         <div id="permissions_display" class="tab-pane">
-          <%= simple_form_for @form.model,
-                              url: hyrax.batch_edits_path,
-                              method: :put,
-                              remote: true,
-                              html: { id: "form_permissions", class: "ajax-form"},
-                              data: { 'param-key' => @form.model_name.param_key } do |f| %>
-            <%= hidden_field_tag('update_type', 'update') %>
-            <%= hidden_field_tag('key', 'permissions') %>
-            <%= render "hyrax/base/form_permission", f: f %>
-            <%= render "hyrax/base/form_share", f: f %>
+          <div class="row">
+            <%= simple_form_for @form,
+                                url: hyrax.batch_edits_path,
+                                method: :put,
+                                remote: true,
+                                builder: Hyrax::FormBuilder,
+                                html: { id: "form_permissions_visibility", class: "ajax-form"},
+                                data: { 'param-key' => @form.model_name.param_key } do |f| %>
+              <div class="col-xs-12 col-sm-4">
+                <a class="accordion-toggle grey glyphicon-chevron-right-helper collapsed" data-toggle="collapse" href="#collapse_permissions_visibility" id="expand_link_permissions_visibility">
+                  <%= f.input_label "Visibility" %> <span class="chevron"></span>
+                </a>
+              </div>
+              <div id="collapse_permissions_visibility" class="collapse scrolly col-xs-12 col-sm-7">
+                <%= hidden_field_tag('update_type', 'update') %>
+                <%= hidden_field_tag('key', 'permissions') %>
+                <%= render "hyrax/base/form_permission", f: f %>
 
-            <% @form.batch_document_ids.each do |batch_id| %>
-              <%= hidden_field_tag "batch_document_ids[]", batch_id %>
+                <% @form.batch_document_ids.each do |batch_id| %>
+                  <%= hidden_field_tag "batch_document_ids[]", batch_id %>
+                <% end %>
+                <div>
+                  <%= f.submit "Save changes", class: 'btn btn-primary field-save', id: "permissions_visibility_save" %>
+                  <a class="btn btn-default" data-toggle="collapse" data-parent="#row_permissions_visibility" href="#collapse_permissions_visibility">Cancel </a>
+                  <div class="status fleft"></div>
+                </div>
+              </div>
             <% end %>
-            <div class="row">
-              <%= f.submit "Save changes", class: 'btn btn-primary', id: 'permissions_save' %>
-              <div id="status_permissions" class="status fleft"></div>
-            </div>
-          <% end %>
+          </div>
+
+          <div class="row">
+            <%= simple_form_for @form,
+                                url: hyrax.batch_edits_path,
+                                method: :put,
+                                remote: true,
+                                builder: Hyrax::FormBuilder,
+                                html: { id: "form_permissions", class: "ajax-form"},
+                                data: { 'param-key' => @form.model_name.param_key } do |f| %>
+              <div class="col-xs-12 col-sm-4">
+                <a class="accordion-toggle grey glyphicon-chevron-right-helper collapsed" data-toggle="collapse" href="#collapse_permissions_sharing" id="expand_link_permissions_sharing">
+                  <%= f.input_label "Sharing" %> <span class="chevron"></span>
+                </a>
+              </div>
+              <div id="collapse_permissions_sharing" class="collapse scrolly col-xs-12 col-sm-7">
+                <%= hidden_field_tag('update_type', 'update') %>
+                <%= hidden_field_tag('key', 'permissions') %>
+                <%= render "hyrax/base/form_share", f: f %>
+
+                <% @form.batch_document_ids.each do |batch_id| %>
+                  <%= hidden_field_tag "batch_document_ids[]", batch_id %>
+                <% end %>
+                <div>
+                  <%= f.submit "Save changes", class: 'btn btn-primary field-save', id: "permissions_sharing_save" %>
+                  <a class="btn btn-default" data-toggle="collapse" data-parent="#row_permissions_sharing" href="#collapse_permissions_sharing">Cancel </a>
+                  <div class="status fleft"></div>
+                </div>
+              </div>
+            <% end %>
+          </div>
         </div>
       </div> <!-- .tab-content -->
     </div>

spec/controllers/hyrax/batch_edits_controller_spec.rb

@@ -32,22 +32,27 @@
 
   describe "update" do
     let(:user) { build(:user) }
+    let(:admin_set) { create(:admin_set) }
+    let(:permission_template) { create(:permission_template, source_id: admin_set.id) }
+    let!(:workflow) { create(:workflow, allows_access_grant: true, active: true, permission_template_id: permission_template.id) }
     let!(:one) do
-      create(:work, creator: ["Fred"], title: ["abc"], language: ['en'], user: user)
+      create(:work, admin_set_id: admin_set.id, creator: ["Fred"], title: ["abc"], language: ['en'], user: user)
     end
 
     let!(:two) do
-      create(:work, creator: ["Fred"], title: ["abc"], language: ['en'], user: user)
+      create(:work, admin_set_id: admin_set.id, creator: ["Fred"], title: ["abc"], language: ['en'], user: user)
     end
 
     let!(:three) do
-      create(:work, creator: ["Fred"], title: ["abc"], language: ['en'])
+      create(:work, admin_set_id: admin_set.id, creator: ["Fred"], title: ["abc"], language: ['en'])
     end
 
     let!(:file_set) do
       create(:file_set, creator: ["Fred"])
     end
 
+    let(:release_date) { Time.zone.today + 2 }
+
     let(:mycontroller) { "hyrax/my/works" }
 
     before do
@@ -83,7 +88,7 @@
     end
 
     it "updates permissions" do
-      put :update, params: { update_type: "update", visibility: "authenticated" }
+      put :update, params: { update_type: "update", generic_work: { visibility: "authenticated" } }
       expect(response).to be_redirect
 
       work1 = GenericWork.find(one.id)
@@ -93,6 +98,76 @@
       expect(GenericWork.find(two.id).visibility).to eq "authenticated"
       expect(GenericWork.find(three.id).visibility).to eq "restricted"
     end
+
+    it 'creates leases' do
+      put :update, params: { update_type: "update",
+                             generic_work: { visibility: "lease", lease_expiration_date: release_date, visibility_during_lease: 'open', visibility_after_lease: 'restricted' } }
+      expect(response).to be_redirect
+
+      work1 = GenericWork.find(one.id)
+      expect(work1.visibility).to eq "open"
+      expect(work1.visibility_during_lease).to eq 'open'
+      expect(work1.visibility_after_lease).to eq 'restricted'
+      expect(work1.lease_expiration_date).to eq release_date
+      expect(work1.file_sets.map(&:visibility)).to eq ['open']
+      expect(work1.file_sets.map(&:visibility_during_lease)).to eq ['open']
+      expect(work1.file_sets.map(&:visibility_after_lease)).to eq ['restricted']
+      expect(work1.file_sets.map(&:lease_expiration_date)).to eq [release_date]
+
+      work2 = GenericWork.find(two.id)
+      expect(work2.visibility).to eq 'open'
+      expect(work2.visibility_during_lease).to eq 'open'
+      expect(work2.visibility_after_lease).to eq 'restricted'
+      expect(work2.lease_expiration_date).to eq release_date
+
+      work3 = GenericWork.find(three.id)
+      expect(work3.visibility).to eq 'restricted'
+      expect(work3.visibility_during_lease).to be_nil
+      expect(work3.visibility_after_lease).to be_nil
+      expect(work3.lease_expiration_date).to be_nil
+    end
+
+    it 'creates embargoes' do
+      put :update, params: { update_type: "update",
+                             generic_work: { visibility: "embargo", embargo_release_date: release_date, visibility_during_embargo: 'authenticated', visibility_after_embargo: 'open' } }
+      expect(response).to be_redirect
+
+      work1 = GenericWork.find(one.id)
+      expect(work1.visibility).to eq "authenticated"
+      expect(work1.visibility_during_embargo).to eq 'authenticated'
+      expect(work1.visibility_after_embargo).to eq 'open'
+      expect(work1.embargo_release_date).to eq release_date
+      expect(work1.file_sets.map(&:visibility)).to eq ['authenticated']
+      expect(work1.file_sets.map(&:visibility_during_embargo)).to eq ['authenticated']
+      expect(work1.file_sets.map(&:visibility_after_embargo)).to eq ['open']
+      expect(work1.file_sets.map(&:embargo_release_date)).to eq [release_date]
+
+      work2 = GenericWork.find(two.id)
+      expect(work2.visibility).to eq 'authenticated'
+      expect(work2.visibility_during_embargo).to eq 'authenticated'
+      expect(work2.visibility_after_embargo).to eq 'open'
+      expect(work2.embargo_release_date).to eq release_date
+
+      work3 = GenericWork.find(three.id)
+      expect(work3.visibility).to eq 'restricted'
+      expect(work3.visibility_during_embargo).to be_nil
+      expect(work3.visibility_after_embargo).to be_nil
+      expect(work3.embargo_release_date).to be_nil
+    end
+
+    context 'with roles' do
+      it 'updates roles' do
+        put :update, params: { update_type: "update", generic_work: { permissions_attributes: [{ type: 'person', access: 'read', name: '[email protected]' }] } }
+        expect(response).to be_redirect
+
+        work1 = GenericWork.find(one.id)
+        expect(work1.read_users).to include "[email protected]"
+        expect(work1.file_sets.map(&:read_users)).to eq [["[email protected]"]]
+
+        expect(GenericWork.find(two.id).read_users).to eq ["[email protected]"]
+        expect(GenericWork.find(three.id).read_users).to eq []
+      end
+    end
   end
 
   describe "#destroy_collection" do

spec/features/batch_edit_spec.rb

@@ -2,10 +2,16 @@
 
 RSpec.describe 'batch', type: :feature, clean_repo: true, js: true do
   let(:current_user) { create(:user) }
-  let!(:work1)       { create(:public_work, user: current_user) }
-  let!(:work2)       { create(:public_work, user: current_user) }
+  let(:admin_set) { create(:admin_set) }
+  let(:permission_template) { create(:permission_template, source_id: admin_set.id) }
+  let!(:workflow) { create(:workflow, allows_access_grant: true, active: true, permission_template_id: permission_template.id) }
+
+  let!(:work1)       { create(:public_work, admin_set_id: admin_set.id, user: current_user, members: [file_set]) }
+  let!(:work2)       { create(:public_work, admin_set_id: admin_set.id, user: current_user) }
+  let!(:file_set)    { create(:file_set) }
 
   before do
+    RoleMapper.byname[current_user.user_key] << 'donor'
     sign_in current_user
     visit '/dashboard/my/works'
     check 'check_all'
@@ -60,6 +66,47 @@
         page.find("input#generic_work_related_url[value='NEW related_url']")
       end
     end
+
+    it 'updates permissions and roles' do
+      click_on 'batch-edit'
+      find('#edit_permissions_link').click
+      expect(page).to have_content('Batch Edit Descriptions')
+
+      # Set visibility to private
+      within "#form_permissions_visibility" do
+        batch_edit_expand('permissions_visibility')
+        find('#generic_work_visibility_authenticated').click
+        find('#permissions_visibility_save').click
+        # This was `expect(page).to have_content 'Changes Saved'`, however in debugging,
+        # the `have_content` check was ignoring the `within` scoping and finding
+        # "Changes Saved" for other field areas
+        find('.status', text: 'Changes Saved')
+      end
+
+      within "#form_permissions" do
+        batch_edit_expand('permissions_sharing')
+        page.select('donor', from: 'new_group_name_skel')
+        page.select('View/Download', from: 'new_group_permission_skel')
+        page.find('#add_new_group_skel').click
+        find('#collapse_permissions_sharing table', text: 'View/Download')
+        find('#collapse_permissions_sharing table', text: 'donor')
+        find('#permissions_sharing_save').click
+        # This was `expect(page).to have_content 'Changes Saved'`, however in debugging,
+        # the `have_content` check was ignoring the `within` scoping and finding
+        # "Changes Saved" for other field areas
+        find('.status', text: 'Changes Saved')
+      end
+
+      # Visit work permissions and verify
+      visit "/concern/generic_works/#{work1.id}/edit#share"
+      page.find('#generic_work_visibility_authenticated:checked')
+      page.find('#share table', text: 'donor')
+
+      # Visit file permissions and verify
+      visit "concern/file_sets/#{work1.file_sets.first.id}/edit#permissions_display"
+      page.find('#file_set_visibility_authenticated:checked')
+      page.find('#permissions_display table', text: 'donor')
+    end
   end
 
   describe 'deleting' do