Bug 38927

Summary: Unecessary call to FindDuplicate if x-confirm-not-duplicate is passed to POST /biblios
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: REST APIAssignee: Tomás Cohen Arazi (tcohen) <tomascohen>
Status: Pushed to main --- QA Contact: Paul Derscheid <paul.derscheid>
Severity: normal    
Priority: P5 - low CC: paul.derscheid, tomascohen
Version: unspecified   
Hardware: All   
OS: All   
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:
Bug Depends on: 31800    
Bug Blocks:    
Attachments: Bug 38927: Only call FindDuplicate if required
Bug 38927: Only call FindDuplicate if required
Bug 38927: Only call FindDuplicate if required

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!