Bug 37686

Summary: render_resource_not_found() and render_resource_deleted() misses
Product: Koha Reporter: Tomás Cohen Arazi (tcohen) <tomascohen>
Component: REST APIAssignee: Tomás Cohen Arazi (tcohen) <tomascohen>
Status: CLOSED FIXED QA Contact: Matt Blenkinsop <matt.blenkinsop>
Severity: enhancement    
Priority: P5 - low CC: lucas, m.de.rooy, martin.renvoize, matt.blenkinsop, tomascohen
Version: unspecified   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37672
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:
Bug Depends on: 24857, 36495    
Bug Blocks: 37791    
Attachments: Bug 37686: Fix render_resource_not_found() and render_resource_deleted() misses
Bug 37686: Fix render_resource_not_found() and render_resource_deleted() misses
Bug 37686: Fix render_resource_not_found() and render_resource_deleted() misses

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.