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

Special case 10.5555 DOIs (and fix ambiguous DOI checker results) #106

Merged
merged 1 commit into from
Aug 20, 2024

Conversation

sneakers-the-rat
Copy link
Contributor

@sneakers-the-rat sneakers-the-rat commented Aug 16, 2024

There's a recurring issue where the DOI checker flags "10.5555" prefixed DOIs as missing with the ambiguous message I left in #101 (my bad). Those DOIs are known to not resolve.

This PR special-cases 10.5555 prefixed DOIs such that

  • if they are present in the {doi} field they are marked as invalid with a prompt to include it in the {url} field instead
  • If they are present in the {url} field they are marked as :skip with a note for the editor to confirm they resolve - I tried testing the resolution, but unhelpfully the website seems to respond with a 200 code and display a 404 page......

To do that, I

  • moved the body of the checker to a handle_missing_doi method
  • made a handle_special_case method to allow for future special cases, if any

I also fixed the ambiguous message by adding a skip type that is "maybe ok, editor take a look" field and made the template render categories with emoji that indicate the status of each DOI, it looks like...

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

✅ - OK DOIs

- this value is ok
- another ok value

❔ - SKIP DOIs

- this value is skipped but may still be ok
- https://dl.acm.org/doi/10.5555/xxxx.yyyy  - correctly put 10.5555 prefixed doi in the url field, editor should ensure this resolves

❌ - MISSING DOIs

- this one needs a doi
- so does this one

❌ - INVALID DOIs

- 10.5555/xxxx.yyyy is INVALID - 10.5555 is a known broken prefix, replace with https://dl.acm.org/doi/{doi} in the {url} field"

My ruby is bad, so someone else please check my work here!

Copy link
Member

@arfon arfon left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me. @xuanxu – I think you usually take Buffy deploys out for a spin on Heroku – would you recommend we do a branch deploy here or are you happy to merge?

Copy link
Member

@xuanxu xuanxu left a comment

Choose a reason for hiding this comment

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

👍 Let's merge it

@xuanxu xuanxu merged commit 42fcd98 into openjournals:main Aug 20, 2024
5 checks passed
@xuanxu
Copy link
Member

xuanxu commented Aug 20, 2024

In action (I've changed the "skip" symbol to a more visible one):
openjournals/joss-reviews#7116 (comment)

@sneakers-the-rat
Copy link
Contributor Author

yeah that looks better, i was looking for a yellow indicator with a question mark that wasn't the ⚠️ which seemed too threatening.

looking at that review^ it seems like maybe there could be more info for "no doi given and none found" kind of citations? i don't mean to overcomplicate this one very small part of the review system, it just seems to come up semi-frequently - i had meant to add a similar directive like "editor check if this resolves" for "editor manually check this reference" but if it works fine as-is then great we can call it shipped and move on <3.

@arfon
Copy link
Member

arfon commented Aug 21, 2024

no doi given and none found

I do think this language is a little confusing to people sometimes. @danielskatz had flagged this a while back too.

So yes, 👍 to making this more directive to editors/authors.

@danielskatz
Copy link

I suggested in Slack, and repeat here for tracking:

I wonder if we can change some DOI checker language? When I see this
https://dl.acm.org/doi/10.5555/1251460.1251470 - correctly put 10.5555 prefixed doi in the url field, editor should ensure this resolves
it's unclear to me what part of it is fact and what part is instructions, along with the error that this is not a DOI...

Could we instead say
https://dl.acm.org/doi/10.5555/1251460.1251470 - non-DOI with 10.5555 correctly placed in the url field, editor should ensure this resolves

And also maybe change the category for this, as Skip DOI doesn't make sense, since these aren't actually DOIs...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants