Bug 28859

Summary: Table Settings should control Checked out by field in Checkout history
Product: Koha Reporter: Sally <sally.healey>
Component: System AdministrationAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: gmcharlt, jonathan.druart, katrin.fischer, kyle, martin.renvoize
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28858
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.11.00
Bug Depends on: 23916, 29218    
Bug Blocks:    
Attachments: Bug 28859: Fix missing column setting
Bug 28859: Fix missing column setting
Bug 28859: Correctly handle 'Checked out by' visibility
Bug 28859: Fix missing column setting
Bug 28859: Correctly handle 'Checked out by' visibility

Description Sally 2021-08-13 14:31:08 UTC
Koha Administration > Table Settings controls all whether fields are visible/can be toggled by users.  

'Checked out by' is solely controlled by a system preference (RecordStaffUserOnCheckout) - and is not an option in the Table Settings controls.

To replicate:

- Set system preference: RecordStaffUserOnCheckout to 'record'
- Check an item out to a patron
- Go to the item's Checkout history - cgi-bin/koha/catalogue/issuehistory.pl
- Note that 'Checked out by' column is visible
- Go to Koha > Administration > Table Settings
- Choose Catalog
- Choose Page: detail
- Choose Table id:  checkoutshistory-table
- Note that 'Checked out by' is not an option

This also has the effect of targeting the wrong field via Table Settings.

To replicate:

- Set system preference: RecordStaffUserOnCheckout to 'record'
- Check an item out to a patron
- Go to the item's Checkout history - cgi-bin/koha/catalogue/issuehistory.pl
- Note that 'Checked out by' column is visible
- Go to Koha > Administration > Table Settings
- Choose Catalog
- Choose Page: detail
- Choose Table id:  checkoutshistory-table
- Check some fields to be 'Is hidden by default' i.e. 'checked_out_from' and 'renewed'
- Reload the item's Checkout history
- Note that the columns 'Checked out from' and 'Checked out by' are hidden, but 'Renewed' is not

This is because 'Checked out by' is sitting in the original 'Renewed' position in the table.
Comment 1 Jonathan Druart 2021-08-16 10:48:52 UTC
Caused by
  commit eb283d13fab70d8a32216a5bee8fb4a71e6d875e
  Bug 23916: Record and display item issuer

Andrew, can you fix this please?
Comment 2 Jonathan Druart 2021-08-30 10:02:15 UTC
*** Bug 28916 has been marked as a duplicate of this bug. ***
Comment 3 Andrew Isherwood 2021-09-17 13:16:41 UTC
Created attachment 124986 [details] [review]
Bug 28859: Fix missing column setting

This commit adds the missing checked_out_by column to
columns_settings.yaml

Test plan:

- Follow the test plans in the bug description and note that the
symptoms are no longer seen
Comment 4 Owen Leonard 2021-09-17 14:03:36 UTC
Created attachment 124990 [details] [review]
Bug 28859: Fix missing column setting

This commit adds the missing checked_out_by column to
columns_settings.yaml

Test plan:

- Follow the test plans in the bug description and note that the
symptoms are no longer seen

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 5 Katrin Fischer 2021-10-02 21:37:56 UTC
I am sorry, but the table configuration is broken with this patch applied and the preference set to "don't record".
Comment 6 Katrin Fischer 2021-10-02 21:38:40 UTC
Uncaught TypeError: b.settings()[0].aoColumns[d] is undefined
Comment 7 Jonathan Druart 2021-10-13 08:28:40 UTC
Created attachment 126146 [details] [review]
Bug 28859: Correctly handle 'Checked out by' visibility

We must have the column in the table and let DT deal with the
visibility.

This patch hides the "Checked out by" column by default if
RecordStaffUserOnCheckout is off, but the DT column settings is aware of
its existence and the end user can still display the column.

IMO that's the most optimal situation considering both maintenance and ergonomic.
Comment 8 Jonathan Druart 2021-10-25 09:00:58 UTC
*** Bug 28858 has been marked as a duplicate of this bug. ***
Comment 9 Martin Renvoize 2021-10-28 08:30:18 UTC
Created attachment 127030 [details] [review]
Bug 28859: Fix missing column setting

This commit adds the missing checked_out_by column to
columns_settings.yaml

Test plan:

- Follow the test plans in the bug description and note that the
symptoms are no longer seen

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 10 Martin Renvoize 2021-10-28 08:30:22 UTC
Created attachment 127031 [details] [review]
Bug 28859: Correctly handle 'Checked out by' visibility

We must have the column in the table and let DT deal with the
visibility.

This patch hides the "Checked out by" column by default if
RecordStaffUserOnCheckout is off, but the DT column settings is aware of
its existence and the end user can still display the column.

IMO that's the most optimal situation considering both maintenance and ergonomic.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize 2021-10-28 08:31:59 UTC
I tested the whole bug series together (and separately for the individual QA's).

This all works as expected now, thanks for working through it Jonathan.

Passing QA
Comment 12 Jonathan Druart 2021-10-28 15:44:53 UTC
Pushed to master for 21.11, thanks to everybody involved!