From 7808e83d75088252cec65ebf1ba6b6516fdad05b Mon Sep 17 00:00:00 2001 From: David Campbell Date: Mon, 3 Jun 2024 13:59:59 -0400 Subject: [PATCH 01/11] generating html that is non visibile to the user containing admin set options to restrict the visibility options provided to the user Closes #6822 --- .../javascripts/hyrax/permissions/control.es6 | 4 +++- app/controllers/hyrax/file_sets_controller.rb | 14 ++++++++++++++ app/helpers/hyrax/hyrax_helper_behavior.rb | 9 ++++++--- .../hyrax/file_sets/_admin_set_options.html.erb | 7 +++++++ app/views/hyrax/file_sets/_permission.html.erb | 5 +++++ .../hyrax/file_sets/_permission_form.html.erb | 2 +- 6 files changed, 36 insertions(+), 5 deletions(-) create mode 100644 app/views/hyrax/file_sets/_admin_set_options.html.erb diff --git a/app/assets/javascripts/hyrax/permissions/control.es6 b/app/assets/javascripts/hyrax/permissions/control.es6 index 48d1b8abbf..9e0b4487ea 100644 --- a/app/assets/javascripts/hyrax/permissions/control.es6 +++ b/app/assets/javascripts/hyrax/permissions/control.es6 @@ -2,6 +2,7 @@ import { Registry } from './registry' import { UserControls } from './user_controls' import { GroupControls } from './group_controls' import VisibilityComponent from '../save_work/visibility_component' +import AdminSetWidget from 'hyrax/editor/admin_set_widget' export default class PermissionsControl { /** @@ -14,13 +15,14 @@ export default class PermissionsControl { if (element.length === 0) { return } + this.adminSetWidget = new AdminSetWidget(element.find('select[id="admin_set_id"]')) this.element = element this.registry = new Registry(this.element, this.object_name(), template_id) this.user_controls = new UserControls(this.element, this.registry) this.group_controls = new GroupControls(this.element, this.registry) if (with_visibility_component) { - this.visibility_component = new VisibilityComponent(this.element) + this.visibility_component = new VisibilityComponent(this.element, this.adminSetWidget) } else { this.visibility_component = null } diff --git a/app/controllers/hyrax/file_sets_controller.rb b/app/controllers/hyrax/file_sets_controller.rb index 11bd5a6fe9..6733351328 100644 --- a/app/controllers/hyrax/file_sets_controller.rb +++ b/app/controllers/hyrax/file_sets_controller.rb @@ -43,6 +43,7 @@ class FileSetsController < ApplicationController # GET /concern/file_sets/:id def edit + @file_set_admin_set_option = file_set_admin_set_option initialize_edit_form end @@ -229,6 +230,19 @@ def add_breadcrumb_for_action end end + # Retrieves the admin set the file_set belongs to + def file_set_admin_set_option + return @admin_set_option if @admin_set_option + parent_work = parent(file_set: presenter) + admin_set_tesim = parent_work.solr_document[:admin_set_tesim].first + + service = Hyrax::AdminSetService.new(self) + admin_set_option = Hyrax::AdminSetOptionsPresenter.new(service).select_options.reject do |option| + option[0] != admin_set_tesim + end + admin_set_option + end + def initialize_edit_form guard_for_workflow_restriction_on!(parent: parent) diff --git a/app/helpers/hyrax/hyrax_helper_behavior.rb b/app/helpers/hyrax/hyrax_helper_behavior.rb index bd1d6030df..60006245e4 100644 --- a/app/helpers/hyrax/hyrax_helper_behavior.rb +++ b/app/helpers/hyrax/hyrax_helper_behavior.rb @@ -23,9 +23,12 @@ module HyraxHelperBehavior ## # @return [Array] the list of all user groups def available_user_groups(ability:) - return ::User.group_service.role_names if ability.admin? - - ability.user_groups + user_groups = ability.user_groups.dup + return user_groups if ability.admin? + # Excluding "public" and "registered" groups if non-admin user + user_groups.delete("public") + user_groups.delete("registered") + user_groups end # Which translations are available for the user to select diff --git a/app/views/hyrax/file_sets/_admin_set_options.html.erb b/app/views/hyrax/file_sets/_admin_set_options.html.erb new file mode 100644 index 0000000000..ff75ace92b --- /dev/null +++ b/app/views/hyrax/file_sets/_admin_set_options.html.erb @@ -0,0 +1,7 @@ + +<% if Flipflop.assign_admin_set? %> + <%= select_tag :admin_set_id, + options_for_select(@file_set_admin_set_option.map { |option| [option[0], option[1], option[2]] }), + include_blank: false, + class: 'form-control d-none' %> +<% end %> \ No newline at end of file diff --git a/app/views/hyrax/file_sets/_permission.html.erb b/app/views/hyrax/file_sets/_permission.html.erb index d0f3dcb0d3..47f104598d 100644 --- a/app/views/hyrax/file_sets/_permission.html.erb +++ b/app/views/hyrax/file_sets/_permission.html.erb @@ -5,6 +5,11 @@ data: { param_key: file_set.model_name.param_key }, class: 'nav-safety' } do |f| %> +
+
+ <%= render 'admin_set_options' %> +
+
<%= hidden_field_tag('redirect_tab', 'permissions') %> <%= render "permission_form", f: f %>
diff --git a/app/views/hyrax/file_sets/_permission_form.html.erb b/app/views/hyrax/file_sets/_permission_form.html.erb index 93da4743a3..ac2d1b2340 100644 --- a/app/views/hyrax/file_sets/_permission_form.html.erb +++ b/app/views/hyrax/file_sets/_permission_form.html.erb @@ -49,7 +49,7 @@
- <%= select_tag 'new_group_name_skel', options_for_select([t('.select_group')] + current_user.groups), class: 'form-control' %> + <%= select_tag 'new_group_name_skel', options_for_select([t('.select_group')] + available_user_groups(ability: current_ability)), class: 'form-control' %>
From 489bc41ff5505ab2b2528c8690bfb667f1b95f58 Mon Sep 17 00:00:00 2001 From: David Campbell Date: Tue, 4 Jun 2024 16:26:24 -0400 Subject: [PATCH 02/11] nil check --- app/views/hyrax/file_sets/_admin_set_options.html.erb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/views/hyrax/file_sets/_admin_set_options.html.erb b/app/views/hyrax/file_sets/_admin_set_options.html.erb index ff75ace92b..31e7866ce7 100644 --- a/app/views/hyrax/file_sets/_admin_set_options.html.erb +++ b/app/views/hyrax/file_sets/_admin_set_options.html.erb @@ -1,7 +1,9 @@ <% if Flipflop.assign_admin_set? %> + <% if @file_set_admin_set_option.present? %> <%= select_tag :admin_set_id, options_for_select(@file_set_admin_set_option.map { |option| [option[0], option[1], option[2]] }), include_blank: false, class: 'form-control d-none' %> + <% end %> <% end %> \ No newline at end of file From dc49eb35ec0284c0f0ea15e636a553255520ad8a Mon Sep 17 00:00:00 2001 From: David Campbell Date: Tue, 4 Jun 2024 16:39:35 -0400 Subject: [PATCH 03/11] extracting solr document from assigns[:parent] for test comparison --- spec/controllers/hyrax/file_sets_controller_spec.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/controllers/hyrax/file_sets_controller_spec.rb b/spec/controllers/hyrax/file_sets_controller_spec.rb index 115844c185..8cbc2c4bef 100644 --- a/spec/controllers/hyrax/file_sets_controller_spec.rb +++ b/spec/controllers/hyrax/file_sets_controller_spec.rb @@ -74,7 +74,9 @@ expect(response).to be_successful expect(assigns[:file_set]).to eq file_set expect(assigns[:version_list]).to be_kind_of Hyrax::VersionListPresenter - expect(assigns[:parent]).to eq parent + parent_solr_document = assigns[:parent].solr_document + parent_work = ActiveFedora::Base.find(parent_solr_document.id) + expect(parent_work).to eq parent expect(response).to render_template(:edit) expect(response).to render_template('dashboard') end From 6e6125649af8052212386a0664e3b310e9be9d82 Mon Sep 17 00:00:00 2001 From: David Campbell Date: Tue, 4 Jun 2024 16:59:31 -0400 Subject: [PATCH 04/11] rolling back changes for returning groups to an admin --- app/helpers/hyrax/hyrax_helper_behavior.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/helpers/hyrax/hyrax_helper_behavior.rb b/app/helpers/hyrax/hyrax_helper_behavior.rb index 60006245e4..1af81dae80 100644 --- a/app/helpers/hyrax/hyrax_helper_behavior.rb +++ b/app/helpers/hyrax/hyrax_helper_behavior.rb @@ -23,9 +23,9 @@ module HyraxHelperBehavior ## # @return [Array] the list of all user groups def available_user_groups(ability:) - user_groups = ability.user_groups.dup - return user_groups if ability.admin? + return ::User.group_service.role_names if ability.admin? # Excluding "public" and "registered" groups if non-admin user + user_groups = ability.user_groups.dup user_groups.delete("public") user_groups.delete("registered") user_groups From 5af76db7f0e0bf82b95979eed1b844f62dd12290 Mon Sep 17 00:00:00 2001 From: David Campbell Date: Wed, 5 Jun 2024 14:22:41 -0400 Subject: [PATCH 05/11] changing fileset initialization --- spec/controllers/hyrax/file_sets_controller_spec.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/spec/controllers/hyrax/file_sets_controller_spec.rb b/spec/controllers/hyrax/file_sets_controller_spec.rb index 8cbc2c4bef..bfcc0fa600 100644 --- a/spec/controllers/hyrax/file_sets_controller_spec.rb +++ b/spec/controllers/hyrax/file_sets_controller_spec.rb @@ -15,7 +15,13 @@ describe "#destroy" do context "file_set with a parent" do - let(:file_set) { FactoryBot.create(:file_set, user: user) } + let(:file_set) do + FactoryBot.create(:file_set, user: user).tap do |fs| + fs.edit_users = [user.user_key] + fs.visibility = Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_AUTHENTICATED + fs.save + end + end let(:work) { FactoryBot.create(:work, title: ['test title'], user: user) } before do From e9538cf96fdcdab652871d737b5f15c0a565645d Mon Sep 17 00:00:00 2001 From: David Campbell Date: Thu, 6 Jun 2024 10:47:45 -0400 Subject: [PATCH 06/11] rubocop --- spec/controllers/hyrax/file_sets_controller_spec.rb | 2 +- spec/features/edit_file_spec.rb | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/spec/controllers/hyrax/file_sets_controller_spec.rb b/spec/controllers/hyrax/file_sets_controller_spec.rb index bfcc0fa600..47b0f228d1 100644 --- a/spec/controllers/hyrax/file_sets_controller_spec.rb +++ b/spec/controllers/hyrax/file_sets_controller_spec.rb @@ -21,7 +21,7 @@ fs.visibility = Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_AUTHENTICATED fs.save end - end + end let(:work) { FactoryBot.create(:work, title: ['test title'], user: user) } before do diff --git a/spec/features/edit_file_spec.rb b/spec/features/edit_file_spec.rb index 870c2c836b..4f4c762c1c 100644 --- a/spec/features/edit_file_spec.rb +++ b/spec/features/edit_file_spec.rb @@ -3,6 +3,7 @@ let(:user) { create(:user) } 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(:options_presenter) { double(select_options: []) } let(:admin_set) do if Hyrax.config.disable_wings @@ -32,6 +33,8 @@ before do sign_in user + # mock the admin set options presenter to avoid hitting Solr + allow(Hyrax::AdminSetOptionsPresenter).to receive(:new).and_return(options_presenter) unless Hyrax.config.disable_wings Hydra::Works::AddFileToFileSet.call(file_set, file, :original_file) From 40aabd7b9acdfd4a7ad1ef8eb9d95760c23e9fae Mon Sep 17 00:00:00 2001 From: David Campbell Date: Thu, 6 Jun 2024 16:51:35 -0400 Subject: [PATCH 07/11] modified build steps to isolate error, creating admin set in tests --- .github/workflows/build-test-lint.yml | 2 +- spec/controllers/hyrax/file_sets_controller_spec.rb | 11 +++-------- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/.github/workflows/build-test-lint.yml b/.github/workflows/build-test-lint.yml index 2ae1551865..d743e2be9b 100644 --- a/.github/workflows/build-test-lint.yml +++ b/.github/workflows/build-test-lint.yml @@ -91,7 +91,7 @@ jobs: CI_NODE_INDEX: ${{ matrix.ci_node_index }} run: >- docker compose -f docker-compose-koppie.yml exec -T -w /app/samvera/hyrax-engine web sh -c - "bundle install && yarn install && rspec_booster --job ${{ matrix.ci_node_index }}/${{ matrix.ci_node_total }}" + "bundle install && yarn install && rspec spec/controllers/hyrax/file_sets_controller_spec" - name: Capture Container Logs if: always() uses: jwalton/gh-docker-logs@v2 diff --git a/spec/controllers/hyrax/file_sets_controller_spec.rb b/spec/controllers/hyrax/file_sets_controller_spec.rb index 47b0f228d1..05cb3eb0a6 100644 --- a/spec/controllers/hyrax/file_sets_controller_spec.rb +++ b/spec/controllers/hyrax/file_sets_controller_spec.rb @@ -3,6 +3,7 @@ RSpec.describe Hyrax::FileSetsController do routes { Rails.application.routes } let(:user) { FactoryBot.create(:user) } + let(:admin_set) { create(:admin_set, id: 'admin_set_1', with_permission_template: { with_active_workflow: true }) } let(:work_user) { user } before do allow(Hyrax.config.characterization_service).to receive(:run).and_return(true) @@ -15,13 +16,7 @@ describe "#destroy" do context "file_set with a parent" do - let(:file_set) do - FactoryBot.create(:file_set, user: user).tap do |fs| - fs.edit_users = [user.user_key] - fs.visibility = Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_AUTHENTICATED - fs.save - end - end + let(:file_set) { FactoryBot.create(:file_set, user: user) } let(:work) { FactoryBot.create(:work, title: ['test title'], user: user) } before do @@ -548,7 +543,7 @@ let(:user) { FactoryBot.create(:user) } before { sign_in user } let(:file) { fixture_file_upload('/world.png', 'image/png') } - let(:work) { FactoryBot.valkyrie_create(:hyrax_work, title: "test title", uploaded_files: [FactoryBot.create(:uploaded_file, user: work_user)], edit_users: [work_user]) } + let(:work) { FactoryBot.valkyrie_create(:hyrax_work, title: "test title", admin_set_id: admin_set.id, uploaded_files: [FactoryBot.create(:uploaded_file, user: work_user)], edit_users: [work_user]) } let(:file_set) { query_service.find_members(resource: work).first } let(:file_metadata) { query_service.custom_queries.find_files(file_set: file_set).first } let(:uploaded) { storage_adapter.find_by(id: file_metadata.file_identifier) } From da298b9cbcd8030d4c1651c907acdbe62ae4744e Mon Sep 17 00:00:00 2001 From: David Campbell Date: Thu, 6 Jun 2024 16:52:55 -0400 Subject: [PATCH 08/11] rubocop --- spec/controllers/hyrax/file_sets_controller_spec.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/controllers/hyrax/file_sets_controller_spec.rb b/spec/controllers/hyrax/file_sets_controller_spec.rb index 05cb3eb0a6..e5b425f703 100644 --- a/spec/controllers/hyrax/file_sets_controller_spec.rb +++ b/spec/controllers/hyrax/file_sets_controller_spec.rb @@ -543,7 +543,9 @@ let(:user) { FactoryBot.create(:user) } before { sign_in user } let(:file) { fixture_file_upload('/world.png', 'image/png') } - let(:work) { FactoryBot.valkyrie_create(:hyrax_work, title: "test title", admin_set_id: admin_set.id, uploaded_files: [FactoryBot.create(:uploaded_file, user: work_user)], edit_users: [work_user]) } + let(:work) do + FactoryBot.valkyrie_create(:hyrax_work, title: "test title", admin_set_id: admin_set.id, uploaded_files: [FactoryBot.create(:uploaded_file, user: work_user)], edit_users: [work_user]) + end let(:file_set) { query_service.find_members(resource: work).first } let(:file_metadata) { query_service.custom_queries.find_files(file_set: file_set).first } let(:uploaded) { storage_adapter.find_by(id: file_metadata.file_identifier) } From 290031d35ad16eb6233f2c1ef697ed2e25bf0d48 Mon Sep 17 00:00:00 2001 From: David Campbell Date: Thu, 6 Jun 2024 17:26:06 -0400 Subject: [PATCH 09/11] rolled back build change --- .github/workflows/build-test-lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build-test-lint.yml b/.github/workflows/build-test-lint.yml index d743e2be9b..2ae1551865 100644 --- a/.github/workflows/build-test-lint.yml +++ b/.github/workflows/build-test-lint.yml @@ -91,7 +91,7 @@ jobs: CI_NODE_INDEX: ${{ matrix.ci_node_index }} run: >- docker compose -f docker-compose-koppie.yml exec -T -w /app/samvera/hyrax-engine web sh -c - "bundle install && yarn install && rspec spec/controllers/hyrax/file_sets_controller_spec" + "bundle install && yarn install && rspec_booster --job ${{ matrix.ci_node_index }}/${{ matrix.ci_node_total }}" - name: Capture Container Logs if: always() uses: jwalton/gh-docker-logs@v2 From e60fd380e432699f87a74633e62ef577f3e69953 Mon Sep 17 00:00:00 2001 From: David Campbell Date: Wed, 14 Aug 2024 14:01:15 -0400 Subject: [PATCH 10/11] only apply admin set restrictions for non admins --- app/controllers/hyrax/file_sets_controller.rb | 1 + app/views/hyrax/file_sets/_permission.html.erb | 11 +++++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/app/controllers/hyrax/file_sets_controller.rb b/app/controllers/hyrax/file_sets_controller.rb index 6733351328..d0f05760d2 100644 --- a/app/controllers/hyrax/file_sets_controller.rb +++ b/app/controllers/hyrax/file_sets_controller.rb @@ -255,6 +255,7 @@ def initialize_edit_form end @version_list = Hyrax::VersionListPresenter.for(file_set: file_set) @groups = current_user.groups + @is_admin = current_ability.admin? end include WorkflowsHelper # Provides #workflow_restriction?, and yes I mean include not helper; helper exposes the module methods diff --git a/app/views/hyrax/file_sets/_permission.html.erb b/app/views/hyrax/file_sets/_permission.html.erb index 47f104598d..3675300be1 100644 --- a/app/views/hyrax/file_sets/_permission.html.erb +++ b/app/views/hyrax/file_sets/_permission.html.erb @@ -5,11 +5,14 @@ data: { param_key: file_set.model_name.param_key }, class: 'nav-safety' } do |f| %> -
-
- <%= render 'admin_set_options' %> + + <% if !@is_admin %> +
+
+ <%= render 'admin_set_options' %> +
-
+ <% end %> <%= hidden_field_tag('redirect_tab', 'permissions') %> <%= render "permission_form", f: f %>
From 3562ca79d9f73cc3a6512cf04538ea9bf4b5c0d6 Mon Sep 17 00:00:00 2001 From: David Campbell Date: Wed, 14 Aug 2024 14:57:59 -0400 Subject: [PATCH 11/11] change test admin set initialization --- spec/controllers/hyrax/file_sets_controller_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/controllers/hyrax/file_sets_controller_spec.rb b/spec/controllers/hyrax/file_sets_controller_spec.rb index 7238fcfa59..e1ef12396d 100644 --- a/spec/controllers/hyrax/file_sets_controller_spec.rb +++ b/spec/controllers/hyrax/file_sets_controller_spec.rb @@ -3,7 +3,7 @@ RSpec.describe Hyrax::FileSetsController do routes { Rails.application.routes } let(:user) { FactoryBot.create(:user) } - let(:admin_set) { create(:admin_set, id: 'admin_set_1', with_permission_template: { with_active_workflow: true }) } + let(:admin_set) { FactoryBot.valkyrie_create(:hyrax_admin_set, user: user, with_permission_template: true) } let(:work_user) { user } before do allow(Hyrax.config.characterization_service).to receive(:run).and_return(true)