| 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: | Pushed to main --- | QA Contact: | Marcel de Rooy <m.de.rooy> |
| Severity: | normal | ||
| Priority: | P5 - low | CC: | jonathan.druart, m.de.rooy |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37684 | ||
| Change sponsored?: | --- | Patch complexity: | Small patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: |
24.11.00
|
|
| Circulation function: | |||
| Bug Depends on: | 31383 | ||
| Bug Blocks: | 37629 | ||
| 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! |