Bug 20806 - Item type in holds history table should be written as description, not code
Summary: Item type in holds history table should be written as description, not code
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Josef Moravec
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 14919 20831
Blocks: 20936
  Show dependency treegraph
 
Reported: 2018-05-23 13:42 UTC by Séverine Queune
Modified: 2018-09-21 07:24 UTC (History)
8 users (show)

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


Attachments
Bug 20806: Add item type description holds history page (1.97 KB, patch)
2018-05-29 11:52 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20806: Add item type description holds history page (3.06 KB, patch)
2018-06-25 11:55 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20806: Add item type description holds history page (3.16 KB, patch)
2018-06-29 19:36 UTC, Maryse Simard
Details | Diff | Splinter Review
Bug 20806: Add item type description holds history page (3.23 KB, patch)
2018-06-29 20:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20806: Do not rely on column position (1.39 KB, patch)
2018-06-29 20:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20806: Surround string by span tag to help translation (1.04 KB, patch)
2018-06-29 20:37 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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