Bug 38844 - Display location filter breaks News
Summary: Display location filter breaks News
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2025-01-07 18:23 UTC by Christopher Brannon
Modified: 2025-02-28 07:55 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 38844: Fix location filter for News html customizations (3.00 KB, patch)
2025-02-27 17:54 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 38844: Fix location filter for News html customizations (3.12 KB, patch)
2025-02-27 18:02 UTC, Brendan Lawlor
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Christopher Brannon 2025-01-07 18:23:39 UTC
When looking at News, if you change the display location filter to anything except all, nothing shows.  This filter appears to be broken.
Comment 1 Katrin Fischer 2025-01-08 08:49:32 UTC
Hi Christopher, were you logged in when looking at the page?
Comment 2 Jason Robb 2025-01-08 17:49:42 UTC
Can confirm this is happening on our system and main as well.

To replicate:

1. Go to Tools > News
2. Add a news item for each of the location types (Staff interface and OPAC, Staff interface, OPAC, Slip)
3. Go back to the News landing page
4. Try using the "Display location" filter in the left column to filter the table of news items - only the "All" and "Slip" choices work correctly.
Comment 3 Katrin Fischer 2025-01-08 17:58:21 UTC
Ah! I was totally in the wrong place. Thanks for explaining!
Comment 4 Brendan Lawlor 2025-02-20 21:15:03 UTC
This seems to be an issue with the data table filter not liking snake case that is used in additional_contents for location for staff_only, opac_only and staff_and_opac.

For other HTML customizations that work the location is in camel case like CookieConsentPopup.

Not sure if we can do something to have the datatable filter escape the _ or if we should change the the snake case to camel case wherever it's required.

Another possible solution would be to have the filter use the option's text instead of its value as is done for the Library column for some reason.
Comment 5 Brendan Lawlor 2025-02-27 17:54:36 UTC Comment hidden (obsolete)
Comment 6 Brendan Lawlor 2025-02-27 18:02:12 UTC
Created attachment 178805 [details] [review]
Bug 38844: Fix location filter for News html customizations

This patch changes the #news_display_location filter to use the option's
text instead of it's value. The filter previously did not work when the
location field in additional_contents used snake case as in staff_only.

Test plan:
1. Go to Tools > News
2. Add a news item for each of the location types (Staff interface and OPAC, Staff interface, OPAC, Slip)
3. Go back to the News landing page
4. Try using the "Display location" filter in the left column to filter the table of news items - only the "All" and "Slip" choices work correctly.
5. Apply patch
6. Confirm the "Display location" filter works for All, Staff interface and OPAC, Staff interface, OPAC, and Slip
Comment 7 Magnus Enger 2025-02-28 07:55:29 UTC
Can't get this to work, sorry. 

- Applied the patch
- yarn js:build
- restart_all
- Tested in a regular window and am incognito window

The table shows news for all the different locations. Changing the "Display location" dropdown does not affect the table at all, all items are always displayed. 

I see this in the console: 

additional-contents.pl:2057 Uncaught TypeError: table.column is not a function
    at filterDataTable (additional-contents.pl:2057:27)
    at HTMLSelectElement.<anonymous> (additional-contents.pl:2133:21)
    at HTMLSelectElement.dispatch (jquery-3.6.0.min_24.1200015.js:2:43064)
    at v.handle (jquery-3.6.0.min_24.1200015.js:2:41048)