Bug 13364

Summary: Add a call number browser to add item page
Product: Koha Reporter: simith.doliveira
Component: CatalogingAssignee: simith.doliveira
Status: CLOSED FIXED QA Contact: Jonathan Druart <jonathan.druart>
Severity: enhancement    
Priority: P3 CC: brendan, eric.begin, jonathan.druart, katrin.fischer, kyle, m.de.rooy, magnus, marjorie.barry-vila, nengard, nick, oleonard, paola.rossi, philippe.blouin, tomascohen, veron
Version: master   
Hardware: All   
OS: All   
Change sponsored?: Sponsored Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 14737, 16577, 26605    
Attachments: Add a call number browser in item edition page
fix patch - search call number does not work
fix patch - searching call number < MIN or > MAX
sub plugin_javascript adjust
Bug 13364 - Add a call number browser in item edition page
Bug 13364 - fix patch - search call number does not work
Bug 13364 - fix patch - searching call number < MIN or > MAX
Bug 13364 - sub plugin_javascript adjust
[PASSED QA] Create a link to the last advanced search in search result page (OPAC)
[PASSED QA] Fix Patch- Search boxes disappear when returning to advanced search
[PASSED QA] Fix jquery.deseriable.min.js
[PASSED QA] Bug 13307 [QA Followup]
Bug 13364 - QA Followup
Bug 13364 - fix tablesorter and squash
Bug 13364 - fix tablesorter and squash
Bug 13364 - fix tablesorter and squash
[Signed-off] Bug 13364 - Add a call number browser in item edition page
fix datatables.inc
Bug 13364 - Add a call number browser in item edition page
Bug 13364 - Add a call number browser in item edition page
Bug 13364 - Add a call number browser in item edition page
Bug 13364 - Add a call number browser in item edition page
Bug 13364 - Add a call number browser in item edition page
Bug 13364 - fix datatables.inc
Bug 13364 - Add a call number browser in item edition page - QA fixed
Bug 13364 - Add a call number browser in item edition page - QA fixed II
[Signed-off] Bug 13364 - Add a call number browser in item edition page
[Signed-off] Bug 13364 - fix datatables.inc
[Signed-off] Bug 13364 - Add a call number browser in item edition page - QA fixed
[Signed-off] Bug 13364 - Add a call number browser in item edition page - QA fixed II
Bug 13364 - Add a call number browser in item edition page
Bug 13364: Fix table id + software error
Bug 13364: Remove unused JS functions
Bug 13364: Use SQL placeholder
Bug 13364: (QA followup) <body> missing id and class

Description simith.doliveira 2014-12-01 13:24:19 UTC
When editing an item, the call number browser search for existing call number. If it shows a line is green between two call numbers, the call numbers searched is available. If the call number is highlighted in red, it means that it already exists. This can be used to avoid repeated  call numbers in your collection.

The call number browser also shows its position relative to the rest of the collection.
Comment 1 simith.doliveira 2014-12-01 13:26:34 UTC
Sponsored-by: CCSR ( http://www.ccsr.qc.ca )
Comment 2 simith.doliveira 2014-12-01 14:19:15 UTC Comment hidden (obsolete)
Comment 3 simith.doliveira 2014-12-01 14:19:55 UTC
To test :

1) Go to MARC bibliographic framework
    Home › Administration › MARC frameworks
2) Click in MARC structure (Default framework)
3) Search field 952 and click in subfields
4) Edit subfield o (Full call number)
5) Click Display more constraints
6) In Others Options, Plugin, Choose callnumber.pl and save changes
7) Search a record and edit its items
Validate : three points (…) beside Full call number input
8)  Click in …
Validate : if it can not find the given call number, a green line is shown. Otherwise,  the given call number is highlighted in red.
Comment 4 Paola Rossi 2014-12-02 11:12:03 UTC
I've applied the patch.

Everything is OK until the 7-th step [included].

At 8) step, nothing happened.

When my mouse is over the three points, I saw "Tag editor".

I was on editing an already existent Y item for the X biblio:
koha/cataloguing/additem.pl?op=edititem&biblionumber=X&itemnumber=Y#edititem&searchid=scs_1417518371030

So I pass the patch to "Failed QA" status.
Comment 5 simith.doliveira 2014-12-03 14:16:41 UTC
Sorry Paola my bad,
in 6) Choose cn_browser.pl not callnumber.pl as I wrote.
Comment 6 Paola Rossi 2014-12-03 17:02:19 UTC
I've applied the patch (head 12123).

Simith says:
>in 6) Choose cn_browser.pl not callnumber.pl as I wrote.

Thanks! So the "Call Numbers browser" pop-up has been opened as required.

