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

Incorrect storage of ORCID's "put code" cause unintended deposit updates #10759

Open
taslangraham opened this issue Jan 2, 2025 · 9 comments
Assignees
Milestone

Comments

@taslangraham
Copy link
Contributor

taslangraham commented Jan 2, 2025

Describe the bug
The ORCID documentation (here & here) mentions that a put code is used to identify a single work item withing ORCID.

Put codes are short numeric codes that reference a particular item on the ORCID record. You use the put code with the API calls to update, delete or read a particular item (as opposed to a summary of items.)

When you post an item to a researcher's record, the API response will contain the put code for that item. You can store the put code to use it later if you need to read, update, or delete that item.

However, our current implementation stores the put code on the review(user)/author object via settings table instead of the individual work item (submission or review work). As a result, we do not have the put code for all items. This creates a problem, particularly when depositing new review work - a check is made for an existing put code on the reviewer(user) object, and if one is found(due to a previously deposited review work) it updates that record instead of creating a new entry.

https://github.com/pkp/ojs/blob/de0da4acfd19e8640ed777930f8beb060fc309c9/jobs/orcid/DepositOrcidReview.php#L74-L78

                if ($putCode = $reviewer->getData('orcidReviewPutCode')) {
                    $uri .= '/' . $putCode;
                    $method = 'PUT';
                    $orcidReview['put-code'] = $putCode;
                }

N.B: The logic for depositing submission work does not make use of the put code stored on the author's object.

To Reproduce
Steps to reproduce the behavior:

  1. Ensure ORCID is enabled in the application, along with test ORCID accounts
  2. Add ORCID to the profile of a Reviewer
  3. Request review from the reviewer for 2 submissions
  4. Log in as the reviewer, and respond to both review request
  5. Log in as Editor, confirm one of the reviews, and thank the reviewer
  6. Log into the Reviewer's ORCID account and notice that there is one new entry under the Peer review section
  7. Log in as editor again, confirm the second review, and thank the reviewer
  8. Log into the Reviewer's ORCID account again and notice that the new review work was not added

What application are you using?
OJS 3.5

PRs
pkp-lib: #10776
ojs : pkp/ojs#4580
omp: pkp/omp#1799
ops: pkp/ops#836

@taslangraham taslangraham added this to the 3.5 Internal milestone Jan 2, 2025
@taslangraham
Copy link
Contributor Author

After further reviewing the issue, I believe the way we are currently storing the put-code for submissions (via author settings) is fine and will work as expected. However, the method for handling reviewers' put-codes needs to be changed.

I have two ideas in mind to address this:

  1. Introduce a review_assignment_settings Table

For the review assignments, we could create a review_assignment_settings table where we store the put-code for each review

2. Introduce an orcid_records Table

Alternatively, we could introduce an orcid_records table that will store all put-codes related to both authors and reviewers. This table would have the following structure:

Column Name Type Description
orcid_records_id int primary key.
assoc_type string ASSOC_TYPE_REVIEW_ASSIGNMENT or ASSOC_TYPE_AUTHOR.
assoc_id int The ID of the review assignment or author.
put_code int The ORCID put-code associated with the record.
user_id (optional) int User this record is related to.

With this setup, we would store all put-codes related to both author and reviewer assignments in one central location.

Additionally, we could introduce an optional orcid_records_settings table to store more information about each ORCID deposit, if necessary. However, I don't see an immediate need for this.

I'm in favor of option 2 (without orcid_records_settings table) as it would allow us to keep all put-codes in one place.

@ewhanson let me know your thoughts on this, please.

@ewhanson
Copy link
Collaborator

ewhanson commented Jan 3, 2025

Hey @taslangraham, thanks for looking into this further. I think in this case, creating a review_assignments_setitngs table would be a better way to go.

If we do want to refactor the way ORCIDs work to have their own tables, I'd prefer to not do that so close to a release. I think option 2 is generally a good idea and something we can look into with future work on the ORCID functionality. As a first step towards that, you could add the information you've put together here and add it to a Github discussion (rather than an issue) as I think this would have benefits for some of the future work we've talked about for the ORCID functionality as well.

@taslangraham
Copy link
Contributor Author

@ewhanson Apologies for the delay. I've added the discussion here.

@taslangraham taslangraham self-assigned this Jan 6, 2025
taslangraham added a commit to taslangraham/ojs that referenced this issue Jan 7, 2025
taslangraham added a commit to taslangraham/omp that referenced this issue Jan 7, 2025
taslangraham added a commit to taslangraham/omp that referenced this issue Jan 7, 2025
taslangraham added a commit to taslangraham/ops that referenced this issue Jan 7, 2025
taslangraham added a commit to taslangraham/pkp-lib that referenced this issue Jan 7, 2025
… header reference in DepositOrcidSubmission
taslangraham added a commit to taslangraham/pkp-lib that referenced this issue Jan 7, 2025
… header reference in DepositOrcidSubmission
taslangraham added a commit to taslangraham/pkp-lib that referenced this issue Jan 7, 2025
… header reference in DepositOrcidSubmission
taslangraham added a commit to taslangraham/ojs that referenced this issue Jan 7, 2025
taslangraham added a commit to taslangraham/omp that referenced this issue Jan 7, 2025
taslangraham added a commit to taslangraham/ops that referenced this issue Jan 7, 2025
@taslangraham
Copy link
Contributor Author

Ready for review @ewhanson. I've added the PRs to the issue description

@ewhanson
Copy link
Collaborator

Thanks, @taslangraham. What is the plan for how to handle existing review put codes? It didn't look like these were taken into account as part of the migration.

@taslangraham
Copy link
Contributor Author

@ewhanson An oversight on my part. I'll look into it

@ewhanson
Copy link
Collaborator

