Bug 6129

Summary:	ISSN url param for serialssolutions.com
Product:	Koha	Reporter:	Srdjan Jankovic <srdjan>
Component:	OPAC	Assignee:	Srdjan Jankovic <srdjan>
Status:	CLOSED WONTFIX	QA Contact:	Bugs List <koha-bugs>
Severity:	enhancement
Priority:	PATCH-Sent (DO NOT USE)	CC:	chris, gmcharlt, jcamins, koha.sekjal, robin, veron
Version:	Main
Hardware:	All
OS:	All
Change sponsored?:	Sponsored	Patch complexity:	---
Documentation contact:		Documentation submission:
Text to go in the release notes:		Version(s) released in:
Circulation function:
Attachments:	patch Bug 6129 - [master] this adds an ISSN to serialssolutions URLs Bug 6129 - [3.4.x] this adds an ISSN to serialssolutions URLs Bug 6129 - [3.4.x] this adds an ISSN to serialssolutions URLs patch

Description Srdjan Jankovic 2011-04-08 03:28:40 UTC

To direct patrons to 360Link page, You either need to make a different URL for all their holdings pages, something like:                                               
http://zz8kv2kj8h.search.serialssolutions.com/?sid=&ISSN=<insert journal ISSN here>"

We will detect serialssolutions url, and if ISSN url arg is missing, we will add it.

Comment 1 Srdjan Jankovic 2011-04-08 03:39:15 UTC Comment hidden (obsolete)

Created attachment 3819 [details] [review]
patch

Comment 2 Robin Sheat 2011-09-01 07:00:47 UTC Comment hidden (obsolete)

Created attachment 5245 [details] [review]
Bug 6129 - [master] this adds an ISSN to serialssolutions URLs

If a URL is a serialssolutions URLs that doesn't already have an ISSN
parameter, this adds one on the OPAC.

Author: Srdjan Jankovic <srdjan@catalyst.net.nz>
Author: Robin Sheat <robin@catalyst.net.nz>

Comment 3 Robin Sheat 2011-09-01 07:01:24 UTC Comment hidden (obsolete)

Created attachment 5246 [details] [review]
Bug 6129 - [3.4.x] this adds an ISSN to serialssolutions URLs

If a URL is a serialssolutions URLs that doesn't already have an ISSN
parameter, this adds one on the OPAC.

Author: Srdjan Jankovic <srdjan@catalyst.net.nz>
Author: Robin Sheat <robin@catalyst.net.nz>

Comment 4 Chris Nighswonger 2011-09-02 19:35:14 UTC

Comment on attachment 5246 [details] [review]
Bug 6129 - [3.4.x] this adds an ISSN to serialssolutions URLs

This patch does not apply cleanly to 3.4.x. Please rebase and resubmit.

Comment 5 Robin Sheat 2011-09-05 00:21:36 UTC Comment hidden (obsolete)

Created attachment 5304 [details] [review]
Bug 6129 - [3.4.x] this adds an ISSN to serialssolutions URLs

If a URL is a serialssolutions URLs that doesn't already have an ISSN
parameter, this adds one on the OPAC.

Author: Srdjan Jankovic <srdjan@catalyst.net.nz>
Author: Robin Sheat <robin@catalyst.net.nz>

Comment 6 Robin Sheat 2011-09-05 00:24:15 UTC

This patch fixes the 3.4.x conflict. Thought I'd fixed that the first time, apparently not.

Comment 7 Paul Poulain 2011-10-24 11:38:16 UTC

Updating Version : This ENH will be for Koha 3.8

Comment 8 Paul Poulain 2011-10-25 15:05:51 UTC

Bug versionned for master. entries will be made against rel_3_8 once the patch has been applied (see thread about that on koha-devel yesterday)

Comment 9 Chris Nighswonger 2011-10-26 18:02:22 UTC

The fix for this bug was published in the 3.4.5 release. If you were the reporter of this bug, please take time to verify the fix and update the status of this bug report accordingly. If the bug is fixed to your satisfaction, please close this report.

