Bug 7684 - inventory : datatable fix actions etc.
Summary: inventory : datatable fix actions etc.
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact:
URL:
Keywords:
Depends on:
Blocks: 12025 14001
  Show dependency treegraph
 
Reported: 2012-03-08 16:18 UTC by delaye
Modified: 2015-06-16 08:43 UTC (History)
12 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 7684: multiple fixes for inventory (11.24 KB, patch)
2012-05-04 10:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
[FOLLOW-UP] Bug 7684: multiple fixes for inventory (2.46 KB, patch)
2012-06-21 15:21 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 7684: multiple fixes for inventory (31.30 KB, patch)
2012-08-23 15:00 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 7684: multiple fixes for inventory (32.47 KB, patch)
2012-08-27 13:45 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 7684: multiple fixes for inventory (36.09 KB, patch)
2012-09-20 12:33 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 7684: multiple fixes for inventory (35.55 KB, patch)
2012-10-29 13:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7684: multiple fixes for inventory (35.48 KB, patch)
2013-03-18 09:39 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Fichier chargé pour le récolement (136 bytes, text/plain)
2013-03-20 10:30 UTC, Koha Team AMU
Details
CSV avec software error (9.13 KB, application/vnd.oasis.opendocument.spreadsheet)
2013-03-20 10:31 UTC, Koha Team AMU
Details
Bug 7684: multiple fixes for inventory (36.49 KB, patch)
2013-03-20 14:16 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 7684: multiple fixes for inventory (37.42 KB, patch)
2013-03-20 16:09 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 7684: multiple fixes for inventory (38.01 KB, patch)
2013-03-21 10:28 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 7684: multiple fixes for inventory (38.06 KB, patch)
2013-03-21 10:53 UTC, Paul Poulain
Details | Diff | Splinter Review
Bug 7684: multiple fixes for inventory (36.57 KB, patch)
2013-04-30 09:34 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 7684: multiple fixes for inventory (35.89 KB, patch)
2013-04-30 13:18 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 7684: multiple fixes for inventory (38.77 KB, patch)
2013-06-06 12:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7684: QA Followup (11.71 KB, patch)
2013-06-06 12:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7684: QA Followup and bugfixes (23.86 KB, patch)
2013-06-24 13:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7684: multiple fixes for inventory (38.91 KB, patch)
2013-08-19 14:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7684: QA Followup and bugfixes (23.82 KB, patch)
2013-08-19 14:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7684: multiple fixes for inventory (38.96 KB, patch)
2013-09-16 07:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7684: QA Followup and bugfixes (23.95 KB, patch)
2013-09-16 07:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7684: QA Followup and bugfixes (23.88 KB, patch)
2013-09-18 12:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7684: multiple fixes for inventory (38.73 KB, patch)
2013-09-23 07:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7684: QA Followup and bugfixes (23.89 KB, patch)
2013-09-23 07:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Double lines in table (208.20 KB, image/jpeg)
2013-09-25 09:12 UTC, Mathieu Saby
Details
[PATCH][SIGNED OFF] Bug 7684: QA Followup and bugfixes (23.96 KB, patch)
2013-09-25 12:54 UTC, Mathieu Saby
Details | Diff | Splinter Review
Bug 7684: Argument missing (973 bytes, patch)
2013-10-11 14:30 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7684: multiple fixes for inventory (38.78 KB, patch)
2013-10-11 15:07 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 7684: QA Followup and bugfixes (24.01 KB, patch)
2013-10-11 15:07 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 7684: Argument missing (1.01 KB, patch)
2013-10-11 15:07 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 7684: multiple fixes for inventory (38.83 KB, patch)
2013-10-21 22:34 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7684: QA Followup and bugfixes (24.06 KB, patch)
2013-10-21 22:34 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7684: Argument missing (1.06 KB, patch)
2013-10-21 22:35 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description delaye 2012-03-08 16:18:17 UTC
Check that the inventory tool operates in the mode explained below.

The Inventory Tool can create a report after each load of barcodes.

Reports back the "mistakes" as follows:

    *
      The poorly-sorted copies: copies read through inventory then they should not have to be read as "out of scope" of the inventory. Example: a copy with a item call number 400 was loaded while the inventory concerned the copies with a item call number range 500 ↔ 600.

    *
      To modify the copies: copies read through inventory then they should not have to be read because they are status (notforloan): "In Control", "Lost" ... etc.. Or because they were borrowed.

When the last load of barcode inventory, the final report states an additional anomaly:

    *
      Missing items to the inventory: the items that should have been read and which are missing inside files that were loaded.

Then the library to check the shelves if the copies are actually missing. If this happens, the library change the item status in "Missing inventory". 

The steps to understand the tools :

Step 1: Read barcodes with a shower without file that stores the bar code in a file of type txt.

Step 2: Load the file listing the barcodes of copies. Indicate : branch, location and item call number of the items inside the file.

Field  "Items Statuses" check only the status that will be read during the inventory

At the bottom , in the second field named "Inventory Date" enter the current date only in two cases: 
(1) the file to be processed is the only file on the inventory of a location / slice dimensions, 
(2) the file to be processed is not the last file to load  (you have cut inventory into multiple files). 
When loading the last file "Inventory Date" you indicate the date of loading the first file that was loaded (start date of the inventory)

Check marked 'Skip copies on loan': don't report books borrowed missing inventory.

Check the words 'Export CSV' if you want to save the report of the inventory in a spreadsheet. 
Notes : exported in the csv file labels codes and not codes only.

Check 'Compare barcodes list to results' only when loading the last file in the inventory. Option to control during the "closing" of the inventory if there are copies missing from the inventory.

Step 3: Final Report of the inventory

when loading the copies are modified : the field 'view for the last time' (items.datelastseen) retrieves the current date / time of loading.

