diff --git a/.rubocop_fixme.yml b/.rubocop_fixme.yml index d8b93c6638..fbf0f039ca 100644 --- a/.rubocop_fixme.yml +++ b/.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' diff --git a/app/assets/javascripts/hyrax/batch_edit.js b/app/assets/javascripts/hyrax/batch_edit.js index ccd9347eb9..f076af6980 100644 --- a/app/assets/javascripts/hyrax/batch_edit.js +++ b/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); } diff --git a/app/controllers/hyrax/batch_edits_controller.rb b/app/controllers/hyrax/batch_edits_controller.rb index a4fccd46b6..b9fb3349c0 100644 --- a/app/controllers/hyrax/batch_edits_controller.rb +++ b/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 diff --git a/app/views/hyrax/batch_edits/edit.html.erb b/app/views/hyrax/batch_edits/edit.html.erb index 28a7d06665..34cefe2292 100644 --- a/app/views/hyrax/batch_edits/edit.html.erb +++ b/app/views/hyrax/batch_edits/edit.html.erb @@ -50,25 +50,65 @@
- <%= 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 %> +
+ <%= 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| %> +
+ +
+
+ <%= 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 %> +
+ <%= f.submit "Save changes", class: 'btn btn-primary field-save', id: "permissions_visibility_save" %> + Cancel +
+
+
<% end %> -
- <%= f.submit "Save changes", class: 'btn btn-primary', id: 'permissions_save' %> -
-
- <% end %> +
+ +
+ <%= 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| %> +
+ +
+
+ <%= 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 %> +
+ <%= f.submit "Save changes", class: 'btn btn-primary field-save', id: "permissions_sharing_save" %> + Cancel +
+
+
+ <% end %> +
diff --git a/spec/controllers/hyrax/batch_edits_controller_spec.rb b/spec/controllers/hyrax/batch_edits_controller_spec.rb index 15ea9a0d49..846b8e3154 100644 --- a/spec/controllers/hyrax/batch_edits_controller_spec.rb +++ b/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: 'foo@bar.com' }] } } + expect(response).to be_redirect + + work1 = GenericWork.find(one.id) + expect(work1.read_users).to include "foo@bar.com" + expect(work1.file_sets.map(&:read_users)).to eq [["foo@bar.com"]] + + expect(GenericWork.find(two.id).read_users).to eq ["foo@bar.com"] + expect(GenericWork.find(three.id).read_users).to eq [] + end + end end describe "#destroy_collection" do diff --git a/spec/features/batch_edit_spec.rb b/spec/features/batch_edit_spec.rb index b50ec72e40..5d0ce8df9b 100644 --- a/spec/features/batch_edit_spec.rb +++ b/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