Skip to content

Commit

Permalink
Proxy deposit requests use valkyrie-specific query service when use_v…
Browse files Browse the repository at this point in the history
…alkyrie? is true
  • Loading branch information
cjcolvar authored and tamsin johnson committed Sep 14, 2022
1 parent df1aa90 commit b1af153
Show file tree
Hide file tree
Showing 8 changed files with 241 additions and 7 deletions.
2 changes: 0 additions & 2 deletions app/controllers/hyrax/transfers_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,13 @@ def new
add_breadcrumb t(:'hyrax.controls.home'), root_path
add_breadcrumb t(:'hyrax.dashboard.breadcrumbs.admin'), hyrax.dashboard_path
add_breadcrumb t(:'hyrax.transfers.new.header'), hyrax.new_work_transfer_path
@work = Hyrax::WorkRelation.new.find(params[:id])
end

def create
@proxy_deposit_request.sending_user = current_user
if @proxy_deposit_request.save
redirect_to hyrax.transfers_path, notice: "Transfer request created"
else
@work = Hyrax::WorkRelation.new.find(params[:id])
render :new
end
end
Expand Down
2 changes: 1 addition & 1 deletion app/models/proxy_deposit_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class ProxyDepositRequest < ActiveRecord::Base
include ActionView::Helpers::UrlHelper

class_attribute :work_query_service_class
self.work_query_service_class = Hyrax::WorkQueryService
self.work_query_service_class = Hyrax.config.use_valkyrie? ? Hyrax::WorkResourceQueryService : Hyrax::WorkQueryService

delegate :deleted_work?, :work, :to_s, to: :work_query_service

Expand Down
49 changes: 49 additions & 0 deletions app/services/hyrax/work_resource_query_service.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# frozen_string_literal: true
module Hyrax
# Responsible for retrieving information based on the given work.
#
# @see ProxyDepositRequest
# @see SolrDocument
# @see Hyrax::SolrService
# @note This was extracted from the ProxyDepositRequest, which was coordinating lots of effort. It was also an ActiveRecord object that required lots of Fedora/Solr interactions.
class WorkResourceQueryService
# @param [String] id - The id of the work
def initialize(id:)
@id = id
end
attr_reader :id

# @return [Boolean] if the work has been deleted
def deleted_work?
Hyrax.query_service.find_by(id: id)
false
rescue Valkyrie::Persistence::ObjectNotFoundError
true
end

def work
# Need to ensure it is a work?
resource = Hyrax.query_service.find_by(id: id)
unless Hyrax.config.curation_concerns.include?(resource.class) ||
Hyrax.config.curation_concerns.map(&:to_s).include?(resource.class.to_s) || # Wings-wrapped models
Hyrax.config.curation_concerns.map(&:to_s).include?(resource.class.name) # Wings-wrapped models
raise ModelMismatchError, "Expected allowed work type but got #{resource.class}"
end
resource
end

def to_s
if deleted_work?
'work not found'
else
solr_doc.to_s
end
end

private

def solr_doc
@solr_doc ||= ::SolrDocument.new(Hyrax::SolrService.search_by_id(id))
end
end
end
2 changes: 1 addition & 1 deletion app/views/hyrax/transfers/new.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<span class="sr-only"><%= t(:'.sr_only_description', work_title: @proxy_deposit_request.to_s) %></span>

<%= simple_form_for @proxy_deposit_request,
url: hyrax.work_transfers_path(@work) do |f| %>
url: hyrax.work_transfers_path(params[:id]) do |f| %>
<%= f.input :transfer_to,
placeholder: t(:'.placeholder') %>
<%= f.input :sender_comment %>
Expand Down
2 changes: 2 additions & 0 deletions lib/hyrax/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,6 @@ class SingleUseError < HyraxError; end
class SingleMembershipError < HyraxError; end

class ObjectNotFoundError < ActiveFedora::ObjectNotFoundError; end

class ModelMismatchError < HyraxError; end
end
146 changes: 144 additions & 2 deletions spec/controllers/hyrax/transfers_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,19 @@
sign_in user
get :new, params: { id: work.id }
expect(response).to be_successful
expect(assigns[:work]).to eq(work)
expect(assigns[:proxy_deposit_request]).to be_kind_of ProxyDepositRequest
expect(assigns[:proxy_deposit_request].work_id).to eq(work.id)
end
end

context 'with work resource' do
let(:work) { FactoryBot.valkyrie_create(:hyrax_work, depositor: user.user_key, edit_users: [user]) }

