If you enter a URL in 555$u, you want to click it in Title Notes/Descriptions. This requires a small adjustment in GetMarcNotes.
Created attachment 39737 [details] [review] Bug 14306: Show URL from MARC21 field 555$u under Title Notes/Descriptions This patch includes: [1] Add some logic to GetMarcNotes to embed the contents of MARC21 field 555$u in a html anchor tag. [2] Add a unit test for GetMarcNotes in Biblio.t [3] Remove calls to GetMarcNotes from sendbasket.pl (opac and staff). A closer look revealed that the data was not used; the notes in the mail of sendbasket are taken from GetBiblioData. Test plan: [1] Edit a record. Add one or two URLS in 555$u. Add something in 500$a too. [2] Check if you can click the URLs in opac and staff detail tab Notes or Descriptions. [3] Run the unit test t/db../Biblio.t [4] Add something in the cart. Click More Details and send the cart. Verify that you have something in Notes (from 500$a).
Created attachment 39741 [details] [review] [Signed-off] Bug 14306: Show URL from MARC21 field 555$u under Title Notes/Descriptions This patch includes: [1] Add some logic to GetMarcNotes to embed the contents of MARC21 field 555$u in a html anchor tag. [2] Add a unit test for GetMarcNotes in Biblio.t [3] Remove calls to GetMarcNotes from sendbasket.pl (opac and staff). A closer look revealed that the data was not used; the notes in the mail of sendbasket are taken from GetBiblioData. Test plan: [1] Edit a record. Add one or two URLS in 555$u. Add something in 500$a too. [2] Check if you can click the URLs in opac and staff detail tab Notes or Descriptions. [3] Run the unit test t/db../Biblio.t [4] Add something in the cart. Click More Details and send the cart. Verify that you have something in Notes (from 500$a). Followed test plan. Works as expected. QA tools OK. Signed-off-by: Marc Véron <veron@veron.ch>
(In reply to Marc Véron from comment #2) > Signed-off-by: Marc Véron <veron@veron.ch> Thanks, Marc.
I feel like it's a bit backwards to add HTML output to a core function. Maybe this would be better handled on template side? There has also been a request to add a span with the field number somewhere, that would allow to hide some notes fields for format them in an easy way.
(In reply to Katrin Fischer from comment #4) > I feel like it's a bit backwards to add HTML output to a core function. > Maybe this would be better handled on template side? There has also been a > request to add a span with the field number somewhere, that would allow to > hide some notes fields for format them in an easy way. Thanks for your feedback. In the current form it is a small fix. Now we pass just an array of notes to the template. We probably do not want to parse the marc record in the template. So that would mean: passing the URLs separately, maybe put a marker somewhere (or changing behavior still more), etc. I did not have/take the time for now to put it in a much larger perspective..
I think passing a 'url' marker and also the field name to the template here would be better. There is an option to mark fields as URLs in the templates, it would make sense to use this, but I would not oppose hardcoding 555 in that case.
Created attachment 41237 [details] [review] Bug 14306: Show URL from MARC21 field 555$u under Title Notes/Descriptions This patch includes: [1] Add some logic to GetMarcNotes to embed the contents of MARC21 field 555$u in a html anchor tag. [2] Add a unit test for GetMarcNotes in Biblio.t [3] Remove calls to GetMarcNotes from sendbasket.pl (opac and staff). A closer look revealed that the data was not used; the notes in the mail of sendbasket are taken from GetBiblioData. Test plan: [1] Edit a record. Add one or two URLS in 555$u. Add something in 500$a too. [2] Check if you can click the URLs in opac and staff detail tab Notes or Descriptions. [3] Run the unit test t/db../Biblio.t [4] Add something in the cart. Click More Details and send the cart. Verify that you have something in Notes (from 500$a).
What is this second patch, there is no signoff on it? I don't think generating html in the module is a good idea. But it's certainly the quickest fix and will avoid to copy/paste code in the different templates.
(In reply to Jonathan Druart from comment #8) > What is this second patch, there is no signoff on it? Hm. I rebased the patch but forgot to take the one with the signoff.. I will resubmit. > I don't think generating html in the module is a good idea. But it's > certainly the quickest fix and will avoid to copy/paste code in the > different templates. I will still have a look for a simple follow-up.
Created attachment 41987 [details] [review] Bug 14306: Show URL from MARC21 field 555$u under Title Notes/Descriptions This patch includes: [1] Add some logic to GetMarcNotes to embed the contents of MARC21 field 555$u in a html anchor tag. [2] Add a unit test for GetMarcNotes in Biblio.t [3] Remove calls to GetMarcNotes from sendbasket.pl (opac and staff). A closer look revealed that the data was not used; the notes in the mail of sendbasket are taken from GetBiblioData. Test plan: [1] Edit a record. Add one or two URLS in 555$u. Add something in 500$a too. [2] Check if you can click the URLs in opac and staff detail tab Notes or Descriptions. [3] Run the unit test t/db../Biblio.t [4] Add something in the cart. Click More Details and send the cart. Verify that you have something in Notes (from 500$a). Signed-off-by: Marc Veron <veron@veron.ch> Followed test plan. Works as expected. QA tools OK.
Created attachment 41995 [details] [review] Bug 14306: Follow-up for URLs in 555$u This patch removes the code for inserting the <a> anchor tags around URLs in GetMarcNotes (as added originally). The URLs are placed in separate array elements; the template should take care of further handling. The unit test has been adjusted accordingly. Test plan: Run the unit test.
Still a patch coming for one/more templates..
Created attachment 42010 [details] [review] Bug 14306: Show URL from MARC21 field 555$u under Title Notes/Descriptions This patch includes: [1] Add some logic to GetMarcNotes to embed the contents of MARC21 field 555$u in a html anchor tag. [2] Add a unit test for GetMarcNotes in Biblio.t [3] Remove calls to GetMarcNotes from sendbasket.pl (opac and staff). A closer look revealed that the data was not used; the notes in the mail of sendbasket are taken from GetBiblioData. Test plan: [1] Edit a record. Add one or two URLS in 555$u. Add something in 500$a too. [2] Check if you can click the URLs in opac and staff detail tab Notes or Descriptions. [3] Run the unit test t/db../Biblio.t [4] Add something in the cart. Click More Details and send the cart. Verify that you have something in Notes (from 500$a). Signed-off-by: Marc Veron <veron@veron.ch> Followed test plan. Works as expected. QA tools OK.
Created attachment 42011 [details] [review] Bug 14306: Follow-up for URLs in 555$u This patch removes the code for inserting the <a> anchor tags around URLs in GetMarcNotes (as added originally). The URLs are placed in separate array elements; the template should take care of further handling. The unit test has been adjusted accordingly. Test plan: Run the unit test.
Created attachment 42012 [details] [review] Bug 14306: Changes for basket and detail templates A simple regex is added to the basket and detail templates to select the URLs passed separately from MARC21 555$u by GetMarcNotes. Note that the regex tests if a note starts with http:// or https:// and does not contain any whitespace in order to be considered as a url. These URLs are put in an anchor tag. This touches four places: [1] opac detail, tab title notes [2] catalogue detail, tab Descriptions [3] opac basket, more details, notes [4] staff basket, more details, notes Test plan: [1] Edit a record. Add a 500$a, 555$a and a URL in 555$u. Put "http://this is not a url" in the 500$a (whitespace!). [2] Check opac-detail, tab Title Notes. Check the URL. [3] Do the same for catalogue/detail. [4] Add record to cart in OPAC. Open basket. Check More details. [5] Repeat previous step in staff.
Created attachment 42013 [details] [review] Bug 14306: Remove call to GetMarcNotes from sendshelf Just as mentioned in the first patch for sendbasket, I discovered that the call of GetMarcNotes is not really used in sendshelf. The array is passed to the template, but the template does not use it. (It uses the information from GetBiblioData.) Test plan: [1] Send a list that includes a record with some notes (opac and staff).
CONFLICT (content): Merge conflict in virtualshelves/sendshelf.pl
Created attachment 46221 [details] [review] Bug 14306: Remove call to GetMarcNotes from sendshelf Just as mentioned in the first patch for sendbasket, I discovered that the call of GetMarcNotes is not really used in sendshelf. The array is passed to the template, but the template does not use it. (It uses the information from GetBiblioData.) Test plan: [1] Send a list that includes a record with some notes (opac and staff).
Rebased. Who adds a signoff?
Hi Marcel, a question about the test plan: Do you expect the values (500$a, 555$a and a URL in 555$u) to be inserted into the mails? Or should we test only that the mails still work?
(In reply to Marc Véron from comment #20) > Hi Marcel, a question about the test plan: > Do you expect the values (500$a, 555$a and a URL in 555$u) to be inserted > into the mails? Or should we test only that the mails still work? Hi Marc, You may check the mails for unexpected side-effects or so. The main changes in this patch are template changes. Please check the tabs mentioned. The 555 field is not passed in the mail IIRC (the code there did not use GetMarcNotes at all).
Created attachment 46251 [details] [review] Bug 14306: Show URL from MARC21 field 555$u under Title Notes/Descriptions This patch includes: [1] Add some logic to GetMarcNotes to embed the contents of MARC21 field 555$u in a html anchor tag. [2] Add a unit test for GetMarcNotes in Biblio.t [3] Remove calls to GetMarcNotes from sendbasket.pl (opac and staff). A closer look revealed that the data was not used; the notes in the mail of sendbasket are taken from GetBiblioData. Test plan: [1] Edit a record. Add one or two URLS in 555$u. Add something in 500$a too. [2] Check if you can click the URLs in opac and staff detail tab Notes or Descriptions. [3] Run the unit test t/db../Biblio.t [4] Add something in the cart. Click More Details and send the cart. Verify that you have something in Notes (from 500$a). Signed-off-by: Marc Veron <veron@veron.ch> Followed test plan. Works as expected. QA tools OK. Tested with all patches together, works as expected Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 46252 [details] [review] Bug 14306: Follow-up for URLs in 555$u This patch removes the code for inserting the <a> anchor tags around URLs in GetMarcNotes (as added originally). The URLs are placed in separate array elements; the template should take care of further handling. The unit test has been adjusted accordingly. Test plan: Run the unit test. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 46253 [details] [review] Bug 14306: Changes for basket and detail templates A simple regex is added to the basket and detail templates to select the URLs passed separately from MARC21 555$u by GetMarcNotes. Note that the regex tests if a note starts with http:// or https:// and does not contain any whitespace in order to be considered as a url. These URLs are put in an anchor tag. This touches four places: [1] opac detail, tab title notes [2] catalogue detail, tab Descriptions [3] opac basket, more details, notes [4] staff basket, more details, notes Test plan: [1] Edit a record. Add a 500$a, 555$a and a URL in 555$u. Put "http://this is not a url" in the 500$a (whitespace!). [2] Check opac-detail, tab Title Notes. Check the URL. [3] Do the same for catalogue/detail. [4] Add record to cart in OPAC. Open basket. Check More details. [5] Repeat previous step in staff. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 46254 [details] [review] Bug 14306: Remove call to GetMarcNotes from sendshelf Just as mentioned in the first patch for sendbasket, I discovered that the call of GetMarcNotes is not really used in sendshelf. The array is passed to the template, but the template does not use it. (It uses the information from GetBiblioData.) Test plan: [1] Send a list that includes a record with some notes (opac and staff). Signed-off-by: Marc Véron <veron@veron.ch>
(In reply to Marc Véron from comment #25) > Signed-off-by: Marc Véron <veron@veron.ch> Thanks, Marc
Created attachment 48681 [details] [review] [PASSED QA] Bug 14306: Show URL from MARC21 field 555$u under Title Notes/Descriptions This patch includes: [1] Add some logic to GetMarcNotes to embed the contents of MARC21 field 555$u in a html anchor tag. [2] Add a unit test for GetMarcNotes in Biblio.t [3] Remove calls to GetMarcNotes from sendbasket.pl (opac and staff). A closer look revealed that the data was not used; the notes in the mail of sendbasket are taken from GetBiblioData. Test plan: [1] Edit a record. Add one or two URLS in 555$u. Add something in 500$a too. [2] Check if you can click the URLs in opac and staff detail tab Notes or Descriptions. [3] Run the unit test t/db../Biblio.t [4] Add something in the cart. Click More Details and send the cart. Verify that you have something in Notes (from 500$a). Signed-off-by: Marc Veron <veron@veron.ch> Followed test plan. Works as expected. QA tools OK. Tested with all patches together, works as expected Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 48682 [details] [review] [PASSED QA] Bug 14306: Follow-up for URLs in 555$u This patch removes the code for inserting the <a> anchor tags around URLs in GetMarcNotes (as added originally). The URLs are placed in separate array elements; the template should take care of further handling. The unit test has been adjusted accordingly. Test plan: Run the unit test. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 48683 [details] [review] [PASSED QA] Bug 14306: Changes for basket and detail templates A simple regex is added to the basket and detail templates to select the URLs passed separately from MARC21 555$u by GetMarcNotes. Note that the regex tests if a note starts with http:// or https:// and does not contain any whitespace in order to be considered as a url. These URLs are put in an anchor tag. This touches four places: [1] opac detail, tab title notes [2] catalogue detail, tab Descriptions [3] opac basket, more details, notes [4] staff basket, more details, notes Test plan: [1] Edit a record. Add a 500$a, 555$a and a URL in 555$u. Put "http://this is not a url" in the 500$a (whitespace!). [2] Check opac-detail, tab Title Notes. Check the URL. [3] Do the same for catalogue/detail. [4] Add record to cart in OPAC. Open basket. Check More details. [5] Repeat previous step in staff. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 48684 [details] [review] [PASSED QA] Bug 14306: Remove call to GetMarcNotes from sendshelf Just as mentioned in the first patch for sendbasket, I discovered that the call of GetMarcNotes is not really used in sendshelf. The array is passed to the template, but the template does not use it. (It uses the information from GetBiblioData.) Test plan: [1] Send a list that includes a record with some notes (opac and staff). Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
(In reply to Katrin Fischer from comment #30) > Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Great. Thx
Pushed to Master - Should be in the May 2016 - release. Thanks!