Bug 39007 - Add last_audit to the sushi_service API spec
Summary: Add last_audit to the sushi_service API spec
Status: Pushed to oldoldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Matt Blenkinsop
QA Contact: Katrin Fischer
URL:
Keywords: additional_work_needed
Depends on:
Blocks:
 
Reported: 2025-01-31 13:09 UTC by Matt Blenkinsop
Modified: 2025-02-27 19:55 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
25.05.00,24.05.07,23.11.12
Circulation function:


Attachments
Bug 39007: Update API spec and unit test (2.14 KB, patch)
2025-01-31 13:15 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 39007: Update API spec and unit test (2.33 KB, patch)
2025-01-31 13:16 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 39007: Update API spec and unit test (2.38 KB, patch)
2025-01-31 13:51 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 39007: (follow-up) Fix typo (1.12 KB, patch)
2025-01-31 14:52 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 39007: Replace warning_like with is (1.34 KB, patch)
2025-02-03 08:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 39007: [24.05] Update API spec and unit test (3.04 KB, patch)
2025-02-13 13:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 39007: [24.05] (follow-up) Fix typo (1.13 KB, patch)
2025-02-13 13:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 39007: [24.05] Replace warning_like with is (1.34 KB, patch)
2025-02-13 13:20 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Matt Blenkinsop 2025-01-31 13:09:28 UTC
A new field has been added to the response object for the sushi service API, this needs updating in our specs
Comment 1 Matt Blenkinsop 2025-01-31 13:15:15 UTC
Created attachment 177381 [details] [review]
Bug 39007: Update API spec and unit test
Comment 2 Matt Blenkinsop 2025-01-31 13:16:48 UTC
Created attachment 177382 [details] [review]
Bug 39007: Update API spec and unit test

Test plan:
1) Run prove t/db_dependent/api/v1/erm_sushi_services.t - FAIL
2) Apply patch
3) Repeat step 1 - PASS
4) Inspect patch diff and note that last_audit has been added to the API definition
Comment 3 Paul Derscheid 2025-01-31 13:51:56 UTC
Created attachment 177390 [details] [review]
Bug 39007: Update API spec and unit test

Test plan:
1) Run prove t/db_dependent/api/v1/erm_sushi_services.t - FAIL
2) Apply patch
3) Repeat step 1 - PASS
4) Inspect patch diff and note that last_audit has been added to the API definition

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 4 Katrin Fischer 2025-01-31 14:34:35 UTC
Fixed inline:

 FAIL	t/db_dependent/api/v1/erm_sushi_services.t
   FAIL	  spelling
		 repsonse ==> response