Hey @taslangraham, here's a write up of what we talked about yesterday. Let me know if you have any thought or questions.


To remedy historical incorrect storage of put codes, including those that may have been overwritten or deleted, we should consider the following. When posting a review, the following should be done to ensure the correct put codes are used:

  1. Before a review is submitted, check if an orcidReviewPutCode is present in the user_settings for the user. This indicates that the update described below needs to be performed.

  2. If this is the case, instead of sending/updating the review, first dispatch a job that will update all put codes for any reviews submitted by the journal, then dispatch the original deposit ORCID review job.

  3. To get all of the put codes to update, query the ORCID endpoint as described below to get all peer review items. Each item in the group array corresponds to one peer review item. The specific ORCID URL will depend on which API (public vs. member, sandbox vs. production) so those helpers can be used. The <ORCID-ID> should be taken from the OJS user.

  4. Ultimately, the goal will be to match all applicable peer reviews returned from the ORCID API endpoint with items in the OJS database, which can be matched as follows:

    • The OJS user ID can be inferred because it is associated with the <ORCID-ID> used to make the query.
    • Whether the review came from this journal can be inferred by matching the ORCID client ID stored for the journal against an item within the group at item.peer-review-group[0].peer-review-summary[0].source.source-client-id.path. This will correspond to the ORCID client ID saved in OJS, and if it does not match, we can assume this review did not come from this OJS installation and can safely be ignored.
    • Instead of being associated with the user directly, the put codes will be stored in the new review_round_settings table. The review round ID is returned from the JSON at ``item.peer-review-group[0].peer-review-summary[0].external-ids.external-id[0].external-id-value. Note: I haven't seen any instances of OJS review contributions having more than one external ID objects in the JSON, but if there are, we are interested in the "external-id-type": "source-work-id"` one.
    • Having the user ID in OJS, the put code from ORCID, and the review round ID, this should be enough to disambiguate the review contributions and update the put codes before attempting any further deposit/update operations with ORCID.

Example response for peer-reviews endpoint, e.g. https://api.sandbox.orcid.org/v3.0/<ORCID-ID>/peer-reviews:

{
  "last-modified-date": {
    "value": 1737394131443
  },
  "group": [
    {
      "last-modified-date": {
        "value": 1737394131443
      },
      "external-ids": {
        "external-id": [
          {
            "external-id-type": "peer-review",
            "external-id-value": "issn:<ISSN>",
            "external-id-normalized": null,
            "external-id-normalized-error": null,
            "external-id-url": null,
            "external-id-relationship": null
          }
        ]
      },
      "peer-review-group": [
        {
          "last-modified-date": {
            "value": 1737394131443
          },
          "external-ids": {
            "external-id": []
          },
          "peer-review-summary": [
            {
              "created-date": {
                "value": 1737394131443
              },
              "last-modified-date": {
                "value": 1737394131443
              },
              "source": {
                "source-orcid": null,
                "source-client-id": {
                  "uri": "https://sandbox.orcid.org/client/<APP-ID>",
                  "path": "<APP-ID>",
                  "host": "sandbox.orcid.org"
                },
                "source-name": {
                  "value": "Open Journal Systems ORCID Integration Dev"
                },
                "assertion-origin-orcid": null,
                "assertion-origin-client-id": null,
                "assertion-origin-name": null
              },
              "reviewer-role": "reviewer",
              "external-ids": {
                "external-id": [
                  {
                    "external-id-type": "source-work-id",
                    "external-id-value": "<REVIEW-ROUND-ID>",
                    "external-id-normalized": {
                      "value": "<REVIEW-ROUND-ID>",
                      "transient": true
                    },
                    "external-id-normalized-error": null,
                    "external-id-url": null,
                    "external-id-relationship": "part-of"
                  }
                ]
              },
              "review-url": null,
              "review-type": "review",
              "completion-date": {
                "year": {
                  "value": "2025"
                },
                "month": {
                  "value": "01"
                },
                "day": {
                  "value": "17"
                }
              },
              "review-group-id": "issn:<ISSN>",
              "convening-organization": {
                "name": "Public Knowledge Project",
                "address": {
                  "city": "Vancouver",
                  "region": null,
                  "country": "IS"
                },
                "disambiguated-organization": null
              },
              "visibility": "public",
              "put-code": <PUT-CODE>,
              "path": "/<ORCID-ID>/peer-review/<PUT-CODE>",
              "display-index": "1"
            }
          ]
        }
      ]
    }
  ],
  "path": "/<ORCID-ID>/peer-reviews"
}

@taslangraham
Copy link
Contributor Author

taslangraham commented Jan 22, 2025

@ewhanson this looks good. I'll follow up if have any issues or questions while implementing it.

taslangraham added a commit to taslangraham/ojs that referenced this issue Jan 24, 2025
taslangraham added a commit to taslangraham/ojs that referenced this issue Jan 24, 2025
taslangraham added a commit to taslangraham/ojs that referenced this issue Jan 24, 2025
taslangraham added a commit to taslangraham/ojs that referenced this issue Jan 24, 2025
taslangraham added a commit to taslangraham/ojs that referenced this issue Jan 24, 2025
taslangraham added a commit to taslangraham/omp that referenced this issue Jan 24, 2025
taslangraham added a commit to taslangraham/omp that referenced this issue Jan 24, 2025
taslangraham added a commit to taslangraham/ops that referenced this issue Jan 24, 2025
@taslangraham
Copy link
Contributor Author

@ewhanson Ready for a second round of review.

pkp-lib: #10776
ojs : pkp/ojs#4580 ->(specific commit with changes for retrieving old put-codes)
omp: pkp/omp#1799
ops: pkp/ops#836

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

No branches or pull requests

2 participants