Bug 37686 - render_resource_not_found() and render_resource_deleted() misses
Summary: render_resource_not_found() and render_resource_deleted() misses
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: Matt Blenkinsop
URL:
Keywords:
Depends on: 24857 36495
Blocks: 37791
  Show dependency treegraph
 
Reported: 2024-08-20 13:08 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2025-06-02 20:46 UTC (History)
5 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This development finished the code cleanup we implemented on bug 36495, by performing the same code changes in new code that was added in between and some misses too.
Version(s) released in:
24.11.00
Circulation function:


Attachments
Bug 37686: Fix render_resource_not_found() and render_resource_deleted() misses (16.65 KB, patch)
2024-08-20 14:03 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 37686: Fix render_resource_not_found() and render_resource_deleted() misses (16.67 KB, patch)
2024-08-21 09:10 UTC, David Nind
Details | Diff | Splinter Review
Bug 37686: Fix render_resource_not_found() and render_resource_deleted() misses (16.78 KB, patch)
2024-08-21 10:31 UTC, Matt Blenkinsop
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi (tcohen) 2024-08-20 13:08:25 UTC
Some bugs pushed later, and some misses, made bug 36495 incomplete. I filed bug 37672 when dealing with record sources, but there are more.
Comment 1 Tomás Cohen Arazi (tcohen) 2024-08-20 14:03:10 UTC
Created attachment 170498 [details] [review]
Bug 37686: Fix render_resource_not_found() and render_resource_deleted() misses

This patch refactors some small pieces of code to use the helpers as
prescribed on our coding guidelines [1] instead of manually crafting
responses.

To test:
1. Apply this patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/
=> SUCCESS: All tests pass
3. Sign off :-D

[1] https://wiki.koha-community.org/wiki/Coding_Guidelines_-_API#REST4:_Controller_code_.5BDRAFT.5D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 David Nind 2024-08-21 09:10:10 UTC
Created attachment 170541 [details] [review]
Bug 37686: Fix render_resource_not_found() and render_resource_deleted() misses

This patch refactors some small pieces of code to use the helpers as
prescribed on our coding guidelines [1] instead of manually crafting
responses.

To test:
1. Apply this patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/
=> SUCCESS: All tests pass
3. Sign off :-D

[1] https://wiki.koha-community.org/wiki/Coding_Guidelines_-_API#REST4:_Controller_code_.5BDRAFT.5D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Comment 3 Matt Blenkinsop 2024-08-21 10:31:27 UTC
Created attachment 170553 [details] [review]
Bug 37686: Fix render_resource_not_found() and render_resource_deleted() misses

This patch refactors some small pieces of code to use the helpers as
prescribed on our coding guidelines [1] instead of manually crafting
responses.

To test:
1. Apply this patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/
=> SUCCESS: All tests pass
3. Sign off :-D

[1] https://wiki.koha-community.org/wiki/Coding_Guidelines_-_API#REST4:_Controller_code_.5BDRAFT.5D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 4 Katrin Fischer 2024-08-27 09:54:33 UTC
What is it missing? (please update bug description :) )
Comment 5 Katrin Fischer 2024-08-29 15:34:41 UTC
I really wish you all would stop sneaking Biblio back in:
https://wiki.koha-community.org/wiki/Terminology#B

git grep "Biblio not found"
Koha/REST/V1/Holds.pm:                openapi => "Biblio not found."
api/v1/swagger/paths/biblios.yaml:        description: Biblio not found
api/v1/swagger/paths/biblios.yaml:        description: Biblio not found
api/v1/swagger/paths/biblios.yaml:        description: Biblio not found
api/v1/swagger/paths/biblios.yaml:        description: Biblio not found
api/v1/swagger/paths/biblios.yaml:        description: Biblio not found
api/v1/swagger/paths/biblios.yaml:        description: Biblio not found
api/v1/swagger/paths/biblios.yaml:        description: Biblio not found
api/v1/swagger/paths/biblios.yaml:        description: Biblio not found
api/v1/swagger/paths/biblios.yaml:        description: Biblio not found
api/v1/swagger/paths/biblios.yaml:        description: Biblio not found
api/v1/swagger/paths/biblios.yaml:        description: Biblio not found
api/v1/swagger/paths/biblios_merge.yaml:        description: Biblio not found
api/v1/swagger/paths/deleted_biblios.yaml:        description: Biblio not found
api/v1/swagger/paths/deleted_biblios.yaml:        description: Biblio not found
api/v1/swagger/paths/holds.yaml:        description: Biblio not found
api/v1/swagger/paths/items.yaml:        description: Biblio not found
misc/maintenance/UNIMARC_sync_date_created_with_marc_biblio.pl:        $debug and warn '[ERROR] Biblio not found.';
misc/maintenance/process_record_through_filter.pl:    print "Biblio not found\n";
t/db_dependent/api/v1/deleted_biblios.t:        ->json_is( '/error', 'Biblio not found' );
Comment 6 Katrin Fischer 2024-08-29 16:17:03 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 7 Tomás Cohen Arazi (tcohen) 2024-08-29 19:29:12 UTC
(In reply to Katrin Fischer from comment #5)
> I really wish you all would stop sneaking Biblio back in:
> https://wiki.koha-community.org/wiki/Terminology#B
> 
> git grep "Biblio not found"

Maybe that query is too broad heh. Happy to provide a follow-up tomorrow morning.
Comment 8 Katrin Fischer 2024-08-29 19:46:31 UTC
You only added one of these, I pondered fixing it.. then got discouraged by the number of occurences.
Comment 9 Katrin Fischer 2024-08-29 19:47:28 UTC
Actually you would need to fix:
+                return $c->render_resource_not_found("Biblio");
Comment 10 Tomás Cohen Arazi (tcohen) 2024-08-30 11:34:51 UTC
(In reply to Katrin Fischer from comment #9)
> Actually you would need to fix:
> +                return $c->render_resource_not_found("Biblio");

I've actually filed bug 37791.