Bug 28859 - Table Settings should control Checked out by field in Checkout history
Summary: Table Settings should control Checked out by field in Checkout history
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
: 28858 28916 (view as bug list)
Depends on: 29218 23916
Blocks:
  Show dependency treegraph
 
Reported: 2021-08-13 14:31 UTC by Sally
Modified: 2021-10-29 16:17 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Text to go in the release notes:
Version(s) released in:
21.11.00


Attachments
Bug 28859: Fix missing column setting (906 bytes, patch)
2021-09-17 13:16 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 28859: Fix missing column setting (957 bytes, patch)
2021-09-17 14:03 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 28859: Correctly handle 'Checked out by' visibility (2.57 KB, patch)
2021-10-13 08:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28859: Fix missing column setting (1022 bytes, patch)
2021-10-28 08:30 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28859: Correctly handle 'Checked out by' visibility (2.63 KB, patch)
2021-10-28 08:30 UTC, Martin Renvoize
Details | Diff | Splinter Review

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