Bug 20131 - Inventory optional filters always shows "For loan" for value 0
Summary: Inventory optional filters always shows "For loan" for value 0
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Target Milestone: ---
Assignee: Fridolin SOMERS
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 12913
Blocks:
  Show dependency treegraph
 
Reported: 2018-02-02 13:32 UTC by Fridolin SOMERS
Modified: 2018-10-24 09:13 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 20131: Inventory optional filters correct "For loan" addition (3.38 KB, patch)
2018-02-02 13:54 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
Bug 20131: Inventory optional filters correct "For loan" addition (3.52 KB, patch)
2018-03-15 10:26 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 20131: Inventory optional filters correct "For loan" addition (3.55 KB, patch)
2018-09-14 23:10 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
Bug 20131: Inventory optional filters correct "For loan" addition (3.65 KB, patch)
2018-09-15 00:36 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 20131: Inventory optional filters correct "For loan" addition (3.61 KB, patch)
2018-10-10 00:29 UTC, Chris Cormack
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin SOMERS 2018-02-02 13:32:45 UTC
In inventory, optional filters are build from authorize values categories used for items.notforloan, items.itemlost, items.withdrawn and items.damaged.

The bug is that description of value 0 is always "For loan".
This seems to be only for "notforloan".

Second bug is that if a description exists for "notforloan" value 0, it is not used.
Comment 1 Fridolin SOMERS 2018-02-02 13:54:22 UTC
"For loan" string has been added by Bug 12913
Comment 2 Fridolin SOMERS 2018-02-02 13:54:57 UTC
Created attachment 71149 [details] [review]
Bug 20131: Inventory optional filters correct "For loan" addition

In inventory, optional filters are build from authorize values categories used for items.notforloan, items.itemlost, items.withdrawn and items.damaged.

The bug is that description of value 0 is always "For loan".
This seems to be only for "notforloan".

Second bug is that if a description exists for "notforloan" value 0, it is not used.

This patch corrects by adding "For loan" only for "notforloan" authorized values and if there is no 0 value.
It changes the description added by code from "ignore" to a more unlikly user defined "__IGNORE__".

Test plan:
1) Make sure you have authorized values defined on items.notforloan, say "NFL" category, with at least a value and description but no 0 value
2) Make sure you have authorized values defined on items.itemlost, say "LOST" category, with 0 = "Not lost" and 1 = "Lost"
3) Go to inventory /cgi-bin/koha/tools/inventory.pl
4) Without patch you see "For loan" on both items.notforloan and items.itemlost
5) With patch you see "For loan" only in items.notforloan, you see for items.itemlost : "Not lost" and "Lost"
6) Create in authorized values category "NFL" a value 0 with description "Available"
7) Go to inventory /cgi-bin/koha/tools/inventory.pl
8) Without patch you see "For loan" in items.notforloan for value 0
9) With patch you see "Available" in items.notforloan for value 0
Comment 3 Lucie Gay 2018-03-12 11:35:47 UTC
We can't test because there is only one inventory filter notforloan but the values are from the authorized value "statut" and not "notforloan". There are no filter for "lost".
Tested on unimarc sandbox.
Comment 4 Victor Grousset/tuxayo 2018-03-12 11:47:08 UTC
@Fridolin can you test on a BibLibre's sandbox?
Comment 5 Séverine Queune 2018-03-15 10:26:39 UTC
Created attachment 72923 [details] [review]
Bug 20131: Inventory optional filters correct "For loan" addition

In inventory, optional filters are build from authorize values categories used for items.notforloan, items.itemlost, items.withdrawn and items.damaged.

The bug is that description of value 0 is always "For loan".
This seems to be only for "notforloan".

Second bug is that if a description exists for "notforloan" value 0, it is not used.

This patch corrects by adding "For loan" only for "notforloan" authorized values and if there is no 0 value.
It changes the description added by code from "ignore" to a more unlikly user defined "__IGNORE__".

