Bug 28698 - News for all displays in all locations
Summary: News for all displays in all locations
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: 21.05
Hardware: All All
: P5 - low major (vote)
Assignee: Fridolin Somers
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-07-09 08:45 UTC by Fridolin Somers
Modified: 2021-12-29 22:44 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Text to go in the release notes:
This corrects the display of news items in the OPAC - if a location was not selected when creating a news item it was displaying in all locations (news, header, right, and so on). It also now displays in the right location for any language.
Version(s) released in:
21.05.08


Attachments
Bug 28698: Fix news for all displays in all locations (1.87 KB, patch)
2021-07-09 08:52 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 28698: Fix news for all displays in all locations (1.91 KB, patch)
2021-11-29 22:53 UTC, David Nind
Details | Diff | Splinter Review
Bug 28698: Fix news for all displays in all locations (1.97 KB, patch)
2021-12-17 15:29 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2021-07-09 08:45:40 UTC
Some news are used for custom text with a specific language, for example "opacheader".
The bug is that in these locations, news for all (staff and opac) are displayed.
This is because GetNewsToDisplay uses SQL : 
  opac_news.lang = '' OR opac_news.lang = ?
Comment 1 Fridolin Somers 2021-07-09 08:52:59 UTC
Created attachment 122698 [details] [review]
Bug 28698: Fix news for all displays in all locations

Some news are used for custom text with a specific language, for example "opacheader".
The bug is that in these locations, news for all (staff and opac) are displayed.
This is because GetNewsToDisplay uses SQL :
  opac_news.lang = '' OR opac_news.lang = ?

This patch is a quick fix.
This feature maybe needs a big revamping.

Test plan :
1) Remove any news
2) Create a news with empty 'Display location' and some content
3) Display OPAC interface
4) without patch you see in location of 'opacheader', 'opacnavright' ... the content
   of the news
5) With patch you see content of the news only in news table
6) Create a news for opacheader
7) Check it appears in correct language
Comment 2 Jonathan Druart 2021-07-09 11:49:34 UTC
Fridolin, bug 22544 removed C4::NewsChannels, and I think it fixes it.

Note that I am about to provide a patch on bug 24387 to clean that area a bit.
Comment 3 Fridolin Somers 2021-07-10 00:35:27 UTC
(In reply to Jonathan Druart from comment #2)
> Fridolin, bug 22544 removed C4::NewsChannels, and I think it fixes it.
I confirm.

> 
> Note that I am about to provide a patch on bug 24387 to clean that area a
> bit.
Ahhh super.

So I close.
Comment 4 Jonathan Druart 2021-07-12 08:42:42 UTC
Don't we need it for 21.05?
Comment 5 Lucas Gass 2021-08-09 17:21:25 UTC
(In reply to Jonathan Druart from comment #4)
> Don't we need it for 21.05?

Yes! I think this can be very useful for 21.05 and below.
Comment 6 David Nind 2021-11-26 21:27:06 UTC
I'm only seeing the news item displayed in the normal news location in the OPAC.

Do I need another language installed and enabled?
Comment 7 Fridolin Somers 2021-11-29 22:00:44 UTC
(In reply to David Nind from comment #6)
> I'm only seeing the news item displayed in the normal news location in the
> OPAC.
> 
> Do I need another language installed and enabled?

Did you test on master ?
This is only for <= 21.05.x
Comment 8 David Nind 2021-11-29 22:51:35 UTC
(In reply to Fridolin Somers from comment #7)

> Did you test on master ?
> This is only for <= 21.05.x

I tested in 21.05. I've retested and I'm seeing the problem now - I had to browse around a few pages, then it appeared in the places it shouldn't (but it didn't initially with the browser cache cleared).

Sign-off on the way!
Comment 9 David Nind 2021-11-29 22:53:20 UTC
Created attachment 128098 [details] [review]
Bug 28698: Fix news for all displays in all locations

Some news are used for custom text with a specific language, for example "opacheader".
The bug is that in these locations, news for all (staff and opac) are displayed.
This is because GetNewsToDisplay uses SQL :
  opac_news.lang = '' OR opac_news.lang = ?

This patch is a quick fix.
This feature maybe needs a big revamping.

Test plan :
1) Remove any news
2) Create a news with empty 'Display location' and some content
3) Display OPAC interface
4) without patch you see in location of 'opacheader', 'opacnavright' ... the content
   of the news
5) With patch you see content of the news only in news table
6) Create a news for opacheader
7) Check it appears in correct language

Signed-off-by: David Nind <david@davidnind.com>
Comment 10 Tomás Cohen Arazi 2021-12-17 15:29:11 UTC
Created attachment 128678 [details] [review]
Bug 28698: Fix news for all displays in all locations

Some news are used for custom text with a specific language, for example "opacheader".
The bug is that in these locations, news for all (staff and opac) are displayed.
This is because GetNewsToDisplay uses SQL :
  opac_news.lang = '' OR opac_news.lang = ?

This patch is a quick fix.
This feature maybe needs a big revamping.

Test plan :
1) Remove any news
2) Create a news with empty 'Display location' and some content
3) Display OPAC interface
4) without patch you see in location of 'opacheader', 'opacnavright' ... the content
   of the news
5) With patch you see content of the news only in news table
6) Create a news for opacheader
7) Check it appears in correct language

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 11 Andrew Fuerste-Henry 2021-12-22 12:18:56 UTC
Pushed to 21.05.x for 21.05.08
Comment 12 Victor Grousset/tuxayo 2021-12-23 00:47:27 UTC
This ticket depend on an enhancement from 21.11

But it was backported to 21.05 . Does that mean 20.11 should be affected as well?
Comment 13 Fridolin Somers 2021-12-23 06:33:47 UTC
Yes, this is only for <= 21.05.x
I removed dependency.
Comment 14 Katrin Fischer 2021-12-29 22:44:44 UTC
I can confirm the bug in 20.11 - we have seen it a few times when updating.