Skip to content

Commit

Permalink
Merge branch 'main' into joss
Browse files Browse the repository at this point in the history
  • Loading branch information
xuanxu committed Aug 20, 2024
2 parents d59249a + 37bbbab commit 26e0b46
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 30 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ jobs:
continue-on-error: ${{ matrix.failure-allowed }}
strategy:
matrix:
ruby-version: ['3.0.7', '3.1.5', '3.2.4', '3.3.3']
ruby-version: ['3.0.7', '3.1.6', '3.2.5', '3.3.4']
failure-allowed: [false]
include:
- ruby-version: 'truffleruby'
Expand Down
17 changes: 10 additions & 7 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ GEM
racc (~> 1.7)
bigdecimal (3.1.8)
cgi (0.4.1)
charlock_holmes (0.7.7)
charlock_holmes (0.7.9)
chronic (0.10.2)
concurrent-ruby (1.3.3)
connection_pool (2.4.1)
Expand All @@ -18,8 +18,9 @@ GEM
rexml
diff-lcs (1.5.1)
dotenv (3.1.2)
faraday (2.9.2)
faraday (2.10.0)
faraday-net_http (>= 2.0, < 3.2)
logger
faraday-follow_redirects (0.1.0)
faraday (>= 2, < 3)
faraday-net_http (3.1.0)
Expand All @@ -42,6 +43,7 @@ GEM
reverse_markdown (>= 1, < 3)
rugged (>= 0.24, < 2.0)
thor (>= 0.19, < 2.0)
logger (1.6.0)
mini_mime (1.1.5)
mini_portile2 (2.8.7)
multi_json (1.15.0)
Expand All @@ -63,7 +65,7 @@ GEM
puma (6.4.2)
nio4r (~> 2.0)
racc (1.8.0)
rack (3.1.4)
rack (3.1.7)
rack-protection (4.0.0)
base64 (>= 0.1.0)
rack (>= 3.0.0, < 4)
Expand All @@ -75,7 +77,7 @@ GEM
connection_pool
reverse_markdown (2.1.1)
nokogiri
rexml (3.3.1)
rexml (3.3.4)
strscan
rspec (3.13.0)
rspec-core (~> 3.13.0)
Expand All @@ -101,11 +103,12 @@ GEM
multi_json (~> 1.15)
rexml (~> 3.2, >= 3.2.5)
thor (~> 1.2, >= 1.2.1)
sidekiq (7.2.4)
sidekiq (7.3.0)
concurrent-ruby (< 2)
connection_pool (>= 2.3.0)
logger
rack (>= 2.2.4)
redis-client (>= 0.19.0)
redis-client (>= 0.22.2)
sinatra (4.0.0)
mustermann (~> 3.0)
rack (>= 3.0.0, < 4)
Expand All @@ -120,7 +123,7 @@ GEM
tilt (~> 2.0)
strscan (3.1.0)
thor (1.3.1)
tilt (2.3.0)
tilt (2.4.0)
uri (0.13.0)
webmock (3.23.1)
addressable (>= 2.8.0)
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

A service to provide a bot helping scientific journals and communities manage submission reviews.