Test plan:
1) Make sure you have authorized values defined on items.notforloan, say "NFL" category, with at least a value and description but no 0 value
2) Make sure you have authorized values defined on items.itemlost, say "LOST" category, with 0 = "Not lost" and 1 = "Lost"
3) Go to inventory /cgi-bin/koha/tools/inventory.pl
4) Without patch you see "For loan" on both items.notforloan and items.itemlost
5) With patch you see "For loan" only in items.notforloan, you see for items.itemlost : "Not lost" and "Lost"
6) Create in authorized values category "NFL" a value 0 with description "Available"
7) Go to inventory /cgi-bin/koha/tools/inventory.pl
8) Without patch you see "For loan" in items.notforloan for value 0
9) With patch you see "Available" in items.notforloan for value 0

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 6 Séverine Queune 2018-03-15 10:28:57 UTC
Everything work as expected playing with authorized_values and mapping for the concerned fields on bibliographic framework menu.
Comment 7 Katrin Fischer 2018-03-27 21:16:11 UTC
>8) Without patch you see "For loan" in items.notforloan for value 0
>9) With patch you see "Available" in items.notforloan for value 0

Still 'For loan' for me. Please fix test plan or patch!
Comment 8 Fridolin SOMERS 2018-03-30 06:53:20 UTC
(In reply to Katrin Fischer from comment #7)
> >8) Without patch you see "For loan" in items.notforloan for value 0
> >9) With patch you see "Available" in items.notforloan for value 0
> 
> Still 'For loan' for me. Please fix test plan or patch!

If you have memcached and/or plack, you need to restart them when you change authorized values.
Comment 9 Katrin Fischer 2018-04-01 11:05:40 UTC
(In reply to Fridolin SOMERS from comment #8)
> (In reply to Katrin Fischer from comment #7)
> > >8) Without patch you see "For loan" in items.notforloan for value 0
> > >9) With patch you see "Available" in items.notforloan for value 0
> > 
> > Still 'For loan' for me. Please fix test plan or patch!
> 
> If you have memcached and/or plack, you need to restart them when you change
> authorized values.

Hi Frido, that should not be necessary in current versions, but I am retesting anyway.
Comment 10 Katrin Fischer 2018-04-01 11:28:48 UTC
Hm, ok, I might not understand the reasoning behind this patch or even the bug it tries to fix :(

From my testing:

6) Create in authorized values category "NFL" a value 0 with description "Available"
7) Go to inventory /cgi-bin/koha/tools/inventory.pl
8) Without patch you see "For loan" in items.notforloan for value 0
9) With patch you see "Available" in items.notforloan for value 0

When I do that, I end up with 2 values showing on the inventory form: 
For loan and Available - both actually meaning the same.

Also: On the edit item form the Available appears twice(!) in the pull down and the X to empty the field is no longer present.

What I can see is an inconsistency on the inventory form as we only have a "0" value for not for loan, while we don't have that for lost, withdrawn and damaged. I'd suggest to either remove it  or add an empty option for all of them. But also not using 'for loan' - because it still might not be, but something like 'not set'. Or: Not lost, not withdrawn, not damaged,...
Comment 11 Fridolin SOMERS 2018-09-14 23:10:00 UTC
Created attachment 78830 [details] [review]
Bug 20131: Inventory optional filters correct "For loan" addition

In inventory, optional filters are build from authorize values categories used for items.notforloan, items.itemlost, items.withdrawn and items.damaged.

The bug is that description of value 0 is always "For loan".
This seems to be only for "notforloan".

Second bug is that if a description exists for "notforloan" value 0, it is not used.

This patch corrects by adding "For loan" only for "notforloan" authorized values and if there is no 0 value.
It changes the description added by code from "ignore" to a more unlikly user defined "__IGNORE__".

