Bug 37686 - render_resource_not_found() and render_resource_deleted() misses
Summary: render_resource_not_found() and render_resource_deleted() misses
Status: Pushed to main
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: 2024-08-30 11:35 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
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.