From dbc9138afb641c74e5dde72d4af70279fdeabd95 Mon Sep 17 00:00:00 2001 From: Justin Littman Date: Thu, 8 Aug 2024 16:57:30 -0400 Subject: [PATCH] Support moving user versions. closes #4614 --- .../show/controls_component.html.erb | 1 + app/components/show/controls_component.rb | 10 +++++ app/controllers/user_versions_controller.rb | 23 ++++++++-- app/presenters/user_versions_presenter.rb | 12 +++++ app/views/user_versions/edit_move.html.erb | 18 ++++++++ config/routes.rb | 2 + .../show/controls_component_spec.rb | 17 ++++++- spec/features/user_version_view_spec.rb | 45 ++++++++++++++++++- .../user_versions_presenter_spec.rb | 43 ++++++++++++++++++ 9 files changed, 165 insertions(+), 6 deletions(-) create mode 100644 app/views/user_versions/edit_move.html.erb diff --git a/app/components/show/controls_component.html.erb b/app/components/show/controls_component.html.erb index c5f427eb6..6899da01d 100644 --- a/app/components/show/controls_component.html.erb +++ b/app/components/show/controls_component.html.erb @@ -44,5 +44,6 @@ <%= create_embargo %> <%= create_text_extraction %> <%= withdraw_or_restore %> + <%= move_user_version %> <% end %> diff --git a/app/components/show/controls_component.rb b/app/components/show/controls_component.rb index dcff87475..d3f57f96a 100644 --- a/app/components/show/controls_component.rb +++ b/app/components/show/controls_component.rb @@ -172,6 +172,16 @@ def withdraw_or_restore end end + def move_user_version + return unless item? && user_versions_presenter.move_version_targets.present? + + render ActionButton.new( + url: item_user_version_edit_move_path(doc, user_version_view), + label: 'Move', + open_modal: true + ) + end + def upload_mods link_to 'Upload MODS', apo_bulk_jobs_path(doc), class: 'btn btn-primary' end diff --git a/app/controllers/user_versions_controller.rb b/app/controllers/user_versions_controller.rb index 50d9c4742..5b75aee18 100644 --- a/app/controllers/user_versions_controller.rb +++ b/app/controllers/user_versions_controller.rb @@ -13,17 +13,29 @@ def show end def withdraw - update_user_version(withdrawn: true) + update_withdrawn(withdrawn: true) redirect_to item_user_version_path(druid_param, user_version_param), notice: 'Withdrawn. Purl will no longer display this version.' end def restore - update_user_version(withdrawn: false) + update_withdrawn(withdrawn: false) redirect_to item_user_version_path(druid_param, user_version_param), notice: 'Restored. Purl will display this version.' end + def edit_move + @druid = druid_param + @user_versions_presenter = UserVersionsPresenter.new(user_version_view: user_version_param, user_version_inventory: client.inventory) + end + + def move + client.update(user_version: Dor::Services::Client::UserVersion::Version.new(userVersion: user_version_param, version: params[:version])) + + redirect_to item_user_version_path(druid_param, user_version_param), + notice: 'Moved user version.' + end + private def druid_param @@ -38,8 +50,11 @@ def find_user_version_cocina @cocina = Repository.find_user_version(druid_param, user_version_param) end - def update_user_version(withdrawn:) - client = Dor::Services::Client.object(druid_param).user_version + def update_withdrawn(withdrawn:) client.update(user_version: Dor::Services::Client::UserVersion::Version.new(withdrawn:, userVersion: user_version_param)) end + + def client + Dor::Services::Client.object(druid_param).user_version + end end diff --git a/app/presenters/user_versions_presenter.rb b/app/presenters/user_versions_presenter.rb index fff697aec..ede842e70 100644 --- a/app/presenters/user_versions_presenter.rb +++ b/app/presenters/user_versions_presenter.rb @@ -43,6 +43,18 @@ def version_view user_version_data_for(user_version_view)&.version&.to_i end + # @return [Array] the versions that this user version can be moved to + def move_version_targets + return [] unless user_version_data + return [] if user_version_data.restorable? # Can't move it if withdrawn. + + next_user_version = user_version_view + 1 + version_for_next_user_version = user_version_data_for(next_user_version)&.version&.to_i + return [] unless version_for_next_user_version + + (version_view + 1..version_for_next_user_version - 1).to_a + end + attr_reader :user_version_view, :user_version_inventory private diff --git a/app/views/user_versions/edit_move.html.erb b/app/views/user_versions/edit_move.html.erb new file mode 100644 index 000000000..008598433 --- /dev/null +++ b/app/views/user_versions/edit_move.html.erb @@ -0,0 +1,18 @@ +<%= render EditModalComponent.new do |component| %> + <% component.with_header { 'Move user version' } %> + <% component.with_body do %> + <%= form_with(url: item_user_version_move_path(@druid, @user_versions_presenter.user_version_view)) do |f| %> + <% move_version_targets = @user_versions_presenter.move_version_targets %> + <% if move_version_targets.one? %> +
Move user version <%= @user_versions_presenter.user_version_view %> to version <%= move_version_targets.first %>.
+ <%= f.hidden_field('version', value: move_version_targets.first) %> + <% else %> + <%= f.label :version, "Move user version #{@user_versions_presenter.user_version_view} to version:", class: 'form-label' %> + <%= f.select :version, move_version_targets, {}, { class: 'form-select', style: 'width: 100px;' } %> + <% end %> +
+ +
+ <% end %> + <% end %> +<% end %> diff --git a/config/routes.rb b/config/routes.rb index c0a1afd93..bf8e3d26a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -236,6 +236,8 @@ resources :user_versions, only: %i[] do post 'withdraw' post 'restore' + get 'move', to: 'user_versions#edit_move', as: 'edit_move' + post 'move' get 'descriptive', to: 'descriptives#show' get 'structure', to: 'structures#show' resources 'files', only: %i[index], constraints: { item_id: /.*/ } do diff --git a/spec/components/show/controls_component_spec.rb b/spec/components/show/controls_component_spec.rb index 3dfa03200..a2db23284 100644 --- a/spec/components/show/controls_component_spec.rb +++ b/spec/components/show/controls_component_spec.rb @@ -24,7 +24,12 @@ version_or_user_version_view?: version.present? || user_version.present?, user_versions_presenter:) end - let(:user_versions_presenter) { instance_double(UserVersionsPresenter, user_version_withdrawable?: user_version_withdrawable, user_version_restorable?: user_version_restorable) } + let(:user_versions_presenter) do + instance_double(UserVersionsPresenter, + user_version_withdrawable?: user_version_withdrawable, + user_version_restorable?: user_version_restorable, + move_version_targets:) + end let(:cocina) { instance_double(Cocina::Models::DRO, dro?: dro, type: 'https://cocina.sul.stanford.edu/models/book') } let(:open) { false } let(:open_and_not_assembling) { false } @@ -32,6 +37,7 @@ let(:version) { nil } let(:user_version_withdrawable) { false } let(:user_version_restorable) { false } + let(:move_version_targets) { [] } before do rendered @@ -164,6 +170,15 @@ expect(page).to have_link 'Restore', href: '/items/druid:kv840xx0000/user_versions/2/restore' end end + + context 'when the user version can be moved' do + let(:user_version) { 2 } + let(:move_version_targets) { [3, 4] } + + it 'the move button is displayed' do + expect(page).to have_link 'Move', href: '/items/druid:kv840xx0000/user_versions/2/move' + end + end end context 'when the object is an AdminPolicy the user can manage' do diff --git a/spec/features/user_version_view_spec.rb b/spec/features/user_version_view_spec.rb index 358fd493b..c5f837d2f 100644 --- a/spec/features/user_version_view_spec.rb +++ b/spec/features/user_version_view_spec.rb @@ -7,7 +7,8 @@ let(:solr_conn) { blacklight_config.repository_class.new(blacklight_config).connection } let(:druid) { 'druid:hj185xx2222' } let(:version_client) { instance_double(Dor::Services::Client::ObjectVersion, current: 1, inventory: [version1]) } - let(:user_version_client) { instance_double(Dor::Services::Client::UserVersion, inventory: [user_version1], find: cocina_object, solr: solr_doc, update: true) } + let(:user_version_client) { instance_double(Dor::Services::Client::UserVersion, inventory:, find: cocina_object, solr: solr_doc, update: true) } + let(:inventory) { [user_version1] } let(:release_tags_client) { instance_double(Dor::Services::Client::ReleaseTags, list: release_tags_list) } let(:version1) { Dor::Services::Client::ObjectVersion::Version.new } let(:user_version1) { Dor::Services::Client::UserVersion::Version.new(version: 4, userVersion: 2, withdrawable:, restorable:) } @@ -237,6 +238,48 @@ end end + context 'when moving a version and only one target' do + let(:inventory) do + [ + Dor::Services::Client::UserVersion::Version.new(version: 1, userVersion: 1, withdrawable: true, restorable: false), + Dor::Services::Client::UserVersion::Version.new(version: 3, userVersion: 2, withdrawable: true, restorable: false) + ] + end + + it 'moves' do + visit item_user_version_path(item_id: druid, user_version_id: 1) + click_link('Move') + expect(page).to have_content('Move user version') + expect(page).to have_content('Move user version 1 to version 2.') + click_button('Submit') + expect(page).to have_content('Moved user version.') + + expect(user_version_client).to have_received(:update) + .with(user_version: Dor::Services::Client::UserVersion::Version.new(userVersion: '1', version: '2')) + end + end + + context 'when moving a version and multiple targets' do + let(:inventory) do + [ + Dor::Services::Client::UserVersion::Version.new(version: 1, userVersion: 1, withdrawable: true, restorable: false), + Dor::Services::Client::UserVersion::Version.new(version: 4, userVersion: 2, withdrawable: true, restorable: false) + ] + end + + it 'moves' do + visit item_user_version_path(item_id: druid, user_version_id: 1) + click_link('Move') + expect(page).to have_content('Move user version') + select('3', from: 'Move user version 1 to version:') + click_button('Submit') + expect(page).to have_content('Moved user version.') + + expect(user_version_client).to have_received(:update) + .with(user_version: Dor::Services::Client::UserVersion::Version.new(userVersion: '1', version: '3')) + end + end + context 'when viewing an unknown version' do before do allow(user_version_client).to receive(:find).and_raise(Dor::Services::Client::NotFoundResponse) diff --git a/spec/presenters/user_versions_presenter_spec.rb b/spec/presenters/user_versions_presenter_spec.rb index 3b376d549..e4e24ba73 100644 --- a/spec/presenters/user_versions_presenter_spec.rb +++ b/spec/presenters/user_versions_presenter_spec.rb @@ -90,4 +90,47 @@ it { is_expected.to eq 2 } end + + describe '#move_version_targets' do + subject { presenter.move_version_targets } + + context 'when no versions are movable' do + it { is_expected.to eq [] } + end + + context 'when versions are movable' do + let(:user_version_inventory) do + [ + Dor::Services::Client::UserVersion::Version.new(userVersion: '1', version: '3', withdrawable: true, restorable: false), + Dor::Services::Client::UserVersion::Version.new(userVersion: '2', version: '6', withdrawable: true, restorable: false) + ] + end + + it { is_expected.to eq [4, 5] } + end + + context 'when the user version is withdrawn' do + let(:user_version_inventory) do + [ + Dor::Services::Client::UserVersion::Version.new(userVersion: '1', version: '3', withdrawable: false, restorable: true), + Dor::Services::Client::UserVersion::Version.new(userVersion: '2', version: '6', withdrawable: true, restorable: false) + ] + end + + it { is_expected.to eq [] } + end + + context 'when the user version is the last user version' do + let(:user_version_inventory) do + [ + Dor::Services::Client::UserVersion::Version.new(userVersion: '1', version: '3', withdrawable: false, restorable: true), + Dor::Services::Client::UserVersion::Version.new(userVersion: '2', version: '6', withdrawable: true, restorable: false) + ] + end + + let(:user_version) { '2' } + + it { is_expected.to eq [] } + end + end end