Test plan:
1) Make sure you have authorized values defined on items.notforloan, say "NFL" category, with at least a value and description but no 0 value
2) Make sure you have authorized values defined on items.itemlost, say "LOST" category, with 0 = "Not lost" and 1 = "Lost"
3) Go to inventory /cgi-bin/koha/tools/inventory.pl
4) Without patch you see "For loan" on both items.notforloan and items.itemlost
5) With patch you see "For loan" only in items.notforloan, you see for items.itemlost : "Not lost" and "Lost"
6) Create in authorized values category "NFL" a value 0 with description "Available"
7) Go to inventory /cgi-bin/koha/tools/inventory.pl
8) Without patch you see "For loan" in items.notforloan for value 0
9) With patch you see "Available" in items.notforloan for value 0

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 12 sandboxes@biblibre.com 2018-09-15 00:36:13 UTC
Patch tested with a sandbox, by Séverine QUEUNE <severine.queune@bulac.fr>
Comment 13 sandboxes@biblibre.com 2018-09-15 00:36:40 UTC
Created attachment 78851 [details] [review]
Bug 20131: Inventory optional filters correct "For loan" addition

In inventory, optional filters are build from authorize values categories used for items.notforloan, items.itemlost, items.withdrawn and items.damaged.

The bug is that description of value 0 is always "For loan".
This seems to be only for "notforloan".

Second bug is that if a description exists for "notforloan" value 0, it is not used.

This patch corrects by adding "For loan" only for "notforloan" authorized values and if there is no 0 value.
It changes the description added by code from "ignore" to a more unlikly user defined "__IGNORE__".

Test plan:
1) Make sure you have authorized values defined on items.notforloan, say "NFL" category, with at least a value and description but no 0 value
2) Make sure you have authorized values defined on items.itemlost, say "LOST" category, with 0 = "Not lost" and 1 = "Lost"
3) Go to inventory /cgi-bin/koha/tools/inventory.pl
4) Without patch you see "For loan" on both items.notforloan and items.itemlost
5) With patch you see "For loan" only in items.notforloan, you see for items.itemlost : "Not lost" and "Lost"
6) Create in authorized values category "NFL" a value 0 with description "Available"
7) Go to inventory /cgi-bin/koha/tools/inventory.pl
8) Without patch you see "For loan" in items.notforloan for value 0
9) With patch you see "Available" in items.notforloan for value 0

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 14 Chris Cormack 2018-10-10 00:29:18 UTC
Created attachment 80316 [details] [review]
Bug 20131: Inventory optional filters correct "For loan" addition

In inventory, optional filters are build from authorize values categories used for items.notforloan, items.itemlost, items.withdrawn and items.damaged.

The bug is that description of value 0 is always "For loan".
This seems to be only for "notforloan".

Second bug is that if a description exists for "notforloan" value 0, it is not used.

This patch corrects by adding "For loan" only for "notforloan" authorized values and if there is no 0 value.
It changes the description added by code from "ignore" to a more unlikly user defined "__IGNORE__".

Test plan:
1) Make sure you have authorized values defined on items.notforloan, say "NFL" category, with at least a value and description but no 0 value
2) Make sure you have authorized values defined on items.itemlost, say "LOST" category, with 0 = "Not lost" and 1 = "Lost"
3) Go to inventory /cgi-bin/koha/tools/inventory.pl
4) Without patch you see "For loan" on both items.notforloan and items.itemlost
5) With patch you see "For loan" only in items.notforloan, you see for items.itemlost : "Not lost" and "Lost"
6) Create in authorized values category "NFL" a value 0 with description "Available"
7) Go to inventory /cgi-bin/koha/tools/inventory.pl
8) Without patch you see "For loan" in items.notforloan for value 0
9) With patch you see "Available" in items.notforloan for value 0

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Comment 15 Nick Clemens 2018-10-15 16:59:12 UTC
Awesome work all!

Pushed to master for 18.11
Comment 16 Martin Renvoize 2018-10-17 10:46:54 UTC
Pushed to 18.05.x for 18.05.05
Comment 17 Fridolin SOMERS 2018-10-17 12:35:00 UTC
Pushed to 17.11.x for 17.11.11