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
[VUFIND-1597] include available ISSNs in OpenURLs for Serial format #2713
[VUFIND-1597] include available ISSNs in OpenURLs for Serial format #2713
Conversation
44baa16
to
55ec486
Compare
Noting that the approach I took hews closest to the specific problem I ran into, but an alternative approach would be to simply move the test of |
Thanks, @gmcharlt! I've put a link to the JIRA ticket in the header here, and I've also commented with some more background/context over there. I think some more discussion may be necessary here. :-) |
4e26483
to
632336b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @gmcharlt, I think this is moving in the right direction, but I had one more thought...
This patch adjusts the algorithm for selecting which OpenURL format to use for a given record. It addresses cases where the record format ends up as Serial due to vagaries of the cataloging of the original MARC record but that should nonetheless have any available ISSNs included in their OpenURL. It also attempts to avoid sending both ISBN and ISSN for titles that are part of monographic series. Signed-off-by: Galen Charlton <gmc@equinoxOLI.org>
632336b
to
0a9730e
Compare
@demiankatz I've updated the PR with the requested change. This is on track to become a very thoroughly documented little function. :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @gmcharlt, I think this all makes a lot of sense. However, before I merge, I'm going to ask for a couple more reviews just in case somebody else thinks of something I'm overlooking!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution @gmcharlt - I have no objections and double checked possible implications with a colleague of mine: this change will very likely improve resolver's handling of proceedings at least in our catalogues so we welcome this pull request.
Records whose format ends up as Serial due to vagaries of the cataloging of the original MARC record should nonetheless have any available ISSNs included in their OpenURL. With this patch, the OpenURL format is set as 'Journal' for such records.
TODO