Step 4: Go check the shelves documents marked as "missing" and / or poorly-sorted. Updated code "state" of copies.


Warning :When reports  displaying the results data in many screens, check the paging the results to see the all the screens results
Comment 1 Jonathan Druart 2012-05-04 10:20:02 UTC Comment hidden (obsolete)
Comment 2 delaye 2012-05-07 12:36:47 UTC
when i test the patch in a sandbox i have this message when i want submit a file of barcode

Software error:

Undefined subroutine &C4::Circulation::GetItemnumberFromBarcode called at /home/koha/src/C4/Circulation.pm line 1541, <fh00001INV_F1> line 2.
Comment 3 Jonathan Druart 2012-05-16 13:51:27 UTC
This patch does not used this routine (GetItemnumberFromBarcode). Could you retest please ? Maybe it was a bug on master when you have tested
Comment 4 delaye 2012-05-16 15:15:23 UTC
I have the shame message in the sandbox one (biblibre) again
Comment 5 delaye 2012-06-19 09:28:22 UTC
the last test with the sandbox 1 it's work. 
you can test with the file of barcodes (homebranch : MAURES, location : Livre audio jeunesse )
Comment 6 Matthias Meusburger 2012-06-21 15:21:38 UTC Comment hidden (obsolete)
Comment 7 delaye 2012-06-22 09:00:02 UTC
the last patch have broken the previous fix actions
the field "Show" must be hidden becasue it's dont work , instead put a paging under the report
Comment 8 claire.hernandez@biblibre.com 2012-07-02 12:23:35 UTC
Fix needs enhancement to answer to Stephane comment.
Comment 9 Matthias Meusburger 2012-08-23 15:00:27 UTC Comment hidden (obsolete)
Comment 10 Matthias Meusburger 2012-08-23 15:07:16 UTC
Explanations about last attached patch:

At biblibre, we had to rewrite the inventory tool rewrite. Indeed, using datatables with a dynamic datasource was buggy.

For reference:

MT9538/9854: Fix "wrong place" items
    
     This patch removes the datatable with dynamic datasource, which was
     buggy. There is now a "simple" datatable. As the inventory tool now
     displays only items for which there is a problem, the amount of data
     to display is not as important as when all items were displayed, so
     having a "simple" datatable should be ok.


This last attachment reflects those modifications. So it does contains all the new features, but works better.
Comment 11 Matthias Meusburger 2012-08-27 13:45:03 UTC Comment hidden (obsolete)
Comment 12 Matthias Meusburger 2012-08-27 13:46:34 UTC
New patch, a file was missing in the previous one.
Comment 13 Matthias Meusburger 2012-09-20 12:33:57 UTC Comment hidden (obsolete)
Comment 14 Matthias Meusburger 2012-09-20 12:34:47 UTC
This new patch removes the .tmpl file and adds the .tt
Comment 15 delaye 2012-10-04 08:54:25 UTC
matts the specifications required are not installed.. is not good for test
Comment 16 delaye 2012-10-12 13:06:38 UTC
in a sandbox biblibre (1)

CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/tools/inventory.tt

need a rebase ?
Comment 17 Jonathan Druart 2012-10-29 13:06:17 UTC Comment hidden (obsolete)
Comment 18 Owen Leonard 2013-02-07 18:05:46 UTC
Could the test plan on this be elaborated please? A rebased patch would be nice too, although the conflict doesn't look too serious.
Comment 19 delaye 2013-02-15 09:46:29 UTC
1 / Identify a list of items barcode that must be inventoried  (Module Results and statistics).List based on the criteria branch, location and itemcallnumber.
2 / Save the result report (list of barcode) in a csv or txt file.

2 / Go into the inventory module.

2.1 THE SETTING

Load txt/csv file which includes barcode documents.

Secure the perimeters of the inventory using the following criteria: branch,location,and itemcallnumber.Use the same values that the report in the step one 

1st area inventory date: date of the download.

Status: You must check the status of items present in the inventory .

Skip items onloan: ---> Ignore the items onloan.

2nd area inventory date: Date of downloading files or start date of the inventory (first loading)
Items that have a  datelastseen below this date will be reported as missing in the final report.

Compare a list of barcode results: A box only when downloading the last file in the inventory.Report the missing inventory in the final report at the end of the last load file inventory


II - OPERATION

Example: Inventory  with the itemcallnumber between 500 and 599
The Inventory  divided into 3 barcode files => 3 downloads

Start date inventory on 10 June


 -> 1st download file  June 10: inventory dates: 10 june/10june. Do not select "Compare a list of barcode results"
        -> update datelastseen in the items of the 1st file -> June 10
                -> Inventory unfinished: Do not report missing
        -> Indicates the number of items modified (datelastseen)
                -> Reports errors shower (koha barcode unknown)
                -> Indicates that items were borrowed (onloan) and passes the paper back.
                -> Indicates that items could not read (misplaced, missing, lost)
 
 -> 2nd  download June 20: inventory dates: 20 june/20 June Do not select "Compare a list of barcode results"
                -> update datelastseen in the items of the 2nd file -> 20 June
                -> Inventory unfinished: Do not report missing
                -> Reports errors shower (koha barcode unknown)
                -> Indicates that items were borrowed (onloan) and passes the paper back.
                -> Indicates that items could not read (misplaced, missing, lost)

  -> 3rd and last download June 30: date inventory 10 juin/10 june (start date of inventory). Check "Compare a list of barcode results"
                -> update datelastseen in the items of the 3rd file -> 30 June
                -> Indicates the number of items modified (datelastseen)
                -> Reports errors shower (koha barcode unknown)
                -> Indicates that items were borrowed (onloan) and passes the paper back.
                -> Indicates that items could not read (misplaced, missing, lost)
                -> Inventory completed: the missing reports (copies with datelastseen = <the 10 June (start date of the inventory) and whose status was supposed to be read in the inventory).

