diff --git a/.circleci/config.yml b/.circleci/config.yml index 9d626597f..15c5b3098 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -95,6 +95,7 @@ jobs: POSTGRES_PASSWORD: scholarsphere POSTGRES_DB: scholarsphere POSTGRES_HOST: 127.0.0.1 + RMD_HOST: https://metadata.libraries.psu.edu steps: - browser-tools/install-browser-tools - checkout diff --git a/.envrc.sample b/.envrc.sample index 3fcba7fce..8a0b16c59 100644 --- a/.envrc.sample +++ b/.envrc.sample @@ -22,4 +22,12 @@ export POSTGRES_PORT= export DIRECTORY_SERVICE_ENDPOINT= export PSU_ID_OAUTH_CLIENT_ID= export PSU_ID_OAUTH_CLIENT_SECRET= -export PSU_ID_OAUTH_ENDPOINT= \ No newline at end of file +export PSU_ID_OAUTH_ENDPOINT= +export DATACITE_ENDPOINT= +export DATACITE_PASSWORD= +export DATACITE_PREFIX= +export DATACITE_PUBLISHER= +export DATACITE_USERNAME= +export RMD_API_KEY= +export RMD_WEBHOOK_SECRET= +export RMD_HOST= diff --git a/app/jobs/work_removed_webhook_job.rb b/app/jobs/work_removed_webhook_job.rb new file mode 100644 index 000000000..0b13afc41 --- /dev/null +++ b/app/jobs/work_removed_webhook_job.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class WorkRemovedWebhookJob < ApplicationJob + queue_as :webhooks + + def perform(work_uuid) + WorkRemovedWebhook.new(work_uuid).notify + end +end diff --git a/app/models/work.rb b/app/models/work.rb index c7a65ed40..bd7b2b60f 100644 --- a/app/models/work.rb +++ b/app/models/work.rb @@ -7,6 +7,8 @@ class Work < ApplicationRecord include GeneratedUuids include ThumbnailSelections + after_destroy :notify_work_deleted + fields_with_dois :doi, :latest_published_version_dois delegate :email, :display_name, to: :depositor @@ -293,4 +295,8 @@ def embargoed_until_is_valid_date nil end end + + def notify_work_deleted + WorkRemovedWebhookJob.perform_later(uuid) + end end diff --git a/app/models/work_version.rb b/app/models/work_version.rb index 86bfd3c72..3b4a74531 100644 --- a/app/models/work_version.rb +++ b/app/models/work_version.rb @@ -211,6 +211,11 @@ def label(id) end event :withdraw do + after_commit do + if work.withdrawn? + WorkRemovedWebhookJob.perform_later(work.uuid) + end + end transitions from: :published, to: :withdrawn end diff --git a/app/services/rmd_client.rb b/app/services/rmd_client.rb index 2da65c57f..9323fce9d 100644 --- a/app/services/rmd_client.rb +++ b/app/services/rmd_client.rb @@ -26,7 +26,7 @@ def endpoint end def rmd_host - 'https://metadata.libraries.psu.edu' + ENV['RMD_HOST'] end def api_key diff --git a/app/services/work_removed_webhook.rb b/app/services/work_removed_webhook.rb new file mode 100644 index 000000000..85173a5f1 --- /dev/null +++ b/app/services/work_removed_webhook.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class WorkRemovedWebhook + def initialize(work_uuid) + @work_uuid = work_uuid + end + + def notify + conn = Faraday.new( + url: ENV['RMD_HOST'], + headers: { 'X-API-KEY' => ENV['RMD_WEBHOOK_SECRET'] } + ) + + conn.post('/webhooks/scholarsphere_events', publication_url: "https://scholarsphere.psu.edu/resources/#{work_uuid}") + end + + private + + attr_reader :work_uuid +end diff --git a/spec/jobs/work_removed_webhook_job_spec.rb b/spec/jobs/work_removed_webhook_job_spec.rb new file mode 100644 index 000000000..70eb5e69d --- /dev/null +++ b/spec/jobs/work_removed_webhook_job_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe WorkRemovedWebhookJob do + let(:webhook) { instance_double WorkRemovedWebhook, notify: nil } + + before do + allow(WorkRemovedWebhook).to receive(:new).with('abc123').and_return(webhook) + end + + describe '#perform' do + it 'triggers a work removed webhook' do + described_class.perform_now('abc123') + + expect(webhook).to have_received(:notify) + end + end +end diff --git a/spec/models/work_spec.rb b/spec/models/work_spec.rb index 26f1ae68c..47a46ba79 100644 --- a/spec/models/work_spec.rb +++ b/spec/models/work_spec.rb @@ -2,7 +2,7 @@ require 'rails_helper' -RSpec.describe Work, type: :model do +RSpec.describe Work do it { is_expected.to delegate_method(:email).to(:depositor) } it { is_expected.to delegate_method(:display_name).to(:depositor) } it { is_expected.to delegate_method(:has_publisher_doi?).to(:latest_version) } @@ -20,12 +20,12 @@ it_behaves_like 'a resource that can provide all DOIs in', [:doi, :latest_published_version_dois] it_behaves_like 'a resource with a thumbnail url' do - let!(:work) { create :work, versions_count: 2 } + let!(:work) { create(:work, versions_count: 2) } let(:resource) { work } end it_behaves_like 'a resource with a thumbnail selection' do - let!(:work) { create :work, versions_count: 2 } + let!(:work) { create(:work, versions_count: 2) } let(:resource) { work } end @@ -90,17 +90,31 @@ end end + describe 'lifecycle callbacks' do + describe 'destroying a work' do + let!(:work) { create(:work) } + + before { allow(WorkRemovedWebhookJob).to receive(:perform_later) } + + it 'enqueues a notification job' do + work.destroy! + + expect(WorkRemovedWebhookJob).to have_received(:perform_later).with(work.uuid) + end + end + end + describe '.recently_published' do - let(:wv1) { build :work_version, :published, work: nil, sent_for_curation_at: nil } - let(:work1) { create :work, versions: [wv1] } + let(:wv1) { build(:work_version, :published, work: nil, sent_for_curation_at: nil) } + let(:work1) { create(:work, versions: [wv1]) } - let(:wv2) { build :work_version, :published, work: nil, sent_for_curation_at: Time.now } - let(:work2) { create :work, versions: [wv2] } + let(:wv2) { build(:work_version, :published, work: nil, sent_for_curation_at: Time.now) } + let(:work2) { create(:work, versions: [wv2]) } - let(:wv4) { build :work_version, work: nil, aasm_state: 'draft' } - let(:work3) { create :work, versions: [wv4] } + let(:wv4) { build(:work_version, work: nil, aasm_state: 'draft') } + let(:work3) { create(:work, versions: [wv4]) } - let(:work4) { create :work } + let(:work4) { create(:work) } it 'returns works with a published WorkVersion that has not been sent for curation' do expect(described_class.recently_published).to match_array([work1]) @@ -357,15 +371,15 @@ end describe 'version accessors' do - let(:draft) { build :work_version, :draft, title: 'Draft', work: nil, created_at: 1.day.ago, version_number: 3 } - let(:v2) { build :work_version, :published, title: 'Published v2', work: nil, created_at: 2.days.ago, version_number: 2 } - let(:v1) { build :work_version, :published, title: 'Published v1', work: nil, created_at: 3.days.ago, version_number: 1 } - let(:withdrawn) { build :work_version, :withdrawn, work: nil, created_at: 3.days.ago, version_number: 1 } + let(:draft) { build(:work_version, :draft, title: 'Draft', work: nil, created_at: 1.day.ago, version_number: 3) } + let(:v2) { build(:work_version, :published, title: 'Published v2', work: nil, created_at: 2.days.ago, version_number: 2) } + let(:v1) { build(:work_version, :published, title: 'Published v1', work: nil, created_at: 3.days.ago, version_number: 1) } + let(:withdrawn) { build(:work_version, :withdrawn, work: nil, created_at: 3.days.ago, version_number: 1) } before { work.reload } context 'with draft, published, and withdrawn versions' do - subject(:work) { create :work, versions: [draft, v2, withdrawn] } + subject(:work) { create(:work, versions: [draft, v2, withdrawn]) } it { is_expected.not_to be_withdrawn } @@ -377,7 +391,7 @@ end context 'with draft and published versions' do - subject(:work) { create :work, versions: [draft, v2, v1] } + subject(:work) { create(:work, versions: [draft, v2, v1]) } it { is_expected.not_to be_withdrawn } @@ -389,7 +403,7 @@ end context 'with draft and withdrawn versions' do - subject(:work) { create :work, versions: [draft, withdrawn] } + subject(:work) { create(:work, versions: [draft, withdrawn]) } it { is_expected.to be_withdrawn } @@ -401,7 +415,7 @@ end context 'with published and withdrawn versions' do - subject(:work) { create :work, versions: [v2, withdrawn] } + subject(:work) { create(:work, versions: [v2, withdrawn]) } it { is_expected.not_to be_withdrawn } @@ -413,7 +427,7 @@ end context 'with only published versions' do - subject(:work) { create :work, versions: [v2, v1] } + subject(:work) { create(:work, versions: [v2, v1]) } it { is_expected.not_to be_withdrawn } @@ -425,7 +439,7 @@ end context 'with only a draft version' do - subject(:work) { create :work, versions: [draft] } + subject(:work) { create(:work, versions: [draft]) } it { is_expected.not_to be_withdrawn } @@ -437,7 +451,7 @@ end context 'with only a withdrawn version' do - subject(:work) { create :work, versions: [withdrawn] } + subject(:work) { create(:work, versions: [withdrawn]) } it { is_expected.to be_withdrawn } @@ -656,7 +670,7 @@ end describe '#stats' do - subject(:work) { create :work, versions_count: 3, has_draft: true } + subject(:work) { create(:work, versions_count: 3, has_draft: true) } before { allow(AggregateViewStatistics).to receive(:call).and_return(:returned_stats) } @@ -668,11 +682,11 @@ describe '#thumbnail_urls' do let(:mock_attacher) { instance_double FileUploader::Attacher } - let!(:work) { create :work, versions_count: 2 } + let!(:work) { create(:work, versions_count: 2) } context "when work's latest_published_version has multiple file_resources with thumbnail urls" do before do - work.latest_published_version.file_resources << (create :file_resource) + work.latest_published_version.file_resources << (create(:file_resource)) work.save allow(mock_attacher).to receive(:url).with(:thumbnail).and_return 'url.com/path/file' end @@ -687,11 +701,11 @@ end describe '#latest_published_version_dois' do - let!(:work) { create :work } + let!(:work) { create(:work) } let(:work_version) do - create :work_version, :published, + create(:work_version, :published, doi: '10.26207/utaj-jfhi', - identifier: '10.26207/xyz-lmno' + identifier: '10.26207/xyz-lmno') end before do diff --git a/spec/models/work_version_spec.rb b/spec/models/work_version_spec.rb index 824a88d59..0d5640099 100644 --- a/spec/models/work_version_spec.rb +++ b/spec/models/work_version_spec.rb @@ -2,7 +2,7 @@ require 'rails_helper' -RSpec.describe WorkVersion, type: :model do +RSpec.describe WorkVersion do it_behaves_like 'a resource with view statistics' do let(:resource) { create(:work_version) } end @@ -100,6 +100,33 @@ it { is_expected.to transition_from(:withdrawn).to(:removed).on_event(:remove) } end + describe 'state transition events' do + let(:work) { create(:work) } + let!(:work_version) { create(:work_version, :published, work: work) } + + before { allow(WorkRemovedWebhookJob).to receive(:perform_later) } + + describe '#withdraw!' do + context 'when withdrawing the work version causes the parent work to be withdrawn' do + it 'enqueues a notification job' do + work_version.withdraw! + + expect(WorkRemovedWebhookJob).to have_received(:perform_later).with(work.uuid) + end + end + + context 'when withdrawing the work version does not cause the parent work to be withdrawn' do + before { create(:work_version, :published, work: work) } + + it 'does not enqueue a notification job' do + work_version.withdraw! + + expect(WorkRemovedWebhookJob).not_to have_received(:perform_later).with(work.uuid) + end + end + end + end + describe 'validations' do subject(:work_version) { build(:work_version) } @@ -263,7 +290,7 @@ it { is_expected.to delegate_method(:thumbnail_url).to(:work) } describe 'after save' do - let(:work_version) { build :work_version, :published } + let(:work_version) { build(:work_version, :published) } # I've heard it's bad practice to mock the object under test, but I can't # think of a better way to do this without testing the contents of @@ -277,7 +304,7 @@ end describe '.build_with_empty_work' do - let(:depositor) { build :actor, surname: 'Expected Depositor' } + let(:depositor) { build(:actor, surname: 'Expected Depositor') } it 'builds a WorkVersion with an initialized work' do wv = described_class.build_with_empty_work(depositor: depositor) @@ -342,7 +369,7 @@ end describe '#resource_with_doi' do - let(:work) { build_stubbed :work } + let(:work) { build_stubbed(:work) } let(:work_version) { described_class.new(work: work) } it 'returns the parent work' do @@ -351,8 +378,8 @@ end describe '#build_creator' do - let(:actor) { build_stubbed :actor } - let(:work_version) { build_stubbed :work_version, :with_creators, creator_count: 0 } + let(:actor) { build_stubbed(:actor) } + let(:work_version) { build_stubbed(:work_version, :with_creators, creator_count: 0) } it 'builds a creator for the given Actor but does not persist it' do expect { @@ -486,7 +513,7 @@ end describe '#publish' do - let(:work_version) { build :work_version, :able_to_be_published } + let(:work_version) { build(:work_version, :able_to_be_published) } let(:work) { work_version.work } it "updates the work's deposit agreement" do @@ -508,7 +535,7 @@ end context 'with a masters_culminating_experience work type' do - let(:work_version) { create :work_version, :grad_culminating_experience_able_to_be_published } + let(:work_version) { create(:work_version, :grad_culminating_experience_able_to_be_published) } it 'sets the publisher to ScholarSphere automatically' do work_version.save @@ -520,8 +547,8 @@ end context 'with a professional_doctoral_culminating_experience work type' do - let(:work_version) { create :work_version, :grad_culminating_experience_able_to_be_published, - work: build(:work, work_type: 'professional_doctoral_culminating_experience') } + let(:work_version) { create(:work_version, :grad_culminating_experience_able_to_be_published, + work: build(:work, work_type: 'professional_doctoral_culminating_experience')) } it 'sets the publisher to ScholarSphere automatically' do work_version.save @@ -533,7 +560,7 @@ end context 'with a work that is not a masters_culminating_experience or a professional_doctoral_culminating_experience' do - let(:work_version) { create :work_version, :able_to_be_published, work: build(:work, work_type: 'article') } + let(:work_version) { create(:work_version, :able_to_be_published, work: build(:work, work_type: 'article')) } it 'does not edit the publisher field' do work_version.save @@ -685,10 +712,10 @@ describe '#set_thumbnail_selection' do context 'when associated work does not have any published versions' do - let(:work) { create :work, has_draft: true } + let(:work) { create(:work, has_draft: true) } before do - work.versions.last.file_resources << (create :file_resource) + work.versions.last.file_resources << (create(:file_resource)) work.versions.last.save end @@ -709,10 +736,10 @@ end context 'when associated work does have a published version' do - let(:work) { create :work, versions_count: 1, has_draft: false } + let(:work) { create(:work, versions_count: 1, has_draft: false) } before do - work.versions.last.file_resources << (create :file_resource) + work.versions.last.file_resources << (create(:file_resource)) work.versions.last.save end diff --git a/spec/services/work_removed_webhook_spec.rb b/spec/services/work_removed_webhook_spec.rb new file mode 100644 index 000000000..a49bf9e6e --- /dev/null +++ b/spec/services/work_removed_webhook_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe WorkRemovedWebhook do + let(:webhook) { described_class.new('abc123') } + + describe '#notify' do + let(:faraday_connection) { instance_spy Faraday::Connection } + + before do + allow(Faraday).to receive(:new).with( + url: ENV['RMD_HOST'], + headers: { 'X-API-KEY' => ENV['RMD_WEBHOOK_SECRET'] } + ).and_return faraday_connection + end + + it 'posts the URL for the work to the RMD webhook endpoint' do + webhook.notify + + expect(faraday_connection).to have_received(:post).with( + '/webhooks/scholarsphere_events', + publication_url: 'https://scholarsphere.psu.edu/resources/abc123' + ) + end + end +end