Comment 10 Robin Sheat 2011-10-26 21:50:15 UTC

Chris N, Paul: this has been put into 3.4.5 and is lined up for 3.8 ... this means that it'll skip 3.6, is that a good thing to do?

Comment 11 Katrin Fischer 2011-12-21 06:13:06 UTC

Patch for master does not apply:

Apply? [y]es/[n]o/[e]dit/[v]iew patch/[a]ccept all y
Applying: Bug 6129 - [master] this adds an ISSN to serialssolutions URLs
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging C4/Biblio.pm
Auto-merging catalogue/detail.pl
CONFLICT (content): Merge conflict in catalogue/detail.pl
Auto-merging koha-tmpl/opac-tmpl/prog/en/xslt/MARC21slim2OPACDetail.xsl
Auto-merging opac/opac-detail.pl
CONFLICT (content): Merge conflict in opac/opac-detail.pl
Failed to merge in the changes.
Patch failed at 0001 Bug 6129 - [master] this adds an ISSN to serialssolutions URLs
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".

I am not sure if we should build in solutions for single vendors like this. Could there be a way to make this more flexible?

Comment 12 Chris Cormack 2011-12-21 17:13:23 UTC

(In reply to comment #11)
> Patch for master does not apply:
> 

> 
> I am not sure if we should build in solutions for single vendors like this.
> Could there be a way to make this more flexible?

There could be, but that should definitely not stop this going in, this only adds the issn parameter to a serialsolutions url if one exists in the data already (and doesn't have the issn parameter set).

Also it is in 3.4.x ... so not putting it in 3.6.x and master means we are conciously causing a regression.

Comment 13 Katrin Fischer 2011-12-21 17:43:45 UTC

That makes sense to me.

I was wondering about it because I was trying to use a web service for serials last week that requires to have an url with the issn. We don't have the issn available in the SearchForTitleIn system preference and it's a bit hard to get it with jQuery. But this can all go on a separate enhancement request :)

Comment 14 Srdjan Jankovic 2011-12-22 01:37:19 UTC

Created attachment 6916 [details] [review]
patch

Comment 15 Ian Walls 2012-02-03 13:06:12 UTC

Hardcoding for one specific service (Serials Solutions) strikes me as a very hacky and unsustainable way of fixing this problem.

This kind of processing should be put into a plugin file or some other kind of external means, rather than into C4/Biblio.pm directly.

Comment 16 Jared Camins-Esakov 2012-02-10 19:37:53 UTC

Wouldn't it be better to add ISSN for SearchForTitleIn?

Comment 17 Jared Camins-Esakov 2012-02-10 19:38:57 UTC

Actually, never mind. This is because serialssolutions apparently provides records with URLs that are missing the ISSN, not for adding serialssolutions URLs to records that don't have that 856 yet.

Comment 18 Galen Charlton 2013-07-03 20:12:24 UTC

(In reply to Jared Camins-Esakov from comment #17)
> Actually, never mind. This is because serialssolutions apparently provides
> records with URLs that are missing the ISSN, not for adding serialssolutions
> URLs to records that don't have that 856 yet.

Question for the audience ... is Serials Solutions still distributing records with (apparently) incomplete URLs?

If so, I think a better implementation of the fixup nowadays would be as a TT plugin.

Comment 19 Marc Véron 2016-10-03 15:35:49 UTC

Still valid?

Comment 20 Katrin Fischer 2023-08-04 20:28:53 UTC

(In reply to Galen Charlton from comment #18)
> (In reply to Jared Camins-Esakov from comment #17)
> > Actually, never mind. This is because serialssolutions apparently provides
> > records with URLs that are missing the ISSN, not for adding serialssolutions
> > URLs to records that don't have that 856 yet.
> 
> Question for the audience ... is Serials Solutions still distributing
> records with (apparently) incomplete URLs?
> 
> If so, I think a better implementation of the fixup nowadays would be as a
> TT plugin.

Closing this now more than 7 years after last comment. Please reopen if you disagree and can confirm that this is still an issue for libraries.