diff --git a/app/actors/hyrax/actors/transfer_request_actor.rb b/app/actors/hyrax/actors/transfer_request_actor.rb
index 6ac589b869..e04c60f9a7 100644
--- a/app/actors/hyrax/actors/transfer_request_actor.rb
+++ b/app/actors/hyrax/actors/transfer_request_actor.rb
@@ -15,8 +15,9 @@ def create(env)
def create_proxy_deposit_request(env)
proxy = env.curation_concern.on_behalf_of
return true if proxy.blank?
- ContentDepositorChangeEventJob.perform_later(env.curation_concern,
- ::User.find_by_user_key(proxy))
+ work = env.curation_concern
+ user = ::User.find_by_user_key(proxy)
+ Hyrax::ChangeContentDepositorService.call(work, user, false)
true
end
end
diff --git a/app/controllers/concerns/hyrax/works_controller_behavior.rb b/app/controllers/concerns/hyrax/works_controller_behavior.rb
index a07f97d125..85a86a21c0 100644
--- a/app/controllers/concerns/hyrax/works_controller_behavior.rb
+++ b/app/controllers/concerns/hyrax/works_controller_behavior.rb
@@ -181,9 +181,12 @@ def create_valkyrie_work
result =
transactions['change_set.create_work']
- .with_step_args('work_resource.add_to_parent' => { parent_id: params[:parent_id], user: current_user },
- 'work_resource.add_file_sets' => { uploaded_files: uploaded_files, file_set_params: params[hash_key_for_curation_concern][:file_set] },
- 'change_set.set_user_as_depositor' => { user: current_user })
+ .with_step_args(
+ 'work_resource.add_to_parent' => { parent_id: params[:parent_id], user: current_user },
+ 'work_resource.add_file_sets' => { uploaded_files: uploaded_files, file_set_params: params[hash_key_for_curation_concern][:file_set] },
+ 'change_set.set_user_as_depositor' => { user: current_user },
+ 'work_resource.change_content_depositor' => { user: ::User.find_by_user_key(form.on_behalf_of) }
+ )
.call(form)
@curation_concern = result.value_or { return after_create_error(transaction_err_msg(result)) }
after_create_response
diff --git a/app/jobs/content_depositor_change_event_job.rb b/app/jobs/content_depositor_change_event_job.rb
index 1daf416685..e6d03867a3 100644
--- a/app/jobs/content_depositor_change_event_job.rb
+++ b/app/jobs/content_depositor_change_event_job.rb
@@ -1,27 +1,24 @@
# frozen_string_literal: true
# Log work depositor change to activity streams
#
-# @attr [Boolean] reset (false) should the access controls be reset. This means revoking edit access from the depositor
+# This class simply logs the transfer, pulling data from the object that was
+# just transferred. It does not perform the transfer.
class ContentDepositorChangeEventJob < ContentEventJob
include Rails.application.routes.url_helpers
include ActionDispatch::Routing::PolymorphicRoutes
- attr_accessor :reset
-
- # @param [ActiveFedora::Base] work the work to be transfered
- # @param [User] user the user the work is being transfered to.
- # @param [TrueClass,FalseClass] reset (false) if true, reset the access controls. This revokes edit access from the depositor
- def perform(work, user, reset = false)
- @reset = reset
- super(work, user)
+ # @param [ActiveFedora::Base] work the work that's been transfered
+ def perform(work)
+ # these get set to repo_object and depositor
+ super(work, new_owner(work))
end
def action
- "User #{link_to_profile work.proxy_depositor} has transferred #{link_to_work work.title.first} to user #{link_to_profile depositor}"
+ "User #{link_to_profile repo_object.proxy_depositor} has transferred #{link_to_work repo_object.title.first} to user #{link_to_profile depositor}"
end
def link_to_work(text)
- link_to text, polymorphic_path(work)
+ link_to text, polymorphic_path(repo_object)
end
# Log the event to the work's stream
@@ -30,18 +27,19 @@ def log_work_event(work)
end
alias log_file_set_event log_work_event
- def work
- @work ||= Hyrax::ChangeContentDepositorService.call(repo_object, depositor, reset)
- end
-
# overriding default to log the event to the depositor instead of their profile
+ # and to log the event for both users
def log_user_event(depositor)
- # log the event to the proxy depositor's profile
- proxy_depositor.log_profile_event(event)
+ previous_owner.log_profile_event(event)
depositor.log_event(event)
end
- def proxy_depositor
- @proxy_depositor ||= ::User.find_by_user_key(work.proxy_depositor)
+ private def previous_owner
+ ::User.find_by_user_key(repo_object.proxy_depositor)
+ end
+
+ # used for @depositor
+ private def new_owner(work)
+ ::User.find_by_user_key(work.depositor)
end
end
diff --git a/app/models/proxy_deposit_request.rb b/app/models/proxy_deposit_request.rb
index da538b20af..3cf0c94a82 100644
--- a/app/models/proxy_deposit_request.rb
+++ b/app/models/proxy_deposit_request.rb
@@ -129,7 +129,7 @@ def send_request_transfer_message_as_part_of_update
# @param [TrueClass,FalseClass] reset (false) if true, reset the access controls. This revokes edit access from the depositor
def transfer!(reset = false)
- ContentDepositorChangeEventJob.perform_later(work, receiving_user, reset)
+ Hyrax::ChangeContentDepositorService.call(work, receiving_user, reset)
fulfill!(status: ACCEPTED)
end
diff --git a/app/services/hyrax/change_content_depositor_service.rb b/app/services/hyrax/change_content_depositor_service.rb
index f06feb3075..261abf5915 100644
--- a/app/services/hyrax/change_content_depositor_service.rb
+++ b/app/services/hyrax/change_content_depositor_service.rb
@@ -4,6 +4,10 @@ class ChangeContentDepositorService
# Set the given `user` as the depositor of the given `work`; If
# `reset` is true, first remove all previous permissions.
#
+ # Used to transfer a an existing work, and to set
+ # depositor / proxy_depositor on a work newly deposited
+ # on_behalf_of another user
+ #
# @param work [ActiveFedora::Base, Valkyrie::Resource] the work
# that is receiving a change of depositor
# @param user [User] the user that will "become" the depositor of
@@ -12,13 +16,21 @@ class ChangeContentDepositorService
# permissions for the given work and contained file
# sets; regardless of true/false make the given user
# the depositor of the given work
+ # @return work, updated if necessary
def self.call(work, user, reset)
- case work
- when ActiveFedora::Base
- call_af(work, user, reset)
- when Valkyrie::Resource
- call_valkyrie(work, user, reset)
- end
+ # user_key is nil when there was no `on_behalf_of` in the form
+ return work unless user&.user_key
+ # Don't transfer to self
+ return work if user.user_key == work.depositor
+
+ work = case work
+ when ActiveFedora::Base
+ call_af(work, user, reset)
+ when Valkyrie::Resource
+ call_valkyrie(work, user, reset)
+ end
+ ContentDepositorChangeEventJob.perform_later(work)
+ work
end
def self.call_af(work, user, reset)
@@ -49,7 +61,6 @@ def self.call_valkyrie(work, user, reset)
apply_valkyrie_changes_to_file_sets(work: work, user: user, reset: reset)
Hyrax.persister.save(resource: work)
- work
end
private_class_method :call_valkyrie
diff --git a/lib/hyrax/transactions/container.rb b/lib/hyrax/transactions/container.rb
index 008acbc81a..9c2762ea7f 100644
--- a/lib/hyrax/transactions/container.rb
+++ b/lib/hyrax/transactions/container.rb
@@ -33,6 +33,7 @@ class Container # rubocop:disable Metrics/ClassLength
require 'hyrax/transactions/steps/add_to_collections'
require 'hyrax/transactions/steps/add_to_parent'
require 'hyrax/transactions/steps/apply_collection_type_permissions'
+ require 'hyrax/transactions/steps/change_content_depositor'
require 'hyrax/transactions/steps/check_for_empty_admin_set'
require 'hyrax/transactions/steps/delete_access_control'
require 'hyrax/transactions/steps/delete_resource'
@@ -204,6 +205,10 @@ class Container # rubocop:disable Metrics/ClassLength
Steps::AddToParent.new
end
+ ops.register 'change_content_depositor' do
+ Steps::ChangeContentDepositor.new
+ end
+
ops.register 'delete' do
Steps::DeleteResource.new
end
diff --git a/lib/hyrax/transactions/steps/change_content_depositor.rb b/lib/hyrax/transactions/steps/change_content_depositor.rb
new file mode 100644
index 0000000000..a620ea1615
--- /dev/null
+++ b/lib/hyrax/transactions/steps/change_content_depositor.rb
@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+require 'dry/monads'
+
+module Hyrax
+ module Transactions
+ module Steps
+ ##
+ # Add a given `::User` as the `#depositor`
+ # Move the previous value of that property to `#proxy_depositor`
+ #
+ #
+ # If no user is given, simply passes as a `Success`.
+ #
+ # @since 3.4.0
+ class ChangeContentDepositor
+ include Dry::Monads[:result]
+
+ ##
+ # @param [Hyrax::Work] obj
+ # @param user [User] the user that will "become" the depositor of
+ # the given work
+ #
+ # @return [Dry::Monads::Result]
+ def call(obj, user: NullUser.new, reset: false)
+ obj = Hyrax::ChangeContentDepositorService.call(obj, user, reset)
+
+ Success(obj)
+ rescue StandardError => err
+ Failure([err.message, obj])
+ end
+
+ ##
+ # @api private
+ class NullUser
+ ##
+ # @return [nil]
+ def user_key
+ nil
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/lib/hyrax/transactions/work_create.rb b/lib/hyrax/transactions/work_create.rb
index 719e06dd35..89479c00fe 100644
--- a/lib/hyrax/transactions/work_create.rb
+++ b/lib/hyrax/transactions/work_create.rb
@@ -13,6 +13,7 @@ class WorkCreate < Transaction
'change_set.set_user_as_depositor',
'change_set.apply',
'work_resource.save_acl',
+ 'work_resource.change_content_depositor',
'work_resource.add_file_sets',
'work_resource.add_to_parent'].freeze
diff --git a/spec/actors/hyrax/actors/transfer_request_actor_spec.rb b/spec/actors/hyrax/actors/transfer_request_actor_spec.rb
index fcc20f5b3b..fa4d3105c3 100644
--- a/spec/actors/hyrax/actors/transfer_request_actor_spec.rb
+++ b/spec/actors/hyrax/actors/transfer_request_actor_spec.rb
@@ -36,7 +36,7 @@
end
it "adds the template users to the work" do
- expect(ContentDepositorChangeEventJob).to receive(:perform_later).with(work, User)
+ expect(ContentDepositorChangeEventJob).to receive(:perform_later).with(work)
expect(middleware.create(env)).to be true
end
end
diff --git a/spec/controllers/concerns/hyrax/works_controller_behavior_spec.rb b/spec/controllers/concerns/hyrax/works_controller_behavior_spec.rb
index bdd2ec5e6e..c5e8d39772 100644
--- a/spec/controllers/concerns/hyrax/works_controller_behavior_spec.rb
+++ b/spec/controllers/concerns/hyrax/works_controller_behavior_spec.rb
@@ -95,6 +95,17 @@
expect(Sipity::Entity(assigns[:curation_concern]).workflow_state).to have_attributes(name: "deposited")
end
+ context 'when depositing as a proxy for (on_behalf_of) another user' do
+ let(:create_params) { { title: 'comet in moominland', on_behalf_of: target_user.user_key } }
+ let(:target_user) { FactoryBot.create(:user) }
+ it 'transfers depositor status to proxy target' do
+ expect { post :create, params: { test_simple_work: create_params } }
+ .to have_enqueued_job(ContentDepositorChangeEventJob)
+ expect(assigns[:curation_concern]).to have_attributes(depositor: target_user.user_key)
+ expect(assigns[:curation_concern]).to have_attributes(proxy_depositor: user.user_key)
+ end
+ end
+
context 'when setting visibility' do
let(:create_params) { { title: 'comet in moominland', visibility: 'open' } }
diff --git a/spec/hyrax/transactions/steps/change_content_depositor_spec.rb b/spec/hyrax/transactions/steps/change_content_depositor_spec.rb
new file mode 100644
index 0000000000..bd80e24b32
--- /dev/null
+++ b/spec/hyrax/transactions/steps/change_content_depositor_spec.rb
@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+require 'spec_helper'
+require 'hyrax/transactions'
+
+RSpec.describe Hyrax::Transactions::Steps::ChangeContentDepositor, valkyrie_adapter: :test_adapter do
+ subject(:step) { described_class.new }
+ let(:work) { FactoryBot.valkyrie_create(:hyrax_work) }
+
+ it 'gives Success(obj) in basic case' do
+ expect(step.call(work).value!).to eql(work)
+ end
+
+ context "when the depositor update is successful" do
+ it "calls the service" do
+ allow(Hyrax::ChangeContentDepositorService).to receive(:call)
+ step.call(work)
+
+ expect(Hyrax::ChangeContentDepositorService).to have_received(:call)
+ end
+ end
+
+ context "when there's an error" do
+ it 'returns a Failure' do
+ allow(Hyrax::ChangeContentDepositorService).to receive(:call).and_raise
+ result = step.call(work)
+
+ expect(result).to be_failure
+ end
+ end
+end
diff --git a/spec/jobs/content_depositor_change_event_job_spec.rb b/spec/jobs/content_depositor_change_event_job_spec.rb
index a7d8b5ed4a..8f4f5b6cb5 100644
--- a/spec/jobs/content_depositor_change_event_job_spec.rb
+++ b/spec/jobs/content_depositor_change_event_job_spec.rb
@@ -1,12 +1,13 @@
# frozen_string_literal: true
RSpec.describe ContentDepositorChangeEventJob do
- let(:user) { create(:user) }
- let(:another_user) { create(:user) }
+ let(:previous_user) { create(:user) }
+ let(:new_user) { create(:user) }
let(:mock_time) { Time.zone.at(1) }
let(:event) do
- { action: "User #{user.user_key} " \
- "has transferred BethsMac " \
- "to user #{another_user.user_key}",
+ { action:
+ "User #{previous_user.user_key} " \
+ "has transferred BethsMac " \
+ "to user #{new_user.user_key}",
timestamp: '1' }
end
@@ -15,42 +16,42 @@
end
context "when passing an ActiveFedora work" do
- let(:generic_work) { create(:generic_work, title: ['BethsMac'], user: user) }
+ let(:generic_work) { create(:generic_work, title: ['BethsMac'], user: new_user, proxy_depositor: previous_user.user_key) }
it "logs the event to the proxy depositor's profile, the depositor's dashboard, and the FileSet" do
- expect { described_class.perform_now(generic_work, another_user) }
- .to change { user.profile_events.length }
+ expect { described_class.perform_now(generic_work) }
+ .to change { previous_user.profile_events.length }
.by(1)
- .and change { another_user.events.length }
+ .and change { new_user.events.length }
.by(1)
.and change { generic_work.events.length }
.by(1)
- expect(user.profile_events.first).to eq(event)
- expect(another_user.events.first).to eq(event)
+ expect(previous_user.profile_events.first).to eq(event)
+ expect(new_user.events.first).to eq(event)
expect(generic_work.events.first).to eq(event)
end
end
context "when passing a valkyrie work" do
- let(:monograph) { valkyrie_create(:monograph, title: ['BethsMac'], depositor: user.user_key) }
+ let(:monograph) { valkyrie_create(:monograph, title: ['BethsMac'], depositor: new_user.user_key, proxy_depositor: previous_user.user_key) }
let(:event) do
- { action: "User #{user.user_key} " \
+ { action: "User #{previous_user.user_key} " \
"has transferred BethsMac " \
- "to user #{another_user.user_key}",
+ "to user #{new_user.user_key}",
timestamp: '1' }
end
it "logs the event to the proxy depositor's profile, the depositor's dashboard, and the FileSet" do
- expect { subject.perform(monograph, another_user) }
- .to change { user.profile_events.length }
+ expect { subject.perform(monograph) }
+ .to change { previous_user.profile_events.length }
.by(1)
- .and change { another_user.events.length }
+ .and change { new_user.events.length }
.by(1)
.and change { monograph.events.length }
.by(1)
- expect(user.profile_events.first).to eq(event)
- expect(another_user.events.first).to eq(event)
+ expect(previous_user.profile_events.first).to eq(event)
+ expect(new_user.events.first).to eq(event)
expect(monograph.events.first).to eq(event)
end
end
diff --git a/spec/models/proxy_deposit_request_spec.rb b/spec/models/proxy_deposit_request_spec.rb
index 92a5710ad4..1f929ca1c5 100644
--- a/spec/models/proxy_deposit_request_spec.rb
+++ b/spec/models/proxy_deposit_request_spec.rb
@@ -77,11 +77,12 @@
describe '#transfer!' do
it 'will change the status, fulfillment_date, and perform later the ContentDepositorChangeEventJob' do
- allow(ContentDepositorChangeEventJob).to receive(:perform_later)
+ allow(Hyrax::ChangeContentDepositorService).to receive(:call)
subject.transfer!
expect(subject.status).to eq(described_class::ACCEPTED)
expect(subject.fulfillment_date).to be_a(Time)
expect(subject).to be_accepted
+ expect(Hyrax::ChangeContentDepositorService).to have_received(:call)
end
end
diff --git a/spec/services/hyrax/change_content_depositor_service_spec.rb b/spec/services/hyrax/change_content_depositor_service_spec.rb
index 368f036e4a..1bc1355fdf 100644
--- a/spec/services/hyrax/change_content_depositor_service_spec.rb
+++ b/spec/services/hyrax/change_content_depositor_service_spec.rb
@@ -62,6 +62,7 @@
context "by default, when permissions are not reset" do
it "changes the depositor and records an original depositor" do
+ expect(ContentDepositorChangeEventJob).to receive(:perform_later)
described_class.call(base_work, receiver, false)
work = Hyrax.query_service.find_by_alternate_identifier(alternate_identifier: base_work.id, use_valkyrie: true)
expect(work.depositor).to eq receiver.user_key
@@ -83,6 +84,7 @@
context "when permissions are reset" do
it "changes the depositor and records an original depositor" do
+ expect(ContentDepositorChangeEventJob).to receive(:perform_later)
described_class.call(base_work, receiver, true)
work = Hyrax.query_service.find_by_alternate_identifier(alternate_identifier: base_work.id, use_valkyrie: true)
expect(work.depositor).to eq receiver.user_key
@@ -101,5 +103,31 @@
end
end
end
+
+ context "when no user is provided" do
+ it "does not update the work" do
+ expect(ContentDepositorChangeEventJob).not_to receive(:perform_later)
+ persister = double("Valkyrie Persister")
+ allow(Hyrax).to receive(:persister).and_return(persister)
+ allow(persister).to receive(:save)
+
+ described_class.call(base_work, nil, false)
+ expect(persister).not_to have_received(:save)
+ end
+ end
+
+ context "when transfer is requested to the existing owner" do
+ let!(:base_work) { valkyrie_create(:hyrax_work, :with_member_file_sets, title: ['AlreadyMine'], depositor: depositor.user_key, edit_users: [depositor]) }
+
+ it "does not update the work" do
+ expect(ContentDepositorChangeEventJob).not_to receive(:perform_later)
+ persister = double("Valkyrie Persister")
+ allow(Hyrax).to receive(:persister).and_return(persister)
+ allow(persister).to receive(:save)
+
+ described_class.call(base_work, depositor, false)
+ expect(persister).not_to have_received(:save)
+ end
+ end
end
end