III - Reports

Two possible forms of report:

1 / In koha on the screen, with the ability to change  datelastseen on items reported as missing in the report while they are on the shelf 

2 / For large inventories and at the ultimate load of the inventory. In a csv file. File that can be used for an update batch of items (tools) to be given to missing items status (notforloan) "missing inventory 2013" for example.



For itemcallnumber NLM
L = Letter
C = Number
NLM rating form
The
Approval to enter into koha
The

NLM rating form
LL
Approval to enter into koha
LL

NLM rating form
The C
Approval to enter into koha
The 00C

NLM rating form
The CC
Approval to enter into koha
The 0CC

NLM rating form
LL CCC
Approval to enter into koha
LL CCC

NLM rating form
LL CC
Approval to enter into koha
LL 0CC

NLM rating form
LL CCC
Approval to enter into koha
LL CCC
Comment 20 delaye 2013-02-15 09:56:11 UTC
To test the items missing remove items from your list of barcode.
To test the items misclassified added items in your list of barcod . 
To test errors scan barcode add barcode unknown in your list of barcode
To test the items in use. A add in your list items in use.

Note : this tool show cases in a report => items missing , items misclassified, items must by modified(notforloan).The tools update only items.datelasteen in the record items.It does not change items.notforloan for example ...
Comment 21 Matthias Meusburger 2013-03-18 09:39:08 UTC Comment hidden (obsolete)
Comment 22 Matthias Meusburger 2013-03-18 09:39:45 UTC
Last attachment is a rebase.
Comment 23 Koha Team AMU 2013-03-20 09:54:50 UTC
Test of different cases :
- item on loan
- scan error of a barcode
- notforloan not supposed to be in the list
- wrong place (homebranch or location or itemcallnumber)
- missing items

OK for missing items
OK wrong barcode
OK change item status when the notforloan should not be read (but not title, no author, and only the code of the norforloan)
OK for changing page in the report on line 

KO wrong place : there are listed but with the problem "change item status". No author, no title, code of the notforloan

KO csv (see attachment) : 
Can't call method &quot;cookie&quot; on an undefined value at /home/koha/src/C4/Templates.pm line 329

KO "Mark seen and continue" (but OK "Mark Seen and Quit")

KO item on loan : 
- when "Skip copies on loan:" (Ignorer les exemplaires en prêt : ) is selected : message  "item was on loan. It was returned before marked as seen " and the book is not anymore on the user's count.
- when "Skip copies on loan:" is not selected : nothing happen and the item is not missing.
Perhaps should the expression "Skip copies on loan" be changed? (or very well documented)
Comment 24 Koha Team AMU 2013-03-20 10:30:46 UTC
Created attachment 16483 [details]
Fichier chargé pour le récolement

Homebranch : BDP
Location : BD adulte
itemcallnumber : BD à BE
1 doc en prêt
1 en statut réparation
Comment 25 Koha Team AMU 2013-03-20 10:31:27 UTC Comment hidden (obsolete)
Comment 26 Matthias Meusburger 2013-03-20 14:16:13 UTC Comment hidden (obsolete)
Comment 27 Matthias Meusburger 2013-03-20 14:17:44 UTC
This new patch fixes:

No author, no title, code of the notforloan
csv
"Mark seen and continue"
Comment 28 Matthias Meusburger 2013-03-20 16:09:47 UTC Comment hidden (obsolete)
Comment 29 Matthias Meusburger 2013-03-20 16:11:12 UTC
This new patch fixes "wrong place" and adds separate lines when an item is in "wrong place" *and* "change item status".
Comment 30 Koha Team AMU 2013-03-21 09:06:27 UTC
OK "item should not have been scanned" and "change item status"
OK description of the notforloan
OK barcode not found
OK title and author for all items
OK csv

KO missing item : no missing item! (neither in csv, nor on line). So : "mark seen and continue" can not be tested.
KO csv column A : no title

And in the csv, an item with the wrong status and the wrong location is twice, but the second time with the 2 reasons.
Comment 31 Koha Team AMU 2013-03-21 09:23:31 UTC
A question : the link on the title (in the online report) shouldn't it lead to the catalog?
http://pro.test3.biblibre.com/cgi-bin/koha/catalogue/detail.pl?biblionumber=1 (the real biblionumber)
instead of
http://pro.test3.biblibre.com/cgi-bin/koha/tools/inventory.pl#
Comment 32 Matthias Meusburger 2013-03-21 10:28:38 UTC Comment hidden (obsolete)
Comment 33 Matthias Meusburger 2013-03-21 10:29:29 UTC
All OK fixed. 

As for the question, it is indeed a link to the record, but in a new window, so the inventory page is not replaced.
Comment 34 Paul Poulain 2013-03-21 10:52:35 UTC
Patch tested with a sandbox, by Leila <koha.aixmarseille@gmail.com>
Comment 35 Paul Poulain 2013-03-21 10:53:01 UTC Comment hidden (obsolete)
Comment 36 Koha Team AMU 2013-03-21 11:01:12 UTC
It's all right. Thanks!

OK Missing item, column A of the csv, no column M, mark seen and continue

