Bug 30406 - Our DT tables not filtering on the correct column if hidden by default
Summary: Our DT tables not filtering on the correct column if hidden by default
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Julian Maurice
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-03-30 10:12 UTC by Jonathan Druart
Modified: 2022-04-25 20:54 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:
22.05.00,21.11.05,21.05.14


Attachments
Bug 30406: Add column filters before hiding columns (2.24 KB, patch)
2022-04-01 08:50 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 30406: Add column filters before hiding columns (2.38 KB, patch)
2022-04-04 08:39 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 30406: Add column filters before hiding columns (2.46 KB, patch)
2022-04-05 12:06 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 Jonathan Druart 2022-03-30 10:12:14 UTC
To recreate, hide city_state for the cities table, create some cities and notice that using the "Country" column filter will actually filter on the hidden "State" column (the one on the left)

Coming from bug 30063 comment 23 (point 5)
Comment 1 Julian Maurice 2022-04-01 08:50:57 UTC
Created attachment 132830 [details] [review]
Bug 30406: Add column filters before hiding columns

The code that adds column filters works only on visible columns, so we
should hide columns after filters are created

Test plan:
1. Apply patch
2. Go to Tables settings. Hide city_state for the cities table.
3. Create some cities and go to the cities page
4. Verify that filtering works as expected.
5. Make the city_state column appear. Verify that filtering on this
   column works too
Comment 2 Séverine Queune 2022-04-04 08:39:41 UTC
Created attachment 132921 [details] [review]
Bug 30406: Add column filters before hiding columns

The code that adds column filters works only on visible columns, so we
should hide columns after filters are created

Test plan:
1. Apply patch
2. Go to Tables settings. Hide city_state for the cities table.
3. Create some cities and go to the cities page
4. Verify that filtering works as expected.
5. Make the city_state column appear. Verify that filtering on this
   column works too

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 3 Séverine Queune 2022-04-04 08:42:16 UTC
I never thought it would be so easily fixed !
Well done Julian :)
Comment 4 Julian Maurice 2022-04-04 09:52:50 UTC
(In reply to Séverine Queune from comment #3)
> I never thought it would be so easily fixed !
> Well done Julian :)

Me neither :)
Thanks for testing!
Comment 5 Jonathan Druart 2022-04-05 12:06:20 UTC
Created attachment 132959 [details] [review]
Bug 30406: Add column filters before hiding columns

The code that adds column filters works only on visible columns, so we
should hide columns after filters are created

Test plan:
1. Apply patch
2. Go to Tables settings. Hide city_state for the cities table.
3. Create some cities and go to the cities page
4. Verify that filtering works as expected.
5. Make the city_state column appear. Verify that filtering on this
   column works too

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 6 Jonathan Druart 2022-04-05 12:07:48 UTC
Ha! Good workaround, thanks Julian!

We could apply the same fix to includes/columns_settings.inc but I didn't manage to recreate the problem on catalogue/detail.pl (I think it's the only place where column filters are used with KohaTable).
Comment 7 Fridolin Somers 2022-04-13 13:57:50 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 8 Kyle M Hall 2022-04-15 11:36:55 UTC
Pushed to 21.11.x for 21.11.05
Comment 9 Andrew Fuerste-Henry 2022-04-25 20:39:11 UTC
Pushed to 21.05.x for 21.05.14
Comment 10 Victor Grousset/tuxayo 2022-04-25 20:54:56 UTC
Not backported to oldoldstable (20.11.x). Feel free to ask if it's needed.