Buffy automates common editorial tasks like those needed by [The Journal of Open Source Software](https://joss.theoj.org/) or [rOpenSci](https://ropensci.org/).
Buffy automates common editorial tasks like those needed by [The Journal of Open Source Software](https://joss.theoj.org/), [rOpenSci](https://ropensci.org/), [JuliaCon](https://proceedings.juliacon.org/) or [pyOpenSci](https://www.pyopensci.org/).

[![Build Status](https://github.com/openjournals/buffy/actions/workflows/tests.yml/badge.svg)](https://github.com/openjournals/buffy/actions/workflows/tests.yml)
[![Documentation Status](https://readthedocs.org/projects/buffy/badge/?version=latest)](https://buffy.readthedocs.io/en/latest/?badge=latest)
60 changes: 45 additions & 15 deletions app/lib/doi_checker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,34 +9,52 @@ def initialize(entries=[])
end

def check_dois
doi_summary = {ok: [], missing: [], invalid: []}
doi_summary = {ok: [], skip: [], missing: [], invalid: []}

if @entries.any?
@entries.each do |entry|
if entry.has_field?('doi') && !entry.doi.empty?
# handle special cases first
if special_case = handle_special_case(entry)
doi_validity = special_case
elsif entry.has_field?('doi') && !entry.doi.empty?
# Validate entries with DOIs
doi_validity = validate_doi(entry.doi.value)
doi_summary[doi_validity[:validity]].push(doi_validity[:msg])
# If there's no DOI present, check Crossref to see if we can find a candidate DOI for this entry.
elsif entry.has_field?('title')
candidate_doi = crossref_lookup(entry.title.value)
truncated_title = entry.title.to_s[0,50]
truncated_title += "..." if truncated_title.length < entry.title.to_s.length
if candidate_doi == "CROSSREF-ERROR"
doi_summary[:missing].push("Errored finding suggestions for \"#{truncated_title}\", please try later")
elsif candidate_doi
doi_summary[:missing].push("#{candidate_doi} may be a valid DOI for title: #{truncated_title}")
else
doi_summary[:missing].push("No DOI given, and none found for title: #{truncated_title}")
end
# Try and find candidate entries if doi absent, but title present
doi_validity = handle_missing_doi(entry)
else
doi_summary[:missing].push("Entry without DOI or title found")
doi_validity = {validity: :missing, msg: "Entry without DOI or title found"}
end

doi_summary[doi_validity[:validity]].push(doi_validity[:msg])
end
end

doi_summary
end

# any special case should return false if not applicable, and an object like
# {:validity => :ok, :msg => "whatever"} otherwise.
# Add additional special cases as private methods and chain in a tidy sequence plz <3
def handle_special_case(entry)
acm_105555_prefix(entry) || false
end


# If there's no DOI present, check Crossref to see if we can find a candidate DOI for this entry.
def handle_missing_doi(entry)
candidate_doi = crossref_lookup(entry.title.value)
truncated_title = entry.title.to_s[0,50]
truncated_title += "..." if truncated_title.length < entry.title.to_s.length
if candidate_doi == "CROSSREF-ERROR"
{ validity: :missing, msg: "Errored finding suggestions for \"#{truncated_title}\", please try later" }
elsif candidate_doi
{ validity: :missing, msg: "#{candidate_doi} may be a valid DOI for title: #{truncated_title}" }
else
{ validity: :skip, msg: "No DOI given, and none found for title: #{truncated_title}" }
end
end

def validate_doi(doi_string)
return { validity: :invalid, msg: "Empty DOI string" } if doi_string.nil? || doi_string.empty?

Expand Down Expand Up @@ -112,4 +130,16 @@ def levenshtein_distance(s, t)
def similar?(string_1, string_2)
levenshtein_distance(string_1, string_2) < 3
end

private

def acm_105555_prefix(entry)
if entry.has_field?('doi') && entry.doi.include?("10.5555/")
{ validity: :invalid, msg: "#{entry.doi} is INVALID - 10.5555 is a known broken prefix, replace with https://dl.acm.org/doi/{doi} in the {url} field" }
elsif entry.has_field?('url') && entry.url.include?("https://dl.acm.org/doi/10.5555")
{ validity: :skip, msg: "#{entry.url} - correctly put 10.5555 prefixed doi in the url field, editor should ensure this resolves" }
else
false
end
end
end
14 changes: 9 additions & 5 deletions app/responses/doi_checks.erb
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
```
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):
<% doi_summary.each do |type, messages| -%>

<%= type.to_s.upcase %> DOIs

<% if type.to_s === "ok" %>
<%= type.to_s.upcase %> DOIs
<% elsif type.to_s === "skip" %>
🟡 <%= type.to_s.upcase %> DOIs
<% else %>
<%= type.to_s.upcase %> DOIs
<% end %>
<% if messages.empty? -%>
- None
<% else -%>
<% messages.each do |message| -%>
- <%= message%>
<% messages.each do |message| -%>
- <%= message %>
<% end -%>
<% end -%>
<% end -%>
Expand Down
38 changes: 37 additions & 1 deletion spec/doi_checker_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
expect(doi_summary[:ok]).to be_empty
expect(doi_summary[:invalid]).to be_empty
expect(doi_summary[:missing]).to be_empty
expect(doi_summary[:skip]).to be_empty
end

it "should classify as invalid entries with invalid DOI" do
Expand All @@ -25,6 +26,7 @@
expect(doi_summary[:invalid].size).to eq(1)
expect(doi_summary[:invalid].first).to eq(validity[:msg])
expect(doi_summary[:missing]).to be_empty
expect(doi_summary[:skip]).to be_empty
end

it "should classify as ok entries with valid DOI" do
Expand All @@ -39,6 +41,7 @@
expect(doi_summary[:ok].first).to eq(validity[:msg])
expect(doi_summary[:invalid]).to be_empty
expect(doi_summary[:missing]).to be_empty
expect(doi_summary[:skip]).to be_empty
end

it "should classify as missing entries without DOI but with a candidate crossref entry" do
Expand All @@ -50,6 +53,7 @@
doi_summary = doi_checker.check_dois
expect(doi_summary[:ok]).to be_empty
expect(doi_summary[:invalid]).to be_empty
expect(doi_summary[:skip]).to be_empty
expect(doi_summary[:missing].size).to eq(1)
expect(doi_summary[:missing][0]).to eq("10.maybe/doi may be a valid DOI for title: No DOI")
end
Expand All @@ -63,6 +67,7 @@
doi_summary = doi_checker.check_dois
expect(doi_summary[:ok]).to be_empty
expect(doi_summary[:invalid]).to be_empty
expect(doi_summary[:skip]).to be_empty
expect(doi_summary[:missing].size).to eq(1)
expect(doi_summary[:missing][0]).to eq('Errored finding suggestions for "No DOI", please try later')
end
Expand All @@ -78,6 +83,7 @@
doi_summary = doi_checker.check_dois
expect(doi_summary[:ok]).to be_empty
expect(doi_summary[:invalid]).to be_empty
expect(doi_summary[:skip]).to be_empty
expect(doi_summary[:missing].size).to eq(1)
expect(doi_summary[:missing][0]).to eq("Errored finding suggestions for \"#{expected_title}\", please try later")
end
Expand All @@ -93,7 +99,9 @@

expect(doi_summary[:ok]).to be_empty
expect(doi_summary[:invalid]).to be_empty
expect(doi_summary[:missing][0]).to eq("No DOI given, and none found for title: #{title}")
expect(doi_summary[:missing]).to be_empty
expect(doi_summary[:skip].size).to eq(1)
expect(doi_summary[:skip][0]).to eq("No DOI given, and none found for title: #{title}")
end

it "should report entries with no DOI or title as missing both" do
Expand All @@ -103,10 +111,38 @@
doi_summary = doi_checker.check_dois
expect(doi_summary[:ok]).to be_empty
expect(doi_summary[:invalid]).to be_empty
expect(doi_summary[:skip]).to be_empty
expect(doi_summary[:missing][0]).to eq("Entry without DOI or title found")
end
end

describe "#handle_special_case" do
it "should treat DOIs with a 10.5555 prefix as invalid" do
entry = BibTeX::Entry.new(doi: "10.5555/xxxxxxx.yyyyyyyyy")
validity = subject.handle_special_case(entry)
expect(validity[:validity]).to eq(:invalid)
expect(validity[:msg]).to include("replace with https://dl.acm.org/doi")
end

it "should treat URLs with a 10.5555 prefix as a skip" do
entry = BibTeX::Entry.new(url: "https://dl.acm.org/doi/10.5555/2827719.2827740")
validity = subject.handle_special_case(entry)
expect(validity[:validity]).to eq(:skip)
expect(validity[:msg]).to eq("https://dl.acm.org/doi/10.5555/2827719.2827740 - correctly put 10.5555 prefixed doi in the url field, editor should ensure this resolves")
end

it "should handle special cases separately from normal DOI checking" do
entry = BibTeX::Entry.new(doi: "10.5555/xxxxxxx.yyyyyyyyy")
doi_checker = DOIChecker.new([entry])

doi_summary = doi_checker.check_dois
expect(doi_summary[:ok]).to be_empty
expect(doi_summary[:missing]).to be_empty
expect(doi_summary[:skip]).to be_empty
expect(doi_summary[:invalid][0]).to include("is INVALID - 10.5555 is a known broken prefix, replace with https://dl.acm.org/doi/")
end
end

describe "#validate_doi" do

it "should invalidate empty doi strings" do
Expand Down

0 comments on commit 26e0b46

Please sign in to comment.