it "is successful" do
expect(controller).to receive(:add_breadcrumb).exactly(3).times
sign_in user
get :new, params: { id: work.id }
expect(response).to be_successful
expect(assigns[:proxy_deposit_request]).to be_kind_of ProxyDepositRequest
expect(assigns[:proxy_deposit_request].work_id).to eq(work.id)
end
Expand Down Expand Up @@ -74,9 +86,43 @@
post :create, params: { id: work.id, proxy_deposit_request: { transfer_to: 'foo' } }
end.not_to change(ProxyDepositRequest, :count)
expect(assigns[:proxy_deposit_request].errors[:transfer_to]).to eq(['Must be an existing user'])
expect(assigns[:work]).to be_instance_of GenericWork
expect(response).to be_successful
end

context 'with work resource' do
let(:work) { FactoryBot.valkyrie_create(:hyrax_work, depositor: user.user_key, edit_users: [user]) }

it "is successful" do
expect do
post :create, params: {
id: work.id,
proxy_deposit_request: {
transfer_to: another_user.user_key,
sender_comment: 'Hi mom!'
}
}
end.to change(ProxyDepositRequest, :count).by(1)
expect(response).to redirect_to routes.url_helpers.transfers_path(locale: 'en')
expect(flash[:notice]).to eq('Transfer request created')
proxy_request = another_user.proxy_deposit_requests.first
expect(proxy_request.work_id).to eq(work.id)
expect(proxy_request.sending_user).to eq(user)
expect(proxy_request.sender_comment).to eq 'Hi mom!'
# AND A NOTIFICATION SHOULD HAVE BEEN CREATED
notification = another_user.reload.mailbox.inbox[0].messages[0]
expect(notification.subject).to eq("Ownership Change Request")
expect(notification.body).to eq("<a href=\"#{routes.url_helpers.user_path(user)}\">#{user.name}</a> " \
"wants to transfer a work to you. Review all " \
"<a href=\"#{routes.url_helpers.transfers_path}\">transfer requests</a>")
end
it "gives an error if the user is not found" do
expect do
post :create, params: { id: work.id, proxy_deposit_request: { transfer_to: 'foo' } }
end.not_to change(ProxyDepositRequest, :count)
expect(assigns[:proxy_deposit_request].errors[:transfer_to]).to eq(['Must be an existing user'])
expect(response).to be_successful
end
end
end

describe "#accept" do
Expand Down Expand Up @@ -127,6 +173,53 @@
expect(flash[:alert]).to eq("You are not authorized to access this page.")
end
end

context 'with work resource' do
around do |example|
@query_class = ProxyDepositRequest.work_query_service_class
ProxyDepositRequest.work_query_service_class = Hyrax::WorkResourceQueryService
example.run
ProxyDepositRequest.work_query_service_class = @query_class
end

context "when I am the receiver" do
let(:work) { FactoryBot.valkyrie_create(:monograph, title: ['incoming'], depositor: another_user.user_key, edit_users: [another_user]) }
let!(:proxy_request) { ProxyDepositRequest.create!(work_id: work.id, receiving_user: user, sending_user: another_user) }

it "is successful when retaining access rights" do
put :accept, params: { id: user.proxy_deposit_requests.first }
expect(response).to redirect_to routes.url_helpers.transfers_path(locale: 'en')
expect(flash[:notice]).to eq("Transfer complete")
expect(assigns[:proxy_deposit_request].status).to eq('accepted')
expect(Hyrax.query_service.find_by(id: work.id).edit_users).to match_array [another_user.user_key, user.user_key]
end
it "is successful when resetting access rights" do
put :accept, params: { id: user.proxy_deposit_requests.first, reset: true }
expect(response).to redirect_to routes.url_helpers.transfers_path(locale: 'en')
expect(flash[:notice]).to eq("Transfer complete")
expect(assigns[:proxy_deposit_request].status).to eq('accepted')
expect(Hyrax.query_service.find_by(id: work.id).edit_users).to match_array [user.user_key]
end
it "handles sticky requests" do
put :accept, params: { id: user.proxy_deposit_requests.first, sticky: true }
expect(response).to redirect_to routes.url_helpers.transfers_path(locale: 'en')
expect(flash[:notice]).to eq("Transfer complete")
expect(assigns[:proxy_deposit_request].status).to eq('accepted')
expect(user.can_receive_deposits_from).to include(another_user)
end
end

context "accepting one that isn't mine" do
let(:work) { FactoryBot.valkyrie_create(:monograph, title: ['incoming'], depositor: user.user_key, edit_users: [user]) }
let!(:proxy_request) { ProxyDepositRequest.create!(work_id: work.id, receiving_user: another_user, sending_user: user) }