Choosing a full call number < MIN [or choosing a full call number > MAX], I saw no green row at the top of the table [or at the bottom of the table].

I think this is an error.
So I pass the patch to "Failed QA" status.

NB1. I've searched a value by the "Search call number:" input field, selecting "Search". I saw the error:

An error has occurred!
Error 404

    This error means that the link was broken and that the page doesn't exist
    To report this error, you can email the Koha administrator.
    Use top menu bar to navigate to another part of Koha.

NB2. In the Call numbers table :
10 has been listed before 2
10 has been listed before 1.
Comment 7 simith.doliveira 2014-12-03 19:31:11 UTC Comment hidden (obsolete)
Comment 8 simith.doliveira 2014-12-03 20:43:02 UTC Comment hidden (obsolete)
Comment 9 simith.doliveira 2014-12-03 20:48:41 UTC
Both problems (comment 6) should be corrected with the patches.
Comment 10 Marcel de Rooy 2014-12-04 07:52:22 UTC
Preliminary QA comment:

function Clic$function_name(i) {
    q = \$('[id^= \"tag_952_subfield_o\"]').val();
    window.open(\"../cataloguing/plugin_launcher.pl?plugin_name=cn_browser.pl&popup&q=\"+q,\"cnbrowser\",\"width=500,height=400,toolbar=false,scrollbars=yes\");

Please do not do it like that. Now you hardcoded the field while it is passed to you in a parameter. Also normally we pass index and value to the plugin in the URL. If you include your parameter i in the URL with something like index=i you can do this:
    var q=\$('#'+i).val();
    window.open(... index="+i+"& ... )

And if you pass index via sub plugin to the template too, you can use it there if needed also. (In a popup you would use it when putting back the value in the field.)

Also I am not sure what you are doing with the 'popup' parameter. You put it in the URL without a value. Later you are checking the value of popup. This is actually useless?
Comment 11 simith.doliveira 2014-12-16 20:14:08 UTC Comment hidden (obsolete)
Comment 12 simith.doliveira 2014-12-16 20:15:04 UTC
function function Clic$function_name(i) changed
Comment 13 Paola Rossi 2014-12-19 14:04:20 UTC Comment hidden (obsolete)
Comment 14 Paola Rossi 2014-12-19 14:05:19 UTC Comment hidden (obsolete)
Comment 15 Paola Rossi 2014-12-19 14:06:10 UTC Comment hidden (obsolete)
Comment 16 Paola Rossi 2014-12-19 14:06:52 UTC Comment hidden (obsolete)
Comment 17 Kyle M Hall 2014-12-19 17:06:46 UTC Comment hidden (obsolete)
Comment 18 Kyle M Hall 2014-12-19 17:06:54 UTC Comment hidden (obsolete)
Comment 19 Kyle M Hall 2014-12-19 17:06:57 UTC Comment hidden (obsolete)
Comment 20 Kyle M Hall 2014-12-19 17:07:00 UTC Comment hidden (obsolete)
Comment 21 Kyle M Hall 2015-01-09 13:40:54 UTC Comment hidden (obsolete)
Comment 22 Jonathan Druart 2015-01-20 16:25:03 UTC
Kyle, Does this pass qa?
Comment 23 Jonathan Druart 2015-01-20 16:27:29 UTC
Note: The tablesorter jquery plugin has been removed from the src.
Does not it possible to do the same thing using DataTables?

+<table style="margin: auto;width:90%;" id="course_reserves">
=> This is wrong
Comment 24 Katrin Fischer 2015-01-20 19:25:31 UTC
Hm, well spotted.
Comment 25 Katrin Fischer 2015-01-20 19:31:18 UTC
I asked Owen on IRC and he confirmed that we have removed the tablesorter plugin - I think we should avoid adding it back and having different implementations.
Comment 26 simith.doliveira 2015-01-26 18:57:51 UTC Comment hidden (obsolete)
Comment 27 Jonathan Druart 2015-02-26 15:36:38 UTC Comment hidden (obsolete)
Comment 28 Marc Véron 2015-02-26 20:20:05 UTC
I followed the test plan (comment #26)

Selected callnumber.pl in step #6 -> did not work

Selected cn_browser.pl in step #6 -> worked, shows Call Numbers browser

Questions & remarks:

1)
Is callnumber.pl obsolete?

2)
Breadcrumb navigation and language selection appear in "Call number browser": I think this is not necessary in a popup window.

3)
Capitalization of title? (Should be "Call numbers browser" instead of "Call Numbers browser")
Comment 29 simith.doliveira 2015-02-27 13:11:52 UTC Comment hidden (obsolete)
Comment 30 simith.doliveira 2015-02-27 13:19:02 UTC
Corrections :
1) Breadcrumb navigation and language selection removed;

