Summary: | Remove get_opac_news_by_id | ||
---|---|---|---|
Product: | Koha | Reporter: | Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski> |
Component: | Architecture, internals, and plumbing | Assignee: | Jonathan Druart <jonathan.druart> |
Status: | RESOLVED FIXED | QA Contact: | Marcel de Rooy <m.de.rooy> |
Severity: | normal | ||
Priority: | P5 - low | CC: | david, fridolin.somers, jonathan.druart, lucas, m.de.rooy, wainuiwitikapark |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37684 | ||
GIT URL: | 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,23.11.11
|
Circulation function: | |
Bug Depends on: | 31383 | ||
Bug Blocks: | 37629, 40132 | ||
Attachments: |
Bug 37628: (follow-up) Remove get_opac_news_by_id
Bug 37628: (follow-up) Remove get_opac_news_by_id Bug 37628: (follow-up) Remove get_opac_news_by_id |
Description
Baptiste Wojtkowski (bwoj)
2024-08-13 14:24:01 UTC
Created attachment 170265 [details] [review] Bug 37628: (follow-up) Remove get_opac_news_by_id Keep the same behaviour, but wondering why we don't return 'content' when there is no match. ie. + is( $additional_contents, undef ); should certainly be + is( $additional_contents->{content}->count, 0 ); WNC amended patch - tidied Signed-off-by: Nick Clemens <nick@bywatersolutions.com> May I switch the state to need signoff since it was signed off by Nick ? I think better to have this tested again as the code base has moved quite a bit. Could you add a quick test plan? Hum... Unfortunately I am not quite sure of the exact purpose of this patch. This was a followup for https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31383 The thing is, I need this patch for https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37629 because it restores news search by ID, but the exact purpose is unclear to me and I found out that it was not applied while investigating on BZ37629. Let's wait for their author's response :) (In reply to Baptiste Wojtkowski (bwoj) from comment #5) > Hum... Unfortunately I am not quite sure of the exact purpose of this patch. > This was a followup for > https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31383 > > The thing is, I need this patch for > https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37629 because it > restores news search by ID, but the exact purpose is unclear to me and I > found out that it was not applied while investigating on BZ37629. It is code cleaning. It removes a sub and makes the code more robust. If no regression found it means that it works. It is a shame it has been missed. Created attachment 170317 [details] [review] Bug 37628: (follow-up) Remove get_opac_news_by_id Keep the same behaviour, but wondering why we don't return 'content' when there is no match. ie. + is( $additional_contents, undef ); should certainly be + is( $additional_contents->{content}->count, 0 ); WNC amended patch - tidied Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 170425 [details] [review] Bug 37628: (follow-up) Remove get_opac_news_by_id Keep the same behaviour, but wondering why we don't return 'content' when there is no match. ie. + is( $additional_contents, undef ); should certainly be + is( $additional_contents->{content}->count, 0 ); WNC amended patch - tidied Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Pushed for 24.11! Well done everyone, thank you! Backported to 24.05.x for upcoming 24.05.06 Pushed to 23.11.x for 23.11.11 Not backporting to 22.11 unless requested No updates to the manual required. |