Syndetics broken cover image retrieval

[Ben Shum]

Evergreen master / 2.6 / 2.5

Since added content by record ID (bug 1065378) was added in Evergreen 2.5, it seems that we occasionally get a broken cover image from Syndetics instead of a blank image. This results in issues where the catalog displays a broken image instead of the empty looking 1x1 pixel cover.

IRC discussions from http://irc.evergreen-ils.org/evergreen/2014-01-22#i_61938 onwards seem to indicate that the problem is bad parsing of the identifiers and Syndetics giving us a broken/invalid page result.

Some of the final conversation noted by jeff at the time:

2014-01-22T16:54:09 <jeff> yeah, okay. i need to get something else done here, but it comes down to Business::ISBN::_common_format not being able to handle '0131500449 (videodisc 2A)' even though it can handle things like '0131500449 (foo)' -- i'll create a bug with some examples of works / doesn't work and throw together a fix.
2014-01-22T16:54:26 <jeff> essentially, pre-clean the isbns based on how we've done it elsewhere / in the past.
2014-01-22T16:55:18 <jeff> and at the same time skip sending the query upstream if we have no identifiers, and "break less" if and when we get an error like this.
2014-01-22T16:57:51 <jeff> and maybe fix up Business::ISBN, though it might simply be that it was never intended to handle what we're throwing at it.

[Dan Scott (denials)]

See public.force_to_isbn13 in Open-ILS/src/sql/Pg/002.schema.config.sql for one approach we already use for dealing with ISBNs like "0131500449 (videodisc 2A)".

Thankfully MARC21 now defines $q for the "(videodisc 2A)" part of ISBNs, so we'll all be fixing up our millions of legacy MARC records accordingly... right?

[Thomas Berezansky (tsbere)]

On a related note, we have noticed that sometimes the ISBN/ISSN/UPC that apparently gets used is inconsistent. I think the branch below will generally ensure that the identifiers are returned in the order they were originally found in the record, so the first one would be the one used for lookups (at least when only one is used).

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/tsbere/addedcontent_order_ids

There may be other/better ways to do that, especially if we are cleaning up ISBNs in other ways first that may be changing more in that area, so I am just throwing this out here for now.

[Jason Stephenson (jstephenson)]

One thing we've found along the way is that certain extra ISBN data can cause Business::ISBN to not produce a usable ISBN object. Specifically, ISBNs with extra numbers or the letter X before or after the ISBN can do this.

Here's a branch with a change to AddedContent.pm that is intended to address this problem. It could possibly be merged with the above branch from Thomas for a more complete fix.

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/dyrcona/added_content_isbn_twiddling

[Jason Stephenson (jstephenson)]

Hmm. Sorry for removing Jeff and putting him back.

I thought that I was going to merge the two branches on this bug and slap a pullrequest on it. I then realized that the two branches don't actually fix the whole problem, but they do seem to help. I wonder if we should make a new bug for added content improvements and merge those branches into one on that bug.

The two branches above seem to help regardless of the content provider.

[Jason Stephenson (jstephenson)]

Ermm... After re-reading the bug description, it does sound like the combined branches will fix this.

So, I did combine them into one branch at working/collab/dyrcona/lp1302207_ac_isbn_fixups

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/dyrcona/lp1302207_ac_isbn_fixups

And, I'll add the pullrequest tag.

[Ben Shum (bshum)]

Sounds good! Picked to master and backported to rel_2_6. Thanks everyone!