Could you confirm if the QA script is showing this fail for you or not? I have been suspecting it might depend on some setup.
Comment 5 Katrin Fischer 2025-01-31 14:41:41 UTC
Thanks for the quick work! Picking this up for main directly.
Comment 6 Matt Blenkinsop 2025-01-31 14:52:09 UTC
Created attachment 177392 [details] [review]
Bug 39007: (follow-up) Fix typo
Comment 7 Matt Blenkinsop 2025-01-31 14:52:44 UTC
(In reply to Katrin Fischer from comment #4)
> Fixed inline:
> 
>  FAIL	t/db_dependent/api/v1/erm_sushi_services.t
>    FAIL	  spelling
> 		 repsonse ==> response
> 
> Could you confirm if the QA script is showing this fail for you or not? I
> have been suspecting it might depend on some setup.

Its a typo, added a follow-up, feel free to squash it if you haven't already pushed the first patch to main
Comment 8 Katrin Fischer 2025-01-31 14:57:12 UTC
(In reply to Matt Blenkinsop from comment #7)
> (In reply to Katrin Fischer from comment #4)
> > Fixed inline:
> > 
> >  FAIL	t/db_dependent/api/v1/erm_sushi_services.t
> >    FAIL	  spelling
> > 		 repsonse ==> response
> > 
> > Could you confirm if the QA script is showing this fail for you or not? I
> > have been suspecting it might depend on some setup.
> 
> Its a typo, added a follow-up, feel free to squash it if you haven't already
> pushed the first patch to main

Already pushed, or more fixed. Hope that's ok. I was just really interested since I get a lof of those misses if the QA script always detects it.
Comment 9 Katrin Fischer 2025-01-31 14:58:23 UTC
Pushed for 25.05!

Well done everyone, thank you!
Comment 10 Jonathan Druart 2025-02-03 08:50:06 UTC
The test will always pass, it's using warning_like, it does not make sense.
Comment 11 Jonathan Druart 2025-02-03 08:50:31 UTC
Created attachment 177463 [details] [review]
Bug 39007: Replace warning_like with is
Comment 12 Jonathan Druart 2025-02-03 08:54:03 UTC
I am also wondering why we are comparing using a "fake" array.

If last_audit is removed from the api spec, the test is still passing. You only need to add it to the @expected_fields list in the .t file.

It would be more robust to actually compare with the response of our route, or (if too complicated) with the spec directly.
Comment 13 Katrin Fischer 2025-02-03 12:55:30 UTC
Is your follow-up ready to be pushed or better wait?
Comment 14 Jonathan Druart 2025-02-03 14:09:21 UTC
(In reply to Katrin Fischer from comment #13)
> Is your follow-up ready to be pushed or better wait?

The test is useless without the follow-up as it will always pass.
Comment 15 Katrin Fischer 2025-02-04 08:09:45 UTC
Picked "Repalye warning_like with is" for next push to main.
Comment 16 Alex Buckley 2025-02-10 02:32:00 UTC
Hi all, 

We are getting a sushi error on jenkins ( https://jenkins.koha-community.org/job/Koha_Sec/job/Koha_24.05/lastCompletedBuild/testReport/(root)/t_db_dependent_api_v1_erm_sushi_services_t/get___tests__2270_/ ) and we have been told to apply this patchfix.

When we apply it is not applying cleanly, see below:

kohadev-koha@kohadevbox:koha((a5afd9c9c8b...)|AM 1/1)$ sed -n '/<<<<</,/>>>>>/p' t/db_dependent/api/v1/erm_sushi_services.t
<<<<<<< HEAD
    is( scalar(@new_fields_in_response), 0, 'The response fields match the expected fields' );
=======
    my $new_fields_string =
          "This is not a new error within Koha, the following new field(s) have been added to the API repsonse: "
        . join( ', ', @new_fields_in_response )
        . '. They should be added to the API definition';
    warning_like { scalar(@new_fields_in_response) } 0, $new_fields_string;

    $schema->storage->txn_rollback;
>>>>>>> Bug 39007: Update API spec and unit test


The difference is the $schema->storage->txn_rollback; line which is introduced by bug 37898

Could someone please rebase for 24.05 or advise what I should do?

Thanks,
Alex
Comment 17 Jonathan Druart 2025-02-13 13:20:08 UTC
Created attachment 178004 [details] [review]
Bug 39007: [24.05] Update API spec and unit test

Test plan:
1) Run prove t/db_dependent/api/v1/erm_sushi_services.t - FAIL
2) Apply patch
3) Repeat step 1 - PASS
4) Inspect patch diff and note that last_audit has been added to the API definition

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>

 Please enter the commit message for your changes. Lines starting
Comment 18 Jonathan Druart 2025-02-13 13:20:11 UTC
Created attachment 178005 [details] [review]
Bug 39007: [24.05] (follow-up) Fix typo
Comment 19 Jonathan Druart 2025-02-13 13:20:13 UTC
Created attachment 178006 [details] [review]
Bug 39007: [24.05] Replace warning_like with is
Comment 20 Alex Buckley 2025-02-14 03:17:24 UTC
Thank you for the rebased patches Jonathan
Comment 21 Wainui Witika-Park 2025-02-17 22:29:25 UTC
Applied to 24.05.x-security
Comment 22 Fridolin Somers 2025-02-21 14:11:05 UTC
Applied to 23.11.x-security
Comment 23 Katrin Fischer 2025-02-27 15:57:50 UTC
Hi RMaints, can you please update the "Versions released in"?
Comment 24 Alex Buckley 2025-02-27 19:55:30 UTC
(In reply to Katrin Fischer from comment #23)
> Hi RMaints, can you please update the "Versions released in"?

All done, thanks Katrin :)