About the link from the title
- if one clicks with the left button : pop up opens on the catalog page
- if one clicks with the middle button : new page opens on the inventory tool
Comment 37 Katrin Fischer 2013-04-16 09:20:13 UTC
Hi Team AMU, 
could you write up some more detailed test plan with steps for me? This bug has quite a long history now and it would make testing easier for me as I have not much experience in using the inventory tool.
Comment 38 Jonathan Druart 2013-04-19 14:18:24 UTC
(In reply to comment #37)
> Hi Team AMU, 
> could you write up some more detailed test plan with steps for me? This bug
> has quite a long history now and it would make testing easier for me as I
> have not much experience in using the inventory tool.

Katrin, A test plan had provided on comment 19, isn't enough?
Comment 39 Katrin Fischer 2013-04-28 19:58:46 UTC
Some small things found by the QA script:

QA SCRIPT
testing 1 commit(s) (applied to 9c79f18 'Merge branch 'bug_10088' into 3.14-ma')

 5fa6a41 Bug 7684: multiple fixes for inventory
 - C4/Auth.pm
 - C4/Items.pm
 - C4/Templates.pm
 - koha-tmpl/intranet-tmpl/prog/en/columns.def
 - koha-tmpl/intranet-tmpl/prog/en/modules/tools/inventory.tt
 - tools/ajax-inventory.pl
 - tools/inventory.pl

 * C4/Auth.pm                                                               OK
	pod                         OK
	forbidden patterns          OK
	valid                       OK
	critic                      OK
 * C4/Items.pm                                                              FAIL
	pod                         FAIL
		*** WARNING: No items in =over  / =back list  in file C4/Items.pm
	forbidden patterns          OK
	valid                       OK
	critic                      OK
 * C4/Templates.pm                                                          OK
	pod                         OK
	forbidden patterns          OK
	valid                       OK
	critic                      OK
 * tools/ajax-inventory.pl                                                  OK
	pod                         OK
	forbidden patterns          OK
	valid                       OK
	critic                      OK
 * tools/inventory.pl                                                       FAIL
	pod                         OK
	forbidden patterns          FAIL
		The patch introduces a forbidden pattern: tabulation character (346)
		The patch introduces a forbidden pattern: tabulation character (345)
		The patch introduces a forbidden pattern: tabulation character (342)
		The patch introduces a forbidden pattern: tabulation character (347)
		The patch introduces a forbidden pattern: tabulation character (344)
		The patch introduces a forbidden pattern: tabulation character (348)
		The patch introduces a forbidden pattern: tabulation character (343)
	valid                       OK
	critic                      OK
 * koha-tmpl/intranet-tmpl/prog/en/modules/tools/inventory.tt               FAIL
	forbidden patterns          FAIL
		The patch introduces a forbidden pattern: tabulation character (209)
		The patch introduces a forbidden pattern: tabulation character (21)
		The patch introduces a forbidden pattern: tabulation character (260)
		The patch introduces a forbidden pattern: tabulation character (19)
	tt_valid                    OK
	valid_template              OK

Please fix and provide a QA follow up. I will continue testing.
Comment 40 Katrin Fischer 2013-04-28 20:32:22 UTC
Hi Matthias,

I finally got to look at this, but I spotted some problems with the patch. I think they might stem from this development being based on an older version of Koha. The major things I found are:

1) You are using SELECT SQL_CALC_FOUND_ROWS which is a MySQLism. Please can you rewrite that part?

2) I think you remove an existing feature as I can't see where this is added back in: 
-        <li><label for="pagesize">Show: </label>
-            <input type="text" id="pagesize" name="pagesize" value="[% pagesize %]" maxlength="5" size="5" /> items</li>
-          <li><label for="offset">Beginning at offset: </label>
-            <input type="text" id="offset" name="offset" value="[% offset %]" size="5" maxlength="5" />
-        </li>

3) There are changes related to authentication that don't seem to belong into this patch?

4) The calendar related code seems to be based on older code, using C4:Dates and i spotted one DHTML... - could you take a look?

5) There are some problems with the templates, like capitalization, terms used and the id in the body going missing.
Comment 41 Matthias Meusburger 2013-04-30 09:34:08 UTC Comment hidden (obsolete)
Comment 42 Matthias Meusburger 2013-04-30 09:34:38 UTC
previous patch is a rebase.
Comment 43 Matthias Meusburger 2013-04-30 13:18:01 UTC Comment hidden (obsolete)
Comment 44 Matthias Meusburger 2013-04-30 13:18:41 UTC
Katrin, here are my answers to your remarks:

1) I could rewrite this without the SELECT SQL_CALC_FOUND_ROWS part, but it would be an important loss of performance (because I would have to relaunch the main time-consuming request a second time)... So is it worth it? (and I say that even though I know we're trying to get rid of mysqlisms)

2) This feature is not longer necessary, as the use of datatable for the results allows the user to change pagination and offset once the results are here.

3) Indeed. Removed.

4) The calendars are using datepicker, the DHTMLCalendar line was a leftover, it has been removed. As for the dates, since the calendars are used to set items.datelastseen, which is a date, and not a datetime, does it make sense to use Koha::DateUtils instead of C4::Dates?

5) For the terms used, I kept the ones from HEAD when I rebased, so it might be ok now? Also, <body> fixed.
Comment 45 Katrin Fischer 2013-05-01 08:33:03 UTC
Hi Matthias, 

looking again at your answers ant the new patch I still spot some problems:

1) Yes, I think it's worth it. We should not introduce new MySQLisms.

4) C4:Dates is deprecated as we are trying to get rid of it in favour of the new modules. Also I was wondering about the display of dates in the templates, it looks to me like this line should be using the TT plugin?

+    [% IF (moddatecount) %]<div class="dialog message">[% moddatecount %] items modified : datelastseen set to [% date %]</div>[% END %]

5) Terms might be wrong already, but it would be good to always use 'library' instead of branch or location when it's actually referring to home branch, holding branch etc. in the database. 

6) There are some terms where capitalization is wrong: Clear All etc.

