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

DOI minting for data and code pathway #1584

Merged
merged 7 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from 5 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
8 changes: 7 additions & 1 deletion app/controllers/dashboard/form/base_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@ def resource_klass
end
end

def process_response(on_error:, validation_context: nil)
def process_response(on_error:, validation_context: nil, &block)
Smullz622 marked this conversation as resolved.
Show resolved Hide resolved
respond_to do |format|
if save_resource(validation_context: validation_context)
yield if block
format.html do
redirect_upon_success
end
Expand Down Expand Up @@ -82,6 +83,11 @@ def allow_curation?
deposit_pathway.allows_curation_request? && in_publish_edit_action?
end

helper_method :allow_mint_doi?
def allow_mint_doi?
deposit_pathway.allows_mint_doi_request?
end

helper_method :allow_publish?
def allow_publish?
if deposit_pathway.work?
Expand Down
9 changes: 8 additions & 1 deletion app/controllers/dashboard/form/publish_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
module Dashboard
module Form
class PublishController < BaseController
class DoiIsMintingError < RuntimeError; end

def edit
@work_version = WorkVersion
.includes(file_version_memberships: [:file_resource])
Expand Down Expand Up @@ -59,7 +61,11 @@ def update
end

validation_context = current_user.admin? ? nil : :user_publish
process_response(on_error: :edit, validation_context: validation_context)
process_response(on_error: :edit, validation_context: validation_context) do
if @resource.mint_doi_requested
allow_mint_doi? ? MintDoiAsync.call(@resource.work) : flash[:error] = t('dashboard.form.publish.doi.error')
end
end
end

private
Expand Down Expand Up @@ -115,6 +121,7 @@ def work_version_params
:published_date,
:depositor_agreement,
:draft_curation_requested,
:mint_doi_requested,
keyword: [],
contributor: [],
publisher: [],
Expand Down
6 changes: 6 additions & 0 deletions app/forms/work_deposit_pathway.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ def allows_curation_request?
data_and_code? && [email protected]_curation_requested
end

def allows_mint_doi_request?
data_and_code? && @resource.doi_blank? && DoiMintingStatus.new(@resource.work).blank?
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should have some unit tests for this, since there's a handful of conditions.

end

def work?
Work::Types.all.include?(work_type)
end
Expand Down Expand Up @@ -117,6 +121,7 @@ def imported_metadata_from_rmd?
:update_doi=,
:work_type,
:draft_curation_requested,
:mint_doi_requested,
to: :work_version, prefix: false

private
Expand Down Expand Up @@ -258,6 +263,7 @@ def form_partial
:aasm,
:update_column,
:draft_curation_requested=,
:mint_doi_requested=,
:set_thumbnail_selection,
to: :work_version,
prefix: false
Expand Down
4 changes: 4 additions & 0 deletions app/models/work_version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,10 @@ def update_doi?
super && latest_published_version?
end

def doi_blank?
work.doi.blank?
end

def set_thumbnail_selection
# work.versions.published.blank? lets us know if the work has just been created
if work.versions.published.blank? && file_resources.map(&:thumbnail_url).compact.present?
Expand Down
4 changes: 4 additions & 0 deletions app/services/doi_minting_status.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ def present?
resource.present? && current_status.present?
end

def blank?
!present?
end

def delete!
redis.del(key)
end
Expand Down
20 changes: 20 additions & 0 deletions app/views/dashboard/form/publish/_doi.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<div class="form-group mt-5">
<div class="keyline keyline--left">
<h4><%= t '.header' %></h4>
</div>
<div>
<div>
<%= t '.request_description' %>
</div>
<br>
<div class="form-check">
<%= form.check_box :mint_doi_requested,
required: false,
class: 'form-check-input' %>

<%= form.label :mint_doi_requested, class: 'form-check-label' do %>
<%= t '.label' %>
<% end %>
</div>
</div>
</div>
4 changes: 4 additions & 0 deletions app/views/dashboard/form/publish/_fields.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@
<%= render 'curation', form: form %>
<% end %>

<% if allow_mint_doi? %>
<%= render 'doi', form: form %>
<% end %>

<div class="form-group mt-5">
<div class="keyline keyline--left">
<h2 id="<%= WorkVersion.human_attribute_name(:depositor_agreement).parameterize %>" class="h4">
Expand Down
5 changes: 5 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,11 @@ en:
curation_requested: Curation has been requested. We will notify you when curation is complete and your work is ready to be published. If you have any questions in the meantime, please contact ScholarSphere curators via our
error: There was an error with your curation request. Please try again later or contact us if the problem persists.
contact: contact form
doi:
header: Mint DOI (Optional)
request_description: Placeholder for mint doi description
label: I would like to mint a DOI upon publish.
error: There was an error minting your doi. Please try again later from Work Settings or contact us if the problem persists.
members:
heading: Works
badge: Work
Expand Down
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this information actually need to be saved to the database? In other words, do we ever need to recall it from the database after it has been saved? Or do we only care about it for the duration of the request where this form field is submitted? Could it simply be added as a transient attribute on WorkVersion instead of a persistent database column?

