There's a template toolkit bug in the condition for BIBLIO_RESULT.ISBN in ./koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-sendshelf.tt so "ISBN:" shows in the email whether or not there is any ISBN data.
Hi David, Just to clarify the issues of bug raised: By "ISBN:" shows in the email whether or not there is any ISBN data." Are you suggesting that, IF a bibliographic record, has NO ISBN FIELD in the marc file? that the output resulted in email form, still displays a blank ISBN label. OR that... IF a bibliographic record, has an empty ISBN FIELD value in the marc file? that the output resulted in email form, still displays a blank ISBN label. The reason I ask, is that in the file: ./koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-sendshelf.tt There is a specific IF statement line 61 - 68 ``` [% IF ( BIBLIO_RESULT.ISBN ) %] <span> ISBN: [% FOREACH isbn IN BIBLIO_RESULT.ISBN %] [% isbn | $raw %] [% UNLESS ( loop.last ) %]; [% END %] [% END %] </span><br/> [% END %] ```
Hi David, Just to clarify the bug raised: ``` "ISBN:" shows in the email whether or not there is any ISBN data." ``` Are you suggesting that, IF a bibliographic record, has NO ISBN FIELD in the marc file? that the output result in email, still displays a blank ISBN label. OR that... IF a bibliographic record, has an empty ISBN FIELD with no value in subfields, in the marc file, that the output result in email, still displays a blank ISBN label. The reason I ask, is that in the file: ./koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-sendshelf.tt There is a specific IF statement line 61 - 68, that will only display the ISBN output IF the marc file itself has an ISBN marc field present (either subfield populated or not). See below ``` [% IF ( BIBLIO_RESULT.ISBN ) %] <span> ISBN: [% FOREACH isbn IN BIBLIO_RESULT.ISBN %] [% isbn | $raw %] [% UNLESS ( loop.last ) %]; [% END %] [% END %] </span><br/> [% END %] ``` I ran a quick test in opac which appears to show the ISBN label, only when the marc record itself has a 020 ISBN field. When I removed 020 ISBN field in test marc record, the ISBN label did not populate in output results. Are you able to create a test plan based on your perspective on this bug? IF so I can test authentically. Thanks.
Hi Adam, Thanks for taking an interest in this one. I'm not sure that I 100% follow your comment. The premise is there is a List with bib records that don't have the 020 MARC field in them. If you view the list in the OPAC, click "Send list", and email the list to yourself, the email should have blank "ISBN:" lines in the email for the records that don't have 020 MARC fields in them. This is because BIBLIO_RESULT.ISBN contains an empty array reference which counts as a "true" value in [% IF ( BIBLIO_RESULT.ISBN ) %]. That IF statement needs to check the size/length of the array reference in the ISBN field of the BIBLIO_RESULT hash. I've already fixed this one locally, so it hasn't been a high priority for me, but I'll attach a patch in a minute.
Created attachment 144366 [details] [review] Bug 32302: Hide "ISBN" label when no ISBN data when sending list There is a logic mistake in the opac-sendshelf.tt template where we test if the "ISBN" key exists in the "BIBLIO_RESULT" hash when we should really be testing if the arrayref returned by "ISBN" is empty or not, since that "ISBN" key always exists and contains at least an empty arrayref. Test plan: 1. Apply patch and restart your Koha Starman 2. Create a public list containing records with 020 MARC fields and records without 020 MARC fields 3. View the list in the OPAC 4. Click "Send list" and email it to yourself 5. Note that only records with 020 MARC fields with show "ISBN:" in the email output
Created attachment 144368 [details] [review] Bug 32302: Hide "ISBN" label when no ISBN data when sending list There is a logic mistake in the opac-sendshelf.tt template where we test if the "ISBN" key exists in the "BIBLIO_RESULT" hash when we should really be testing if the arrayref returned by "ISBN" is empty or not, since that "ISBN" key always exists and contains at least an empty arrayref. Test plan: 1. Apply patch and restart your Koha Starman 2. Create a public list containing records with 020 MARC fields and records without 020 MARC fields 3. View the list in the OPAC 4. Click "Send list" and email it to yourself 5. Note that only records with 020 MARC fields with show "ISBN:" in the email output Signed-off-by: David Nind <david@davidnind.com>
Created attachment 144369 [details] [review] Bug 32302: (follow-up) Hide ISBN label when list sent from the staff interface This makes the same change to the staff interface. Lists sent from the staff interface hide the ISBN label in the email message when there are no ISBNs for a record. Signed-off-by: David Nind <david@davidnind.com>
Hi David. I added a follow-up for lists sent from the staff interface, as they were doing exactly the same thing - I didn't read the test plan correctly, and wondered why it wasn't working from the staff interface. Hope you don't mind! David Testing notes using KTD (koha-testing-docker): 1. Created a list from the search results for 'perl' (11 results, 2 don't have ISBNs, some results have multiple ISBNs). 2. To setup KTD to send email using a Google account: 2.1 Edit your /etc/koha/sites/kohadev/koha-conf.xml file 2.2 Add this configuration near the end: . <user_name> = your Google email address . <password> = an APP password that you need to create in your Google account, not your Google account password <smtp_server> <host>smtp.gmail.com</host> <port>587</port> <timeout>5</timeout> <ssl_mode>STARTTLS</ssl_mode> <user_name>GOOGLEACCOUNTUSER</user_name> <password>GOOGLEAPPPASSWORD</password> <debug>1</debug> </smtp_server>
(In reply to David Nind from comment #7) > Hope you don't mind! Not at all! Thanks for testing and for the follow-up :).
Hi David and David, Firstly, apologies if my questions on your initial bug report were not as coherent as they were in my head lol. I am new to the Koha dev community and will work on my communication skills. Thanks for the patch, I was looking for something to fix and this looked like a quick one, I will look around for others 8-) Thanks again.
(In reply to Adam Styles from comment #9) > Firstly, apologies if my questions on your initial bug report were not as > coherent as they were in my head lol. I am new to the Koha dev community and > will work on my communication skills. No worries at all. I think my initial description was pretty vague! Glad to have you aboard. Always good to see other Australians working on Koha! It's not often I get to share a timezone with another Koha dev ;). > Thanks for the patch, I was looking for something to fix and this looked > like a quick one, I will look around for others 8-) Good instinct. It was a quick one! If you're looking for easy bugs, you might considering doing a Bugzilla search on the keyword "Academy".
Created attachment 144526 [details] [review] Bug 32302: Hide "ISBN" label when no ISBN data when sending list There is a logic mistake in the opac-sendshelf.tt template where we test if the "ISBN" key exists in the "BIBLIO_RESULT" hash when we should really be testing if the arrayref returned by "ISBN" is empty or not, since that "ISBN" key always exists and contains at least an empty arrayref. Test plan: 1. Apply patch and restart your Koha Starman 2. Create a public list containing records with 020 MARC fields and records without 020 MARC fields 3. View the list in the OPAC 4. Click "Send list" and email it to yourself 5. Note that only records with 020 MARC fields with show "ISBN:" in the email output Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 144527 [details] [review] Bug 32302: (follow-up) Hide ISBN label when list sent from the staff interface This makes the same change to the staff interface. Lists sent from the staff interface hide the ISBN label in the email message when there are no ISBNs for a record. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Pushed to master for 23.05. Nice work everyone, thanks!
Nice work everyone! Pushed to 22.11.x for the next release
Backported to 22.05.x for upcoming 22.05.09
applied to 21.11.x for 21.11.16
Not backported to 21.05.x