Bug 38927 - Unecessary call to FindDuplicate if x-confirm-not-duplicate is passed to POST /biblios
Summary: Unecessary call to FindDuplicate if x-confirm-not-duplicate is passed to POST...
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: Paul Derscheid
URL:
Keywords:
Depends on: 31800
Blocks:
  Show dependency treegraph
 
Reported: 2025-01-20 11:52 UTC by Jonathan Druart
Modified: 2025-01-24 11:03 UTC (History)
2 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:
25.05.00
Circulation function:


Attachments
Bug 38927: Only call FindDuplicate if required (1.92 KB, patch)
2025-01-20 13:31 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 38927: Only call FindDuplicate if required (1.98 KB, patch)
2025-01-20 15:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38927: Only call FindDuplicate if required (2.04 KB, patch)
2025-01-20 17:49 UTC, Paul Derscheid
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2025-01-20 11:52:11 UTC
695         my ( $duplicatebiblionumber, $duplicatetitle );
696             ( $duplicatebiblionumber, $duplicatetitle ) = FindDuplicate($record);
697 
698         my $confirm_not_duplicate = $headers->header('x-confirm-not-duplicate');
699 
700         return $c->render(
701             status  => 400,
702             openapi => {
703                 error => "Duplicate biblio $duplicatebiblionumber",
704             }
705         ) unless !$duplicatebiblionumber || $confirm_not_duplicate;

We don't need to call FindDuplicate if the header is set.
Comment 1 Tomás Cohen Arazi (tcohen) 2025-01-20 13:26:56 UTC
Nice finding.
Comment 2 Tomás Cohen Arazi (tcohen) 2025-01-20 13:31:48 UTC
Created attachment 176806 [details] [review]
Bug 38927: Only call FindDuplicate if required

This patch makes the code call `FindDuplicate` only if
`x-confirm-not-duplicate` is not passed. This makes sense, as having it
passed makes it override any duplicate found.

This patch doesn't change any end user behavior, just makes the code
more efficient.

To test:
1. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/biblios.t
=> SUCCESS: Tests pass
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass, no behavior change
4. Sign off :-D
Comment 3 Jonathan Druart 2025-01-20 15:32:46 UTC
Created attachment 176825 [details] [review]
Bug 38927: Only call FindDuplicate if required

This patch makes the code call `FindDuplicate` only if
`x-confirm-not-duplicate` is not passed. This makes sense, as having it
passed makes it override any duplicate found.

This patch doesn't change any end user behavior, just makes the code
more efficient.

To test:
1. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/biblios.t
=> SUCCESS: Tests pass
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass, no behavior change
4. Sign off :-D

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 4 Paul Derscheid 2025-01-20 17:49:51 UTC
Created attachment 176837 [details] [review]
Bug 38927: Only call FindDuplicate if required

This patch makes the code call `FindDuplicate` only if
`x-confirm-not-duplicate` is not passed. This makes sense, as having it
passed makes it override any duplicate found.

This patch doesn't change any end user behavior, just makes the code
more efficient.

To test:
1. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/biblios.t
=> SUCCESS: Tests pass
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass, no behavior change
4. Sign off :-D

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 5 Katrin Fischer 2025-01-24 10:41:49 UTC
Hm, this reads a little confusing:

+        if ( !$confirm_not_duplicate ) {
+            my ( $duplicatebiblionumber, $duplicatetitle ) = FindDuplicate($record);

But after checking docs: confirm_not_duplicate doesn't mean "Koha, confirm this is not a duplicate" but "API user says this is not a duplicate".

OK.
Comment 6 Katrin Fischer 2025-01-24 11:03:54 UTC
Pushed for 25.05!

Well done everyone, thank you!