2) Title changed ("Call numbers browser" instead of "Call Numbers browser"); 

3) Test texte correction : 6) cn_browser.pl instead of callnumber.pl
Comment 31 Marc Véron 2015-03-01 17:07:41 UTC Comment hidden (obsolete)
Comment 32 Brendan Gallagher 2015-03-03 10:39:50 UTC
 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/tools/cn_browser.tt
   FAIL	  forbidden patterns
		forbidden pattern: To include datatables files, please use the include file (see bug 10868) (line 12)
   OK	  tt_valid
   OK	  valid_template
Comment 33 simith.doliveira 2015-03-03 22:01:26 UTC Comment hidden (obsolete)
Comment 34 simith.doliveira 2015-03-03 22:03:12 UTC
QA corrected
Comment 35 Magnus Enger 2015-03-05 13:43:04 UTC Comment hidden (obsolete)
Comment 36 Magnus Enger 2015-03-05 13:43:41 UTC Comment hidden (obsolete)
Comment 37 Magnus Enger 2015-03-05 13:44:17 UTC Comment hidden (obsolete)
Comment 38 Magnus Enger 2015-03-05 13:44:57 UTC Comment hidden (obsolete)
Comment 39 Magnus Enger 2015-03-05 13:45:37 UTC Comment hidden (obsolete)
Comment 40 Magnus Enger 2015-03-05 13:50:55 UTC Comment hidden (obsolete)
Comment 41 Magnus Enger 2015-03-05 13:52:26 UTC
(Sorry about the spam with the first patch! Git bz was timing out and I didn't realize it was actually attaching the first patch...)
Comment 42 Jonathan Druart 2015-03-05 15:49:54 UTC
Comment on attachment 36600 [details] [review]
Bug 13364 - Add a call number browser in item edition page

Review of attachment 36600 [details] [review]:
-----------------------------------------------------------------

Is there a good reason to have the template file in the tools directory, instead of cataloguing/value_builder?

The green line is always at the top of the table. If I am remember correctly, in the previous version it was inserted correctly.

::: cataloguing/value_builder/cn_browser.pl
@@ +1,4 @@
> +#!/usr/bin/perl
> +
> +use strict;
> +use warnings;

Prefer use Modern::Perl;

@@ +9,5 @@
> +use C4::Output;
> +use POSIX;
> +use C4::ClassSource;
> +use URI::Escape;
> +use Data::Dumper;

No needed.

::: koha-tmpl/intranet-tmpl/prog/en/modules/tools/cn_browser.tt
@@ +12,5 @@
> +<script type="text/javascript" src="[% interface %]/lib/jquery/plugins/jquery.dataTables.min.js"></script>
> +<script language="JavaScript">
> +$(document).ready(function()
> +{
> +    $("#course_reserves").DataTable({"paging":   false, "bFilter": false, "info":     false});

Hum? Why did you choose this id?
Comment 43 simith.doliveira 2015-03-06 19:23:47 UTC Comment hidden (obsolete)
Comment 44 simith.doliveira 2015-03-06 19:32:31 UTC
Concerning #42 :

Is there a good reason to have the template file in the tools directory, instead of cataloguing/value_builder?

- No, Moved.

The green line is always at the top of the table. If I am remember correctly, in the previous version it was inserted correctly.

- Bug fixed


::: cataloguing/value_builder/cn_browser.pl
@@ +1,4 @@
> +#!/usr/bin/perl
> +
> +use strict;
> +use warnings;

Prefer use Modern::Perl;

- Fixed

@@ +9,5 @@
> +use C4::Output;
> +use POSIX;
> +use C4::ClassSource;
> +use URI::Escape;
> +use Data::Dumper;

No needed.

- Fixed. use C4::ClassSource needed

::: koha-tmpl/intranet-tmpl/prog/en/modules/tools/cn_browser.tt
@@ +12,5 @@
> +<script type="text/javascript" src="[% interface %]/lib/jquery/plugins/jquery.dataTables.min.js"></script>
> +<script language="JavaScript">
> +$(document).ready(function()
> +{
> +    $("#course_reserves").DataTable({"paging":   false, "bFilter": false, "info":     false});

Hum? Why did you choose this id?

- In intranet-tmpl/prog/en/css/staff-global.css 
  there is "div#course_reserves_wrapper > table#course_reserves" needed to the table. So I just use it (do not change staff-global.css file).
Comment 45 Jonathan Druart 2015-03-09 16:18:16 UTC
(In reply to simith.doliveira from comment #44)
> > +    $("#course_reserves").DataTable({"paging":   false, "bFilter": false, "info":     false});
> 
> Hum? Why did you choose this id?
> 
> - In intranet-tmpl/prog/en/css/staff-global.css 
>   there is "div#course_reserves_wrapper > table#course_reserves" needed to
> the table. So I just use it (do not change staff-global.css file).

Your last patch adds it.
Comment 46 simith.doliveira 2015-03-09 18:58:15 UTC
(In reply to comment #45)

- Your last patch adds it.

Yep, you are right. I don't remember why I put "course_reserves", maybe I was working with other bz at the same time.
Comment 47 Jonathan Druart 2015-03-10 15:45:21 UTC
(In reply to simith.doliveira from comment #46)
> (In reply to comment #45)
> 
> - Your last patch adds it.
> 
> Yep, you are right. I don't remember why I put "course_reserves", maybe I
> was working with other bz at the same time.

So please resubmit a patch with a correct id for this table.
Comment 48 simith.doliveira 2015-03-10 19:56:52 UTC Comment hidden (obsolete)
Comment 49 Marc Véron 2015-03-24 17:15:11 UTC Comment hidden (obsolete)
Comment 50 Marc Véron 2015-03-24 17:34:42 UTC Comment hidden (obsolete)
Comment 51 Marc Véron 2015-03-24 17:36:53 UTC Comment hidden (obsolete)
Comment 52 Marc Véron 2015-03-24 17:38:24 UTC Comment hidden (obsolete)
Comment 53 Jonathan Druart 2015-04-17 09:47:07 UTC
Comment on attachment 37195 [details] [review]
[Signed-off] Bug 13364 - Add a call number browser in item edition page - QA fixed II

Review of attachment 37195 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/cn_browser.tt
@@ +34,4 @@
>  </div>
>  <br />
>  
> +<table id="cn_browser_table">

Yes fine, but a previous patch changed the css file and you don't update it here...
Comment 54 Jonathan Druart 2015-04-17 09:47:33 UTC
I will submit a follow-up if it's ready.
Comment 55 Jonathan Druart 2015-04-17 09:56:27 UTC
Created attachment 38010 [details] [review]
Bug 13364 - Add a call number browser in item edition page

When editing an item, the call number browser search for existing call number. If it shows a line is green between two calli numbers, the call numbers searched is available. If the call number is highlighted in red, it means that it already exists. This can be used to avoid repeated  call numbers in your collection.

The call number browser also shows its position relative to the rest of the collection.

Sponsored-by: CCSR

To test :

                   1) Go to MARC bibliographic framework
                   Home › Administration › MARC frameworks
                   2) Click in MARC structure (Default framework)
                   3) Search field 952 and click in subfields
                   4) Edit subfield o (Full call number)
                   5) Click Display more constraints
                   6) In Others Options, Plugin, Choose cn_browser.pl and save changes
                   7) Search a record and edit its items
                   Validate : three points (…) beside Full call number input
                   8)  Click in …
                   Validate : if it can not find the given call number, a green line is shown. Otherwise,  the given call number is highlighted in red.

Followed test plan. Works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Magnus <magnus@enger.priv.no>
Works as advertised. QA scripts pass.

Tested together with other patches. Works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Bug 13364 - fix datatables.inc

http://bugs.koha-community.org/show_bug.cgi?id=13364
Signed-off-by: Magnus <magnus@enger.priv.no>
Works as advertised. QA scripts pass.

Tested together with other patches, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Bug 13364 - Add a call number browser in item edition page - QA fixed

Tested together with other patches.
Signed-off-by: Marc Véron <veron@veron.ch>

Bug 13364 - Add a call number browser in item edition page - QA fixed II

Tested together with oter patches.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 56 Jonathan Druart 2015-04-17 09:56:35 UTC
Created attachment 38011 [details] [review]
Bug 13364: Fix table id + software error

change table id in css file
+ C4::Output is used (Undefined subroutine
CGI::output_html_with_http_headers)

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 57 Jonathan Druart 2015-04-17 09:57:37 UTC
I squashed the 4 patches (add + mv of the file in the same patch set) and perltidy on it.

Marked as Passed QA.
Comment 58 Jonathan Druart 2015-04-17 10:04:51 UTC
Created attachment 38018 [details] [review]
Bug 13364: Remove unused JS functions
Comment 59 Jonathan Druart 2015-04-17 10:07:32 UTC
Created attachment 38019 [details] [review]
Bug 13364: Use SQL placeholder
Comment 60 Tomás Cohen Arazi 2015-05-06 15:10:08 UTC
Feature pushed to master.

Thanks Simith!
Comment 61 Tomás Cohen Arazi 2015-05-06 17:21:08 UTC
Created attachment 38894 [details] [review]
Bug 13364: (QA followup) <body> missing id and class

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>