Bug 20806

Summary: Item type in holds history table should be written as description, not code
Product: Koha Reporter: Séverine Queune <severine.queune>
Component: PatronsAssignee: Josef Moravec <josef.moravec>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: minor    
Priority: P5 - low CC: fridolin.somers, gmcharlt, jonathan.druart, josef.moravec, katrin.fischer, kyle.m.hall, martin.renvoize, nick
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 14919, 20831    
Bug Blocks: 20936    
Attachments: Bug 20806: Add item type description holds history page
Bug 20806: Add item type description holds history page
Bug 20806: Add item type description holds history page
Bug 20806: Add item type description holds history page
Bug 20806: Do not rely on column position
Bug 20806: Surround string by span tag to help translation

Description Séverine Queune 2018-05-23 13:42:47 UTC
There is 2 display issues with the table "Holds history" tab :
   - no change when modifying "Column settings > Patrons > (id=holdshistory-table)". I don't know if it is linked, but the <table id> on the tab is "table_holdshistory".
  - column "Item type" is always empty
Comment 1 Josef Moravec 2018-05-29 08:43:51 UTC
(In reply to Séverine Queune from comment #0)
> There is 2 display issues with the table "Holds history" tab :
>    - no change when modifying "Column settings > Patrons >
> (id=holdshistory-table)". I don't know if it is linked, but the <table id>
> on the tab is "table_holdshistory".

This is caused by bug 9573, where the first parameter of KohaTable function was changed from selector to id

>   - column "Item type" is always empty
Comment 2 Josef Moravec 2018-05-29 11:52:26 UTC
Created attachment 75625 [details] [review]
Bug 20806: Add item type description holds history page

Test plan:

0) Do not apply the patch
1) Set AllowHoldItemTypeSelection to Allow
2) Place some hold for a patron, some with specific item type and some
without item typ
3) On holds history page you should see blank or item type code in item
type column
4) Apply the patch
5) Now you should see description in item type column
Comment 3 Séverine Queune 2018-05-29 12:44:46 UTC
I'm  quite confused...
If I understand correctly the patch, the (new) column "Request item type" is filled with the hold level.
Personally, I though to see the 'items.itype' value. As we only authorised item level holds because of our local use, it would have been more useful because with this patch we will only have "Any item itype" displayed.

Can you confirm this is the behaviour you were waiting for ?
If so, as patch applies correctly, I will sign it.

PS : sorting option for the table is a good idea !
Comment 4 Jonathan Druart 2018-05-31 15:43:49 UTC
Séverine is right, the column is confusing.

Maybe we should display it only when AllowHoldItemTypeSelection is set?
Comment 5 Katrin Fischer 2018-06-02 21:56:55 UTC
I think this might be related to AllowHoldItemTypeSelection that allows you to pick an itemtype when placing the hold? Maybe we can make the visibility of the column depend on that?
Comment 6 Katrin Fischer 2018-06-03 19:36:12 UTC
... which is exactly what Jonathan suggested.
Comment 7 Katrin Fischer 2018-06-19 21:29:15 UTC
I think with the change to "Requested item type" the patch makes sense to me. Only thing I'd is a check for the AllowHoldItemTypeSelection system preference to decide about showing it or not.
Comment 8 Josef Moravec 2018-06-25 11:55:22 UTC
Created attachment 76370 [details] [review]
Bug 20806: Add item type description holds history page

Test plan:

0) Do not apply the patch
1) Set AllowHoldItemTypeSelection to Allow
2) Place some hold for a patron, some with specific item type and some
without item type
3) On holds history page you should see blank or item type code in item
type column
4) Apply the patch
5) Now you should see description in item type column
6) Set AllowHoldItemTypeSelection to Don't Allow
7) You shold not see the item type column, but column visibility
settings should continue working ;)
Comment 9 Josef Moravec 2018-06-25 11:56:36 UTC
Thanks all for comments/suggestions. I added the check of AllowHoldItemTypeSelection system preference
Comment 10 Maryse Simard 2018-06-29 19:36:01 UTC
Created attachment 76585 [details] [review]
Bug 20806: Add item type description holds history page

Test plan:

0) Do not apply the patch
1) Set AllowHoldItemTypeSelection to Allow
2) Place some hold for a patron, some with specific item type and some
without item type
3) On holds history page you should see blank or item type code in item
type column
4) Apply the patch
5) Now you should see description in item type column
6) Set AllowHoldItemTypeSelection to Don't Allow
7) You shold not see the item type column, but column visibility
settings should continue working ;)

Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>
Followed the test plan and it works.
Comment 11 Jonathan Druart 2018-06-29 20:31:55 UTC
Created attachment 76591 [details] [review]
Bug 20806: Add item type description holds history page

Test plan:

0) Do not apply the patch
1) Set AllowHoldItemTypeSelection to Allow
2) Place some hold for a patron, some with specific item type and some
without item type
3) On holds history page you should see blank or item type code in item
type column
4) Apply the patch
5) Now you should see description in item type column
6) Set AllowHoldItemTypeSelection to Don't Allow
7) You shold not see the item type column, but column visibility
settings should continue working ;)

Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>
Followed the test plan and it works.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 12 Jonathan Druart 2018-06-29 20:31:59 UTC
Created attachment 76592 [details] [review]
Bug 20806: Do not rely on column position

It is not the first time we have this splice call, it seems safer to
rely on the column's name instead of the column's position

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 13 Jonathan Druart 2018-06-29 20:37:14 UTC
Created attachment 76593 [details] [review]
Bug 20806: Surround string by span tag to help translation

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 14 Nick Clemens 2018-07-18 19:16:24 UTC
Awesome work all!

Pushed to master for 18.11
Comment 15 Martin Renvoize 2018-08-02 12:16:41 UTC
Pushed to 18.05.x for 18.05.03
Comment 16 Fridolin Somers 2018-09-21 07:24:51 UTC
Depends on Bug 20831  not in 17.11.x