Fix for duplicate ids appearing in connections when events contain uppercase and lowercase source_ids and target_ids

[codycooperross]

Purpose

Addresses issue where uppercase and lowercase ids in an Event's source_id or target_id would duplicate connections in a Doi record, like citations and thus citation_ids, citation_count. See the citations attribute here: https://api.datacite.org/dois/10.17632/579pxjyjz8.1

"citations": {
"data": [
{
"id": "10.32725/jab.2020.004",
"type": "dois"
},
{
"id": "10.32725/jab.2020.004",
"type": "dois"
}
]
},

closes: datacite/datacite#2195

Approach

Adds .compact.map(&:downcase) before .uniq when calculating connections. Slightly refactors citations_over_time method.

Open Questions and Pre-Merge TODOs

Would require a re-index to be reflected on every record in the REST API.

Learning

Types of changes

• Bug fix (non-breaking change which fixes an issue)

• New feature (non-breaking change which adds functionality)

• Breaking change (fix or feature that would cause existing functionality to change)

Reviewer, please remember our guidelines:

• Be humble in the language and feedback you give, ask don't tell.
• Consider using positive language as opposed to neutral when offering feedback. This is to avoid the negative bias that can occur with neutral language appearing negative.
• Offer suggestions on how to improve code e.g. simplification or expanding clarity.
• Ensure you give reasons for the changes you are proposing.

[wendelfabianchinsamy]

@codycooperross the only suggestion I would make is not exactly to do with your change but with the existing code repetition. This happens in a few places but if you look at the method reference_count it's code is exactly the same as reference_ids except for the additional length method invocation. Because we didn't reuse the code we had to make this change in two places.

This is certainly outside the scope of your PR. I will create an issue to clean up this code to make more DRY (don't repeat yourself).

I will approve this PR.

[codycooperross]

Thank you, Wendel!

[codycooperross]

@wendelfabianchinsamy I just added back the pluck because I think the map would have selected the entire Event record and may not have been performant for DOIs with many events. All good?

[sentry-io]

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

• ‼️ **NoMethodError: undefined method downcase' for nil:NilClass** app/models/doi.rb in block in citations_over_time` View Issue
• ‼️ **NoMethodError: undefined method downcase' for nil:NilClass** app/models/doi.rb in block in citations_over_time` View Issue
• ‼️ **NoMethodError: undefined method downcase' for nil:NilClass** DataciteDoisController#show` View Issue
• ‼️ **NoMethodError: undefined method downcase' for nil:NilClass** DataciteDoisController#update` View Issue

_{Did you find this useful? React with a 👍 or 👎}