it "does not allow me" do
put :accept, params: { id: another_user.proxy_deposit_requests.first }
expect(response).to redirect_to root_path
expect(flash[:alert]).to eq("You are not authorized to access this page.")
end
end
end
end

describe "#reject" do
Expand Down Expand Up @@ -162,6 +255,31 @@
expect(flash[:alert]).to eq("You are not authorized to access this page.")
end
end

context 'with work resource' do
context "when I am the receiver" do
let(:work) { FactoryBot.valkyrie_create(:monograph, title: ['incoming'], depositor: another_user.user_key, edit_users: [another_user]) }
let!(:proxy_request) { ProxyDepositRequest.create!(work_id: work.id, receiving_user: user, sending_user: another_user) }

it "is successful" do
put :reject, params: { id: user.proxy_deposit_requests.first }
expect(response).to redirect_to routes.url_helpers.transfers_path(locale: 'en')
expect(flash[:notice]).to eq("Transfer rejected")
expect(assigns[:proxy_deposit_request].status).to eq('rejected')
end
end

context "accepting one that isn't mine" do
let(:work) { FactoryBot.valkyrie_create(:monograph, title: ['incoming'], depositor: user.user_key, edit_users: [user]) }
let!(:proxy_request) { ProxyDepositRequest.create!(work_id: work.id, receiving_user: another_user, sending_user: user) }

it "does not allow me" do
put :reject, params: { id: another_user.proxy_deposit_requests.first }
expect(response).to redirect_to root_path
expect(flash[:alert]).to eq("You are not authorized to access this page.")
end
end
end
end

describe "#destroy" do
Expand Down Expand Up @@ -196,6 +314,30 @@
expect(flash[:alert]).to eq("You are not authorized to access this page.")
end
end

context 'with work resource' do
context "when I am the sender" do
let(:work) { FactoryBot.valkyrie_create(:monograph, title: ['incoming'], depositor: user.user_key, edit_users: [user]) }
let!(:proxy_request) { ProxyDepositRequest.create!(work_id: work.id, receiving_user: another_user, sending_user: user) }

it "is successful" do
delete :destroy, params: { id: another_user.proxy_deposit_requests.first }
expect(response).to redirect_to routes.url_helpers.transfers_path(locale: 'en')
expect(flash[:notice]).to eq("Transfer canceled")
end
end

context "accepting one that isn't mine" do
let(:work) { FactoryBot.valkyrie_create(:monograph, title: ['incoming'], depositor: another_user.user_key, edit_users: [another_user]) }
let!(:proxy_request) { ProxyDepositRequest.create!(work_id: work.id, receiving_user: user, sending_user: another_user) }

it "does not allow me" do
delete :destroy, params: { id: user.proxy_deposit_requests.first }
expect(response).to redirect_to root_path
expect(flash[:alert]).to eq("You are not authorized to access this page.")
end
end
end
end
end
end
2 changes: 1 addition & 1 deletion spec/services/hyrax/work_query_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ module Hyrax

subject { work_query_service.work }

context 'when not in SOLR' do
context 'when in SOLR' do
before { allow(work_relation).to receive(:find).with(work_id).and_return(expected_work) }
it { is_expected.to eq(expected_work) }
end
Expand Down
43 changes: 43 additions & 0 deletions spec/services/hyrax/work_resource_query_service_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# frozen_string_literal: true
module Hyrax
RSpec.describe WorkResourceQueryService do
let(:user) { create(:user) }
let(:work) { FactoryBot.valkyrie_create(:monograph, title: ['Test work']) }
let(:work_query_service) { described_class.new(id: work.id) }

describe '#deleted_work?' do
subject { work_query_service.deleted_work? }

context 'when not in SOLR' do
let(:work_query_service) { described_class.new(id: 'deleted-id') }

it { is_expected.to be_truthy }
end
context 'when in SOLR' do
it { is_expected.to be_falsey }
end
end
describe '#work' do
subject { work_query_service.work }

context 'when in SOLR' do
it { expect(subject.id).to eq(work.id) }
end
end
describe '#to_s' do
subject { work_query_service.to_s }

context 'when the work is deleted' do
let(:work_query_service) { described_class.new(id: 'deleted-id') }

it { is_expected.to eq('work not found') }
end

context 'when the work is not deleted' do
it 'will retrieve the SOLR document and use the #to_s method of that' do
expect(subject).to eq(work.title.first)
end
end
end
end
end

0 comments on commit b1af153

Please sign in to comment.