Bug 29962 - Table of items on item edit page missing columns button
Summary: Table of items on item edit page missing columns button
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 27526
Blocks:
  Show dependency treegraph
 
Reported: 2022-01-27 17:43 UTC by Owen Leonard
Modified: 2022-12-12 21:23 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.05.00,21.11.03


Attachments
Bug 29962: (bug 27526 follow-up): Fix columns button on item list (2.23 KB, patch)
2022-01-28 08:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 29962: (bug 27526 follow-up): Fix columns button on item list (2.28 KB, patch)
2022-02-14 23:03 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 29962: (bug 27526 follow-up): Fix columns button on item list (2.34 KB, patch)
2022-02-18 16:46 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2022-01-27 17:43:59 UTC
On the item edit page, the table of items is missing a "Columns" button.

Git bisect blames Bug 27526
Comment 1 Jonathan Druart 2022-01-28 08:55:26 UTC
Created attachment 129919 [details] [review]
Bug 29962: (bug 27526 follow-up): Fix columns button on item list

The "Columns" button on the item list view has been broken (hidden) by
bug 27526.
"column_name" was removed from the item_header_loop:
-    if ( $kohafield && $kohafield =~ /items.(.+)/ ) {
-        $header_value{column_name} = $1;
-    }

We can still access the attribute name using 'attribute' however.
The difference is that non-mapped subfield will have an "attribute", but
there is a net to prevent a crash in columns_settings.inc:
 12         var named_id = $( 'thead th[data-colname="' + this.columnname + '"]', selector ).index( selector + ' th' );
 13         var used_id = dt_parameters.bKohaColumnsUseNames ? named_id : counter;
 14         if ( used_id == -1 ) return;

We should simply pass attribute.

Test plan:
Create a non-mapped subfield (952$k)
Edit an item, add a value for k
On the item table at the top of the edit form notice that the "Columns"
button is back and that it's working correctly.
Note that 'k' cannot be hidden (may be considered as a bug, but it's not
a regression).
Comment 2 Lucas Gass 2022-02-11 23:12:20 UTC
This patch works and restores the Columns button. However the order of the columns is wrong in the tool is wrong. This is a bit confusing.

On every other page I looked at with a Columns button, the order in the tool is the same as the order in the table.
Comment 3 Jonathan Druart 2022-02-14 08:11:18 UTC
(In reply to Lucas Gass from comment #2)
> This patch works and restores the Columns button. However the order of the
> columns is wrong in the tool is wrong. This is a bit confusing.
> 
> On every other page I looked at with a Columns button, the order in the tool
> is the same as the order in the table.

I would report this on a separate bug report. This one is restoring a regression from bug 27526.
The way we deal with the table settings is different on this view, we pass the bKohaColumnsUseNames flag to KohaTable: we won't display all the columns defined in the yaml file, only those that have at least a non-empty value.
Comment 4 Lucas Gass 2022-02-14 23:03:59 UTC
Created attachment 130584 [details] [review]
Bug 29962: (bug 27526 follow-up): Fix columns button on item list

The "Columns" button on the item list view has been broken (hidden) by
bug 27526.
"column_name" was removed from the item_header_loop:
-    if ( $kohafield && $kohafield =~ /items.(.+)/ ) {
-        $header_value{column_name} = $1;
-    }

We can still access the attribute name using 'attribute' however.
The difference is that non-mapped subfield will have an "attribute", but
there is a net to prevent a crash in columns_settings.inc:
 12         var named_id = $( 'thead th[data-colname="' + this.columnname + '"]', selector ).index( selector + ' th' );
 13         var used_id = dt_parameters.bKohaColumnsUseNames ? named_id : counter;
 14         if ( used_id == -1 ) return;

We should simply pass attribute.

Test plan:
Create a non-mapped subfield (952$k)
Edit an item, add a value for k
On the item table at the top of the edit form notice that the "Columns"
button is back and that it's working correctly.
Note that 'k' cannot be hidden (may be considered as a bug, but it's not
a regression).

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 5 Nick Clemens 2022-02-18 16:46:16 UTC
Created attachment 130877 [details] [review]
Bug 29962: (bug 27526 follow-up): Fix columns button on item list

The "Columns" button on the item list view has been broken (hidden) by
bug 27526.
"column_name" was removed from the item_header_loop:
-    if ( $kohafield && $kohafield =~ /items.(.+)/ ) {
-        $header_value{column_name} = $1;
-    }

We can still access the attribute name using 'attribute' however.
The difference is that non-mapped subfield will have an "attribute", but
there is a net to prevent a crash in columns_settings.inc:
 12         var named_id = $( 'thead th[data-colname="' + this.columnname + '"]', selector ).index( selector + ' th' );
 13         var used_id = dt_parameters.bKohaColumnsUseNames ? named_id : counter;
 14         if ( used_id == -1 ) return;

We should simply pass attribute.

Test plan:
Create a non-mapped subfield (952$k)
Edit an item, add a value for k
On the item table at the top of the edit form notice that the "Columns"
button is back and that it's working correctly.
Note that 'k' cannot be hidden (may be considered as a bug, but it's not
a regression).

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 6 Fridolin Somers 2022-02-22 01:14:33 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 7 Kyle M Hall 2022-02-25 13:32:35 UTC
Pushed to 21.11.x for 21.11.03
Comment 8 Andrew Fuerste-Henry 2022-03-07 21:09:34 UTC
Missing dependency, not backported to 21.05