Bug 37629 - Link to news are broken
Summary: Link to news are broken
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Baptiste Wojtkowski (bwoj)
QA Contact: Marcel de Rooy
URL:
Keywords: regression
Depends on: 37628
Blocks: 37684
  Show dependency treegraph
 
Reported: 2024-08-13 14:37 UTC by Baptiste Wojtkowski (bwoj)
Modified: 2024-11-18 21:49 UTC (History)
3 users (show)

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


Attachments
Bug 37629: Link to news are broken (1.73 KB, patch)
2024-08-13 14:55 UTC, Baptiste Wojtkowski (bwoj)
Details | Diff | Splinter Review
Bug 37629: Link to news are broken (1.79 KB, patch)
2024-08-14 12:34 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 37629: Link to news are broken (1.89 KB, patch)
2024-08-16 06:45 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Baptiste Wojtkowski (bwoj) 2024-08-13 14:37:25 UTC
When browsing news in the opac (opac-main.pl), links in the titles of news are broken. The id pasted in the link is the id from the additional_contents_localizations instead of the id from additional_contents.
Comment 1 Baptiste Wojtkowski (bwoj) 2024-08-13 14:49:00 UTC
This behaviour is due to the behavior of Koha::AdditionnalContents->search_for_display

It calls : Koha::AdditionalContentsLocalizations->search( $search_params, $attribs ), which returns a set of AdditionalContentsLocalizationsn which are very close to AdditionnalContents and whose interface allows to deal with them as if they were AdditionnalContents EXCEPT that the field "id" contains the id of the AdditionalContentsLocalizations.

Hence, when searching for the id of the news, one should use the field AdditionalContentLocalization->additional_content->id.


Note : the fix I am about to propose fixes the issue but I am not sure about the function search_for_display itself : the function AdditionnalContent::search_for_display() should return objects with the exact same interface as an AdditionnalContent, thus an AdditionnalContent.id that allows to search and find for an additional_content.
Moreover it looks like it is a static method declared as an object method since $self is declared as parameter but never used.

Do you think I should open a bug to fix that method or are these points normal ?
Comment 2 Baptiste Wojtkowski (bwoj) 2024-08-13 14:55:14 UTC
Created attachment 170266 [details] [review]
Bug 37629: Link to news are broken

When browsing news in the opac (opac-main.pl), links in the titles of news are broken. The id pasted in the link is the id from the additional_contents_localizations instead of the id from additional_contents.

TEST PLAN:
1 - In intranet create a news with multiple languages
2 - Create a second news
3 - In OPAC, the link of the title of the second news (and maybe the first) should be broken
APPLY PATCH
4 - In OPAC, the link of the title of both news should work
Comment 3 Owen Leonard 2024-08-14 12:34:02 UTC
Created attachment 170318 [details] [review]
Bug 37629: Link to news are broken

When browsing news in the opac (opac-main.pl), links in the titles of
news are broken. The id pasted in the link is the id from the
additional_contents_localizations instead of the id from
additional_contents.

TEST PLAN:
1 - In intranet create a news with multiple languages
2 - Create a second news
3 - In OPAC, the link of the title of the second news (and maybe the
    first) should be broken APPLY PATCH
4 - In OPAC, the link of the title of both news should work

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 4 Marcel de Rooy 2024-08-16 06:45:05 UTC
Created attachment 170426 [details] [review]
Bug 37629: Link to news are broken

When browsing news in the opac (opac-main.pl), links in the titles of
news are broken. The id pasted in the link is the id from the
additional_contents_localizations instead of the id from
additional_contents.

TEST PLAN:
1 - In intranet create a news with multiple languages
2 - Create a second news
3 - In OPAC, the link of the title of the second news (and maybe the
    first) should be broken APPLY PATCH
4 - In OPAC, the link of the title of both news should work

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 5 Katrin Fischer 2024-08-16 15:37:54 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 6 Lucas Gass (lukeg) 2024-11-18 21:49:41 UTC
Backported to 24.05.x for upcoming 24.05.06