Sorry :(
Comment 46 Jonathan Druart 2013-06-06 12:40:40 UTC Comment hidden (obsolete)
Comment 47 Jonathan Druart 2013-06-06 12:40:47 UTC Comment hidden (obsolete)
Comment 48 Jonathan Druart 2013-06-06 12:42:41 UTC
First patch has been amended in order to fix some QA issues (whitespaces and tabulations)
Comment 49 Jonathan Druart 2013-06-06 12:45:14 UTC
Note:
The following qa fail seems to be a false positive:
 FAIL	C4/Items.pm
   FAIL	  pod
		*** WARNING: No items in =over  / =back list  in file C4/Items.pm
Comment 50 Mathieu Saby 2013-06-11 21:06:56 UTC
Hello
This patch is needed, but I fear it broke something : 

If I use inventory module to export all the items of my library (all locations), without comparing the result with a file of barcode :

A. Without the patch : I get a list of 944 records
B. With the patch : 0  records (whatever boxes I check in statuses list...) 

M. Saby
Comment 51 Sonia Lemaire 2013-06-20 15:33:31 UTC
I tested the patch using the two ways of making an inventory.

The first way, by comparing a barcode file with a list of results :

- I upload a barcode file
- I select the library and the shelving location of the items I want to check
- I choose the statuses of the items
- I check the box “skip copies on loan”
- I check the box “export to CSV file”
- I check the box “compare barcodes list to results”


Ok for items on loan : they have been checked in.
Ok for misplaced items : marked as “should not have been scanned” on the screen and “wrong place” in the CSV file.
Ok for wrong status : a notforloan item which should not be on the shelf  is marked as “change item status” in both the online report and the CSV file.
Ok for unknown barcode (scan error or item not found in koha) : marked as “barcode not found”
ok for setting datelastseen to the inventory date
Ok for CSV export

Not ok : missing item : if a barcode has not been scanned when it should have been, it is ignored by the tool and no problem is reported.


The second way, by creating a list of items to be checked “manually”

- I select the library and the shelving location of the items I want to check
- I choose the statuses of the items
- I check the box “export to CSV file”

As Mathieu Saby notes, it doesn't work. No list is created, neither on the online report, nor in the CSV file.
Comment 52 Jonathan Druart 2013-06-24 13:14:37 UTC Comment hidden (obsolete)
Comment 53 Jonathan Druart 2013-07-02 10:11:43 UTC
A wiki page has been created: http://wiki.koha-community.org/wiki/Inventory
Comment 54 Jonathan Druart 2013-08-19 14:47:29 UTC Comment hidden (obsolete)
Comment 55 Jonathan Druart 2013-08-19 14:47:36 UTC Comment hidden (obsolete)
Comment 56 Mathieu Saby 2013-09-14 08:16:55 UTC
Sorry, main patch does not apply 

Applying: Bug 7684: multiple fixes for inventory
fatal: sha1 information is lacking or useless (tools/inventory.pl).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 7684: multiple fixes for inventory

Mathieu
Comment 57 Jonathan Druart 2013-09-16 07:56:46 UTC Comment hidden (obsolete)
Comment 58 Jonathan Druart 2013-09-16 07:56:52 UTC Comment hidden (obsolete)
Comment 59 Mathieu Saby 2013-09-18 12:44:37 UTC
Sorry...


Software error:

syntax error at /home/msaby/kohamaster/src/tools/inventory.pl line 206, near "HEAD
# now build the result list: inventoried items if requested, and mis-placed items -always-
my "
Global symbol "$inventorylist" requires explicit package name at /home/msaby/kohamaster/src/tools/inventory.pl line 206.
Global symbol "$inventorylist" requires explicit package name at /home/msaby/kohamaster/src/tools/inventory.pl line 211.
Global symbol "$inventorylist" requires explicit package name at /home/msaby/kohamaster/src/tools/inventory.pl line 218.
Global symbol "$inventorylist" requires explicit package name at /home/msaby/kohamaster/src/tools/inventory.pl line 259.
Global symbol "$inventorylist" requires explicit package name at /home/msaby/kohamaster/src/tools/inventory.pl line 271.
Global symbol "$inventorylist" requires explicit package name at /home/msaby/kohamaster/src/tools/inventory.pl line 292.
BEGIN not safe after errors--compilation aborted at /home/msaby/kohamaster/src/tools/inventory.pl line 301.

For help, please send mail to the webmaster (webmaster@msaby-VirtualBox), giving this error message and the time and date of the error.
Comment 60 Jonathan Druart 2013-09-18 12:58:55 UTC Comment hidden (obsolete)
Comment 61 Jonathan Druart 2013-09-18 13:00:00 UTC
(In reply to mathieu saby from comment #59)
> Sorry...
> 
> 
> Software error:
> 
> syntax error at /home/msaby/kohamaster/src/tools/inventory.pl line 206, near
> "HEAD

Oops, sorry. A conflict was not fixed.
Comment 62 Mathieu Saby 2013-09-19 10:46:13 UTC
My first test.
For the moment I only try to display a list of items and mark them seen, without comparing with a barcode file

Trying to see all the items of a branch and location

Before patch :
- need to select XX items
- in the resulting table : 5 columns "Seen", "Barcode", "Title", "Unseen since", "Problems"
- the problem column contains the string "None"
- no way to sort the table

After the patch
- no more need to select XX items : all the items are listed in a dynamic table with a pager
- I checked the number of results, it is correct
- the pager works fine
- in the resulting table : 4 more columns : "Library", "Status", "Lost", "Damaged"
- no column for type of document (at item level or biblio level)
- no column for withdrawn status
- column library contains the name of the branch, followed by the location and the callnumber into [] : "BU En Magasin [130593/5/1]"
- status contains the "translated" value of coded status : "Empruntable" for item for loan
- columns are sortable. it works well
- column "problem" is void. but the "none" message was useless

3 buttons on bottom of table :
Mark seen and Quit, Mark seen and Continue, Continue without marking

On 1st page of results, I check 2 items and click on "Mark seen and Continue".
The pager shows me automaticaly the 2d page. I click on 2 more items
I click on "Mark seen and Quit"

I am redirected to the home page of inventory.
No message confirming me that 4 items were marked seen
I made a report to confirm that : the 4 items were marked seen

Same test, but checking "export to csv" : data well exported (encoded in iso 8859 and not in unicode). I can see the same informations as in screen display (but 3 columns for library - location - callnumber)
=> OK

Same test, but using the filters in inventory home page on "items.notforloan", "
items.itemlost", "items.withdrawn", "items.damaged"
Only the items matching my filter criteria are displayed on screen or exported to csv
=> OK
 
Same test, but only showing callnumber between XA and Z (my callnumber are like A 000, A 002 .... B 001, B 002 ... Z 999)
Only callnumbers begining with XA->XG are displayed
=> OK for this kind of callnumbers

Same test, but only showing callnumber between 130593/5/1 and 166026/1962/2
(my callnumber are numeric, sometimes with a slash after the main number)
0 results
=> KO for this kind of callnumbers

=====
It looks better than before, but 
- why not a column on "withdrawn" status, as it is one of the possible filters?
- wot sure the callnumber filter is working well.

Could you remind me when the column "Problem" is supposed to show something?

I haven't try to compare the results with a file of barcode yet.


Mathieu
Comment 63 Mathieu Saby 2013-09-19 10:52:24 UTC
I know it was already tested by Marseille, but there is a looot of things to check, so I prefered make my "full" own tests.

Mathieu
Comment 64 Jonathan Druart 2013-09-19 12:08:50 UTC
(In reply to mathieu saby from comment #62)
> Same test, but only showing callnumber between 130593/5/1 and 166026/1962/2
> (my callnumber are numeric, sometimes with a slash after the main number)
> 0 results
> => KO for this kind of callnumbers

It is not introduce by this patch, it was the previous behavior. Maybe a fix could be added later. Currently the selection is done with the sql query (itemcallnumber < ? and itemcallnumber > ?)

> It looks better than before, but 
> - why not a column on "withdrawn" status, as it is one of the possible
> filters?

It could be added later too if needed.

> - wot sure the callnumber filter is working well.

Bug 9456 introduces a sort on callnumber values, but it is not pushed for now.

> Could you remind me when the column "Problem" is supposed to show something?

There are 4 possible problems:
  * wrong place: the item should not have been scanned
  * missing: if the "compare barcodes list to results" has been checked, the alert is raised if the item does not appear in the results list.
  * change status: if an item has been scanned with a non-matching notforloan value
  * not scanned: the opposite of "missing".
Comment 65 Mathieu Saby 2013-09-19 12:30:54 UTC
Ok, so if I understand well, the "problem" column is always blank is we don't compare Koha's data with a barcode file.
That's what I wanted to know.

I'll make an other test with a file of barcode.

Mathieu
Comment 66 I'm just a bot 2013-09-20 10:09:06 UTC
This bug depends on 10625 which is in status In Discussion
Comment 67 Mathieu Saby 2013-09-20 10:15:53 UTC
Oups, I hope Mr. Bot is wrong?

Mathieu
Comment 68 Jonathan Druart 2013-09-20 10:20:38 UTC
(In reply to I'm just a bot from comment #66)
> This bug depends on 10625 which is in status In Discussion

The patch can be tested even so.
Comment 69 Chris Cormack 2013-09-20 19:41:08 UTC
(In reply to Jonathan Druart from comment #68)
> (In reply to I'm just a bot from comment #66)
> > This bug depends on 10625 which is in status In Discussion
> 
> The patch can be tested even so.

In that case, why don't we remove the dependency? If that is not needed for this, then it isn't really dependent?

If the dependency is removed, the bot can test the patches, otherwise It can't because it can't apply the dependency. This goes for humans too, it is confusing to be marked dependent if it isn't.
Comment 70 Mathieu Saby 2013-09-21 09:20:16 UTC
Error while trying a read a file :
"Undefined subroutine &main::read_file called at /home/msaby/kohamaster/src/tools/inventory.pl line 163."

Also  :
- I never understood why there are 2 "inventory date" fields. Could you tell me?

- I never understood why there is the need to check a box for comparing the result with a file, if we have uploaded  a Barcode file. Is not it obvious? Or I am missing something? What is supposed to happend if we upload a file and don't check the box?

Mathieu
Comment 71 Jonathan Druart 2013-09-23 07:38:57 UTC
(In reply to Chris Cormack from comment #69)
> > The patch can be tested even so.
> 
> In that case, why don't we remove the dependency? If that is not needed for
> this, then it isn't really dependent?
> 
> If the dependency is removed, the bot can test the patches, otherwise It
> can't because it can't apply the dependency. This goes for humans too, it is
> confusing to be marked dependent if it isn't.

The depend patch was necessary (if you want this patch applies cleanly) but since it is blocker in discussion, I will remove the dependency.

(In reply to mathieu saby from comment #70)
> Error while trying a read a file :
> "Undefined subroutine &main::read_file called at
> /home/msaby/kohamaster/src/tools/inventory.pl line 163."

You didn't apply bug 10625. But I will resubmit patch without using this dependency!

> Also  :
> - I never understood why there are 2 "inventory date" fields. Could you tell
> me?
> 
> - I never understood why there is the need to check a box for comparing the
> result with a file, if we have uploaded  a Barcode file. Is not it obvious?
> Or I am missing something? What is supposed to happend if we upload a file
> and don't check the box?

Did you have a look at the wiki page? (see comment 53)
Comment 72 Jonathan Druart 2013-09-23 07:40:05 UTC Comment hidden (obsolete)
Comment 73 Jonathan Druart 2013-09-23 07:40:12 UTC Comment hidden (obsolete)
Comment 74 Mathieu Saby 2013-09-23 07:46:29 UTC
I did not understand it was needed ;-)
I will test it again today.

Mathieu
Comment 75 Mathieu Saby 2013-09-24 09:28:20 UTC
Failed QA : 

I exported a list of barcode, put it in a file, try to use the file in inventory tool.

Some barcodes were not recognized

"1754773" : barcode not found
"RETRO20-833237" : barcode not found
"RETRO20-833257" : barcode not found
"RETRO20-773337" : barcode not found
"RETRO20-833238" : barcode not found
"0287755958" : barcode not found 

Some were recognized and put in the table

The issue does not seem to be the structure of barcodes (numeric or alphanumeric with "RETRO-" string before a number).

Mathieu
Comment 76 Jonathan Druart 2013-09-24 09:34:36 UTC
(In reply to mathieu saby from comment #75)
> Failed QA : 
> Some were recognized and put in the table

With something in the problem column?
Comment 77 Mathieu Saby 2013-09-24 09:35:24 UTC
oups, indeed.

"Item should have been scanned"

Mathieu
Comment 78 Mathieu Saby 2013-09-24 09:36:13 UTC
So, I suppose ALL the items of my file were in the list of 
.... barcodes not found
(in yellow boxes)
Comment 79 Jonathan Druart 2013-09-24 09:39:04 UTC
(In reply to mathieu saby from comment #77)
> oups, indeed.
> 
> "Item should have been scanned"

So, it is the expected behavior.
Comment 80 Mathieu Saby 2013-09-24 09:52:34 UTC
For the table, yes
But all the barcodes of my file were in my catalog. So I should not have 68 yellow boxes with "item not found".

Mathieu
Comment 81 Jonathan Druart 2013-09-24 10:05:03 UTC
(In reply to mathieu saby from comment #80)
> For the table, yes
> But all the barcodes of my file were in my catalog. So I should not have 68
> yellow boxes with "item not found".

Did you mean "*barcode* not found"?

What is the result for "SELECT * FROM items WHERE barcode = 'RETRO20-833238';"?
Comment 82 Mathieu Saby 2013-09-24 11:32:42 UTC
yes, barcode notfound.
The query gives 1 answer

Mathieu
Comment 83 Mathieu Saby 2013-09-24 11:33:58 UTC
Note : my csv file has " " around barcode.
I think the issue comes from there
Comment 84 Mathieu Saby 2013-09-24 11:59:19 UTC
The " was the cause of the problem... I tested without them, it is ok.

Test 1 : 
- file with 68 barcodes from Library1 - Location 1 
- compared with the collection of Library1 - Location 1 
- message "68 items modified : datelastseen set to 24/09/2013"
- in the table : 621 entries
-- 68 items with "Change item status" in Problem column
-- the other with "Item should not have been scanned"
=> OK

Test 1 : 
- same file
- compared with the collection of Library1 - Location 1 
- checkbox checked "Exclu du prêt" (in the first column)
- message "68 items modified : datelastseen set to 24/09/2013"
- in the table : 129 entries
-- 68 items with "Change item status" in Problem column
-- the other with "Item should not have been scanned"
=> KO because I don't have 129 "Exclu du prêt" items in my database

The query
select itemnumber from items where homebranch="BU" and location ="MAG" and notforloan = 3
gives me 18 items with notforloan=3
"3" is the value of "Exclu du prêt" in authorized values lists mapped with notforloan.

And indeed, in the list of 129 entries, some have "Empruntable" status...

So I think something is wrong.

An other point (a little detail) : maybe in english "Change item status" is not right. What about "Item status changed" or something more explicit like "Item marked seen" ?

Mathieu
Comment 85 Mathieu Saby 2013-09-24 12:02:40 UTC
Oups, I forgot an other issue : 
"Item should not have been scanned" is not a good message. 
this items were NOT in my barcode file, were IN my database
So the message should have been "Item have been scanned"

Mathieu
Comment 86 delaye 2013-09-25 07:49:44 UTC
the test description is not quite accurate:
we not know the detailed contents of the test file
How many copies by status (notforlona)?
How many copies borrowed? How many copies missing unread?
otherwise we do not know if you checked the reference comparrer barcodes ...


the patch work properly..I do not agree to change the messages
Comment 87 Mathieu Saby 2013-09-25 08:25:51 UTC
Maybe my data are corrupted? So I will test on a sandbox and give you all these informations.

Mathieu
Comment 88 Mathieu Saby 2013-09-25 09:12:15 UTC
New test on Sandbox 3 with its data.
There is an issue, but not the one I thought:
Some lines are doubled in the table!
Look at the screenshot.

Operations I made :
I took 33 barcodes from MAURES / Documentaires adultes, whose 8 are "Exclus du prêt" and 0 on loan.
I put them in a file, inject the file in inventory tool, check the box "Exclu du prêt"

List of barcodes in my file :
Exclus du prêt
00519000155050
00519000177146
00519000190438
00519000095587
00519000100510
00519000106178
00519000106848
00519000106855

other:
00519000153048
00519000162478
00519000163450
00519000164128
00519000164870
00519000175439
00519000085356
00519000098656
00519000099480
00519000099597
00519000108943
00519000111699
00519000120146
00519000120518
00519000120831
00519000121250
00519000121763
00519000122902
00519000140805
00519000141605
00519000142793
00519000150549
00519000151505
00519000000140
00519000000207


M. Saby
Comment 89 Mathieu Saby 2013-09-25 09:12:41 UTC Comment hidden (obsolete)
Comment 90 Jonathan Druart 2013-09-25 09:20:06 UTC
(In reply to mathieu saby from comment #89)
> Created attachment 21440 [details]
> Double lines in table

1 line / problem. It is normal.
Comment 91 Mathieu Saby 2013-09-25 09:36:55 UTC
AHHH
OK I begin to understand how it works! So the number of "entries" of the table is not the number of items. 
Now that I have understood that, I'm very close to sign off.

But what kind of problem is "Change item status" if the item was scanned and if it not a surprise? Do I need to change the item status? in what way?

[Little bug, but I think it was here before, so it could be fixed in an other patch : when you click on a title, you don't see the record, but the home page of inventory page.]


Mathieu
Comment 92 Jonathan Druart 2013-09-25 09:49:08 UTC
(In reply to mathieu saby from comment #91)
> But what kind of problem is "Change item status" if the item was scanned and
> if it not a surprise? Do I need to change the item status? in what way?

Search "change item status" on this page. Koha Team AMU explains how she/he tested the patch.

> [Little bug, but I think it was here before, so it could be fixed in an
> other patch : when you click on a title, you don't see the record, but the
> home page of inventory page.]

I don't reproduce. When I click on a title, a new window is opened and point to catalogue/MARCdetail.pl?biblionumber=XXX
Comment 93 Mathieu Saby 2013-09-25 10:17:05 UTC
(In reply to Jonathan Druart from comment #92)
> (In reply to mathieu saby from comment #91)
> > But what kind of problem is "Change item status" if the item was scanned and
> > if it not a surprise? Do I need to change the item status? in what way?
> 
> Search "change item status" on this page. Koha Team AMU explains how she/he
> tested the patch.
> 

I did not read all the long discussion for this bug. I see there is a lot of informations about how to test it that were not included in the commit message, especially comment 19. That's why I lost some time.

> I don't reproduce. When I click on a title, a new window is opened and point
> to catalogue/MARCdetail.pl?biblionumber=XXX

Ok, I tried to open in a new tab instead of just clicking...

I am taking a last look at this, but it should be ok.

Mathieu
Comment 94 Mathieu Saby 2013-09-25 12:52:23 UTC
Ok, I have also read the documentation on your wiki (http://wiki.koha-community.org/wiki/Inventory)
Some details are only in comments on this bug, for example the way of doing an inventory split between 3 files on 3 different dates. I think I understand now the 2 fields for date and the checkbox "Compare barcodes list to results".

When it is pushed the documentation for this feature will need some substantial updates.

I sign off, but I make a cosmetic remark : in csv file, the message "wrong place" stands for the message "Item should not have been scanned" in staff interface. Maybe it should be the same message.

Mathieu
Comment 95 Mathieu Saby 2013-09-25 12:54:12 UTC Comment hidden (obsolete)
Comment 96 Jonathan Druart 2013-10-11 14:30:51 UTC Comment hidden (obsolete)
Comment 97 Biblibre Sandboxes 2013-10-11 15:06:13 UTC
Patch tested with a sandbox, by Koha Team Amu <koha.aixmarseille@gmail.com>
Comment 98 Biblibre Sandboxes 2013-10-11 15:07:08 UTC Comment hidden (obsolete)
Comment 99 Biblibre Sandboxes 2013-10-11 15:07:13 UTC Comment hidden (obsolete)
Comment 100 Biblibre Sandboxes 2013-10-11 15:07:17 UTC Comment hidden (obsolete)
Comment 101 Koha Team AMU 2013-10-11 15:11:18 UTC
Just a problem fixed : the csv was ko.

It's really better now! Thanks
Comment 102 Kyle M Hall 2013-10-21 22:34:40 UTC
Created attachment 22208 [details] [review]
Bug 7684: multiple fixes for inventory

* when a file was uploaded and the comparison with catalogue range
 requested, the comparison was wrong: the logic was wrong
* items that were not supposed to be scanned (ie: supposed to be on another shelf)
  didn't had the author and title, it was hard to retrieve them on the shelved
* some useful fields were missing, like homebranch, location, status
* the CSV export contained all the item information. It should contain the same
   informations as the screen

Behaviour now:
   * scan a list of barcode & select a range of location
   * if a barcode has been scanned and should not be (mis placed item), the information is displayed
   * if you choose "compare barcodes list to result option", the resulting list contains all items that have been scanned and those that were supposed to be. Any item not in both list appears with a specific message on the last column

Signed-off-by: Leila <koha.aixmarseille@gmail.com>

Signed-off-by: Koha Team Amu <koha.aixmarseille@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 103 Kyle M Hall 2013-10-21 22:34:51 UTC
Created attachment 22209 [details] [review]
Bug 7684: QA Followup and bugfixes

This followup fixes some QA issues:
- replace the MySQLism SQL_CALC_FOUND_ROWS
- use Koha::DateUtils instead of C4::Dates
- replace "branch" and "location" with "library"
- fixe wrong capitalisation on "Clear all" and "Select all"

and fixes some behaviors:
- the inventory tools can be used without barcode file (fixed for the
  csv export too).
- mark as not scanned a non scanned item.
- update the datelastseen 1 time per biblio (and fixes the displayed
  count)

Signed-off-by: Mathieu Saby <mathieu.saby@univ-rennes2.fr>

Signed-off-by: Koha Team Amu <koha.aixmarseille@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 104 Kyle M Hall 2013-10-21 22:35:15 UTC
Created attachment 22210 [details] [review]
Bug 7684: Argument missing

The cgi argument in missing for routine
C4::Reports::Guided::_get_column_defs. It causes a crash on generating
the CSV file.

Signed-off-by: Koha Team Amu <koha.aixmarseille@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 105 Galen Charlton 2013-11-01 00:26:07 UTC
Pushed to master, along with a follow-up to fixes UTF-8 encoding issues in the CSV export.  Thanks, Mathieu!