Or maybe it doesn't even need to be an attribute of WorkVersion at all. Is it enough for it to just be an attribute of the form object so that we only use it to determine whether or not to queue up the DOI minting and then maybe we don't even pass it along to the WorkVersion?

I'm asking as if I actually know the answer, but I don't. I don't immediately see why this would need to be saved, but there might be a reason that I don't know about. I just wanted to make sure that it was something that you considered.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I was debating about that as well. I landed on saving it since users can save & exit the form and come back later to publish. My instinct was that users who previously checked the box might assume it was still checked, submit without rechecking, and incorrectly think they're minting a doi upon publish. I could still go either way though & am open to suggestion.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah! I hadn't thought of that, and it totally makes sense. You're way ahead of me. 🤐

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddMintDoiRequestedToWorkVersion < ActiveRecord::Migration[6.1]
def change
add_column :work_versions, :mint_doi_requested, :boolean
end
end
4 changes: 3 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 2024_03_21_203236) do
ActiveRecord::Schema.define(version: 2024_09_10_195820) do

# These are extensions that must be enabled in order to support this database
enable_extension "pg_stat_statements"
enable_extension "plpgsql"
Expand Down Expand Up @@ -263,6 +264,7 @@
t.boolean "imported_metadata_from_rmd"
t.boolean "draft_curation_requested"
t.datetime "sent_for_curation_at"
t.boolean "mint_doi_requested"
t.index ["external_app_id"], name: "index_work_versions_on_external_app_id"
t.index ["work_id", "version_number"], name: "index_work_versions_on_work_id_and_version_number", unique: true
t.index ["work_id"], name: "index_work_versions_on_work_id"
Expand Down
66 changes: 66 additions & 0 deletions spec/features/dashboard/work_version_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1242,4 +1242,70 @@ def mock_solr_indexing_job
end
end
end

describe 'minting a doi', js: true do
let(:user) { work_version.work.depositor.user }

context 'with a draft eligible for doi minting' do
let(:work) { create :work, work_type: 'dataset', versions_count: 1, has_draft: true, doi: nil }
let(:work_version) { work.versions.first }

it 'renders a checkbox requesting a doi be minted upon publish' do
visit dashboard_form_publish_path(work_version)

expect(page).to have_content(I18n.t('dashboard.form.publish.doi.label'))
end
end

context 'with a draft not eligible for doi minting' do
let(:work) { create :work, work_type: 'dataset', versions_count: 1, has_draft: true, doi: '10.26207/x8qe-xp35' }
let(:work_version) { work.versions.first }

it 'does not render a checkbox requesting a doi be minted upon publish' do
visit dashboard_form_publish_path(work_version)

expect(page).not_to have_content(I18n.t('dashboard.form.publish.doi.label'))
end
end

context 'when minting doi is successfully requested' do
let(:work_version) { create :work_version, :dataset_able_to_be_published }
let(:work) { create :work, work_type: 'dataset', versions_count: 1, has_draft: true, doi: nil, versions: [work_version] }

before do
allow(MintDoiAsync).to receive(:call).with(work_version.work)
end

it 'calls the MintDoi job' do
visit dashboard_form_publish_path(work_version)

check I18n.t('dashboard.form.publish.doi.label')
check 'I have read and agree to the deposit agreement.'

click_on 'Publish'

expect(MintDoiAsync).to have_received(:call).with(work_version.work)
end
end

context 'when an error occurs and the work version does not publish' do
let(:work_version) { create :work_version, :dataset_able_to_be_published }
let(:work) { create :work, work_type: 'dataset', versions_count: 1, has_draft: true, doi: nil, versions: [work_version] }

before do
work_version.work.doi = nil
allow(MintDoiAsync).to receive(:call).with(work_version.work)
end

it 'does not mint a doi' do
visit dashboard_form_publish_path(work_version)

check I18n.t('dashboard.form.publish.doi.label')

click_on 'Publish'

expect(MintDoiAsync).not_to have_received(:call).with(work_version.work)
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,17 @@
end
end

describe '#mint_doi_requested=' do
before { allow(wv).to receive(:mint_doi_requested=) }

let(:arg) { double }

it 'delegates to the given work version' do
form.mint_doi_requested = arg
expect(wv).to have_received(:mint_doi_requested=).with(arg)
end
end

describe '.form_fields' do
it "returns a frozen array of the names of the form's fields" do
expect(described_class.form_fields).to match_array %w{
Expand Down
1 change: 1 addition & 0 deletions spec/models/work_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,7 @@
keyword_sim
keyword_tesim
language_tesim
mint_doi_requested_tesim
model_ssi
proxy_id_isi
published_date_dtrsi
Expand Down