Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow events to trigger on specific conditions #1052

Merged
merged 6 commits into from
Dec 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 5 additions & 11 deletions app/models/concerns/indexable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,11 @@ module Indexable
end
end

if (
instance_of?(DataciteDoi) || instance_of?(OtherDoi) ||
instance_of?(Doi)
) &&
(
saved_change_to_attribute?("related_identifiers") ||
saved_change_to_attribute?("creators") ||
saved_change_to_attribute?("funding_references")
)
if aasm_state == "findable" && !Rails.env.test?
send_import_message(to_jsonapi)
if instance_of?(DataciteDoi) || instance_of?(OtherDoi) || instance_of?(Doi)
if aasm_state == "findable"
changed_attributes = saved_changes
relevant_changes = changed_attributes.keys & %w[related_identifiers creators funding_references aasm_state]
send_import_message(to_jsonapi) if relevant_changes.any?
Comment on lines +25 to +27
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like this refactor away from an ever-increasing list of boolean clauses, very elegant!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @digitaldogsbody :)

Also saved_change_to_attribute? this method only execute when we have update call, so we never captured the create DOI call before because of this method checks.

We missed the events for the DOI which are created directly with findable state.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saved_changes will work for both create and update action.

end
elsif instance_of?(Event)
OtherDoiJob.perform_later(dois_to_import)
Expand Down
79 changes: 79 additions & 0 deletions spec/models/doi_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,85 @@
end
end

describe "after_commit" do
let(:doi) { create(:doi, aasm_state: "findable") }
let(:sqs_client) { instance_double(Aws::SQS::Client) }

before do
allow_any_instance_of(DataciteDoi).to receive(:send_message)
allow(Aws::SQS::Client).to receive(:new).and_return(sqs_client)
allow(IndexJob).to receive(:perform_later)
allow(DataciteDoi).to receive_message_chain(:__elasticsearch__, :index_document)
end

context "On Update event" do
it "sends import message if relevant attributes are modified" do
travel_to(Time.zone.local(2023, 12, 14, 10, 7, 40)) do
expect(doi).to receive(:send_import_message).with(doi.to_jsonapi)

doi.update(related_identifiers: [{ "relatedIdentifier" => "new_identifier", "relatedIdentifierType" => "DOI", "relationType" => "IsPartOf" }])
end
end

it "sends import message if creators are modified" do
travel_to(Time.zone.local(2023, 12, 14, 10, 7, 40)) do
expect(doi).to receive(:send_import_message).with(doi.to_jsonapi)

doi.update(creators: [{ "nameType" => "Personal", "name" => "New Creator" }])
end
end

it "sends import message if funding_references are modified" do
travel_to(Time.zone.local(2023, 12, 14, 10, 7, 40)) do
jrhoads marked this conversation as resolved.
Show resolved Hide resolved
expect(doi).to receive(:send_import_message).with(doi.to_jsonapi)

doi.update(funding_references: [{ "funder" => "New Funder", "title" => "New Title" }])
end
end

it "does not send import message if no relevant attributes are modified" do
expect(doi).not_to receive(:send_import_message)

doi.update(titles: "New Title")
end

it "does not send import message if aasm_state is not 'findable'" do
expect(doi).not_to receive(:send_import_message)

doi.update(aasm_state: "draft")
end

it "does not send import message after create if aasm_state is not 'findable'" do
new_doi = create(:doi, aasm_state: "draft")

expect(new_doi).not_to receive(:send_import_message)
end
end

context "On Create event" do
it "sends import message after create if aasm_state is 'findable'" do
travel_to(Time.zone.local(2023, 12, 14, 10, 7, 40)) do
new_doi = build(:doi, aasm_state: "findable")
allow(new_doi).to receive(:send_import_message)
new_doi.save

# Sleep for a short duration to ensure the asynchronous after_commit has completed
sleep 1

expect(new_doi).to have_received(:send_import_message).with(new_doi.to_jsonapi)
end
end

it "does not send import message after create if aasm_state is not 'findable'" do
travel_to(Time.zone.local(2023, 12, 14, 10, 7, 40)) do
new_doi = create(:doi, aasm_state: "draft")

expect(new_doi).not_to receive(:send_import_message)
end
end
end
end

describe "validate agency" do
it "DataCite" do
subject = build(:doi, agency: "DataCite")
Expand Down
1 change: 1 addition & 0 deletions spec/rails_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
config.include RSpec::Benchmark::Matchers
config.include Rack::Test::Methods, type: :request
config.include RSpec::GraphqlMatchers::TypesHelper
config.include ActiveSupport::Testing::TimeHelpers

# don't use transactions, use database_clear gem via support file
config.use_transactional_fixtures = false
Expand Down
5 changes: 4 additions & 1 deletion spec/requests/events_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -925,7 +925,10 @@
end

context "show" do
let(:doi) { create(:doi, client: client, aasm_state: "findable") }
let(:doi) do
allow_any_instance_of(DataciteDoi).to receive(:send_import_message)
create(:doi, client: client, aasm_state: "findable")
end
let(:source_doi) { create(:doi, client: client, aasm_state: "findable") }
let!(:event) do
create(
Expand Down
5 changes: 4 additions & 1 deletion spec/requests/old_events_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -915,7 +915,10 @@
end

context "show" do
let(:doi) { create(:doi, client: client, aasm_state: "findable") }
let(:doi) do
allow_any_instance_of(DataciteDoi).to receive(:send_import_message)
jrhoads marked this conversation as resolved.
Show resolved Hide resolved
create(:doi, client: client, aasm_state: "findable")
end
let(:source_doi) { create(:doi, client: client, aasm_state: "findable") }
let!(:event) do
create(
Expand Down