[Bug 14345] New: parsed isbn has semi-colon fails to nicely link for idreambooks
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14345 Bug ID: 14345 Summary: parsed isbn has semi-colon fails to nicely link for idreambooks Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: trivial Priority: P5 - low Component: OPAC Assignee: oleonard@myacpl.org Reporter: mtompset@hotmail.com QA Contact: testopia@bugs.koha-community.org While testing Bug 14318, I discovered this semicolon issue. <img src="https://idreambooks.com/remotereadometer/9780812993257;.jpg" alt="The Road to Character by David Brooks" title="Rating based on reviews of The Road to Character"> The isbn logic didn't remove trailing semi-colon. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14345 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=14318 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14345 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |14318 Assignee|oleonard@myacpl.org |mtompset@hotmail.com Status|NEW |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14345 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14345 --- Comment #1 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 39913 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39913&action=edit Bug 14345: broken isbn logic prevents display of idreambooks image TEST PLAN --------- 1) Ensure you have a book that would pull up an idreambooks image (e.g. The road to character / David Brooks.) 2) Ensure your system preferences for idreambook stuff is set. 3) Search for that book in OPAC and go to details. -- There should be a readometer image, but it will be broken. 4) Apply this patch 5) Refresh page -- readometer image appears. YAY! 6) run koha qa test tools -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14345 Nick Clemens <nick@quecheelibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14345 Nick Clemens <nick@quecheelibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #39913|0 |1 is obsolete| | --- Comment #2 from Nick Clemens <nick@quecheelibrary.org> --- Created attachment 40105 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=40105&action=edit Bug 14345: broken isbn logic prevents display of idreambooks image TEST PLAN --------- 1) Ensure you have a book that would pull up an idreambooks image (e.g. The road to character / David Brooks.) 2) Ensure your system preferences for idreambook stuff is set. 3) Search for that book in OPAC and go to details. -- There should be a readometer image, but it will be broken. 4) Apply this patch 5) Refresh page -- readometer image appears. YAY! 6) run koha qa test tools Signed-off-by: Nick Clemens <nick@quecheelibrary.org> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14345 Nick Clemens <nick@quecheelibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@quecheelibrary.org --- Comment #3 from Nick Clemens <nick@quecheelibrary.org> --- Took me a while here, the missing step is "Have an ISBN ending in ; (with no space before the punctuation)" It fixes a problem caused by questionable punctuation in record, but it doesn't hurt anything that I see -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14345 --- Comment #4 from Nick Clemens <nick@quecheelibrary.org> --- (In reply to Nick Clemens from comment #3)
Took me a while here, the missing step is "Have an ISBN ending in ; (with no space before the punctuation)"
It fixes a problem caused by questionable punctuation in record, but it doesn't hurt anything that I see
Ah, forgive me, this actually crops up when you have more than one isbn or an ISBN with no punctuation. It's pulling the isbn from the document so any added punctuation (semicolon between multiples or period if single) is breaking the link. Seems to be affecting both SSL and non-SSL as well -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14345 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@biblibre.co | |m --- Comment #5 from Jonathan Druart <jonathan.druart@biblibre.com> --- Silly question but... Why do you have an isbn ending in ';'? Shouldn't the ibns be sent to the template once sanitized? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14345 --- Comment #6 from M. Tompsett <mtompset@hotmail.com> --- (In reply to Jonathan Druart from comment #5)
Silly question but... Why do you have an isbn ending in ';'? Shouldn't the ibns be sent to the template once sanitized?
Because it is parsed from idreambooks in javascript, not from the perl. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14345 --- Comment #7 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to M. Tompsett from comment #6)
(In reply to Jonathan Druart from comment #5)
Silly question but... Why do you have an isbn ending in ';'? Shouldn't the ibns be sent to the template once sanitized?
Because it is parsed from idreambooks in javascript, not from the perl.
And the first part for the question? :) Just looking at the code, the isbns are retrieved from: 228 [% IF ( MARCISBNS ) %] 229 <span class="results_summary isbn"><span class="label">ISBN:</span> [% FOREACH MARCISBN IN MARCISBNS %] [% MARCISBN %][% IF ( loop.last ) %].[% ELSE %]; [% END %] [% END %] </span> 230 [% ELSE %] 231 [% IF ( normalized_isbn ) %] 232 <span class="results_summary isbn"><span class="label">ISBN: </span>[% normalized_isbn %]</span> 233 [% END %] 234 [% END %] (I added some indentation for the readability). Does the ';' you want to remove is the one generated by [% IF ( loop.last ) %].[% ELSE %]; [% END %]? Because looking at the JS: var isbn = $(".isbn:last").text().split(" ")[1]; It seems that several .isbn is expected, but only one is created. Maybe the template should be more something like: <span class="results_summary isbns"><span class="label">ISBN:</span> [% FOREACH MARCISBN IN MARCISBNS %] <span class="isbn"[% MARCISBN %]</span>[% IF ( loop.last ) %].[% ELSE %]; [% END %] [% END %] </span> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14345 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #40105|0 |1 is obsolete| | --- Comment #8 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 40598 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=40598&action=edit Bug 14345: broken isbn logic prevents display of idreambooks image TEST PLAN --------- 1) Ensure you have a book that would pull up an idreambooks image (e.g. The road to character / David Brooks.) -- I imported from the Library of Congress via z39.50 2) Ensure your system preferences for idreambook stuff is set. 3) Ensure you have multiple 020$a's for your book, and the first one must just be an ISBN number! -- I removed the '(hardcover: acidfree paper)' from the first. -- I took the second 020 field and changed 020$z to 020$a 4) Search for that book in OPAC and go to details. -- There should be a readometer image, but it will be broken. -- Notice the ISBN line has "#############; ..." This could also be attained by just putting two semi-colon separated ISBN's on the first 020$a. 5) Apply this patch 6) Refresh page -- readometer image appears. YAY! 7) run koha qa test tools NOTE: Only changed test plan, so leaving sign-off on. Signed-off-by: Nick Clemens <nick@quecheelibrary.org> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14345 --- Comment #9 from M. Tompsett <mtompset@hotmail.com> --- (In reply to Jonathan Druart from comment #7)
Does the ';' you want to remove is the one generated by [% IF ( loop.last ) %].[% ELSE %]; [% END %]?
Yes, if: 1) there are multiple 020$a's on the biblio record, and the first one is just a number. OR No, if: 2) someone intentionally types multiple ISBN's into 020$a on a biblio record with the first one as a number followed immediately by semi-colon as a delimiter. We have no ISBN validation in the MARC Editor area.
Because looking at the JS: var isbn = $(".isbn:last").text().split(" ")[1]; It seems that several .isbn is expected, but only one is created.
Actually, I was surprised that alert($(".isbn:last").text()) actually generated that entire ISBN line. It's expecting spaces. The [1] is the first ISBN in the list.
Maybe the template should be more something like: <span class="results_summary isbns"><span class="label">ISBN:</span> [% FOREACH MARCISBN IN MARCISBNS %] <span class="isbn"[% MARCISBN %]</span>[% IF ( loop.last ) %].[% ELSE %]; [% END %] [% END %] </span>
In order to make the semantics clearer, perhaps, but if you changed that just change .isbn to .isbns in the JS code section after your suggested change, you'd get the equivalent problem. If we make the template change, we may have other things to test as well. This is the quickest fix that is easily testable. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14345 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14345 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #40598|0 |1 is obsolete| | --- Comment #10 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 40760 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=40760&action=edit Bug 14345: broken isbn logic prevents display of idreambooks image TEST PLAN --------- 1) Ensure you have a book that would pull up an idreambooks image (e.g. The road to character / David Brooks.) -- I imported from the Library of Congress via z39.50 2) Ensure your system preferences for idreambook stuff is set. 3) Ensure you have multiple 020$a's for your book, and the first one must just be an ISBN number! -- I removed the '(hardcover: acidfree paper)' from the first. -- I took the second 020 field and changed 020$z to 020$a 4) Search for that book in OPAC and go to details. -- There should be a readometer image, but it will be broken. -- Notice the ISBN line has "#############; ..." This could also be attained by just putting two semi-colon separated ISBN's on the first 020$a. 5) Apply this patch 6) Refresh page -- readometer image appears. YAY! 7) run koha qa test tools NOTE: Only changed test plan, so leaving sign-off on. Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14345 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com Status|Passed QA |Pushed to Master --- Comment #11 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patch pushed to master. Thanks Mark! -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org