Bug 39771

Summary: The `data` attribute in job.yaml should be nullable
Product: Koha Reporter: Tomás Cohen Arazi (tcohen) <tomascohen>
Component: REST APIAssignee: Tomás Cohen Arazi (tcohen) <tomascohen>
Status: Pushed to main --- QA Contact: Martin Renvoize (ashimema) <martin.renvoize>
Severity: normal    
Priority: P5 - low CC: david, dcook, martin.renvoize, tadeusz, tomascohen
Version: Main   
Hardware: All   
OS: All   
GIT URL: Change sponsored?: ---
Patch complexity: --- Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
25.05.00
Circulation function:
Attachments: Bug 39771: Regression tests
Bug 39771: Make `data` attribute nullable for jobs
Bug 39771: Regression tests
Bug 39771: Make `data` attribute nullable for jobs
Bug 39771: Regression tests
Bug 39771: Make `data` attribute nullable for jobs

Description Tomás Cohen Arazi (tcohen) 2025-04-29 10:38:58 UTC
The attribute can be null (i.e. you can call `$job->finish()` and have it be set `null`) but the API spec expects an object.
Comment 1 Tomás Cohen Arazi (tcohen) 2025-04-29 10:55:38 UTC
Created attachment 181641 [details] [review]
Bug 39771: Regression tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Tomás Cohen Arazi (tcohen) 2025-04-29 10:55:41 UTC
Created attachment 181642 [details] [review]
Bug 39771: Make `data` attribute nullable for jobs

This patch makes the `data` attribute nullable. This happens when
`$job->finish()` is called with no parameters.

To test:
1. Apply the regression tests patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/jobs.t
=> FAIL: Tests don't pass, a 500 is returned when `finish()` was called
with no data
3. Apply this patch
4. Rebuild the API spec:
  k$ yarn api:bundle
5. Repeat 2
=> SUCCESS: Tests pass! The API doesn't explode
6. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 HKS3 Tadeusz Sośnierz 2025-04-29 11:11:35 UTC
Created attachment 181643 [details] [review]
Bug 39771: Regression tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Tadeusz „tadzik” Sośnierz <tadeusz@sosnierz.com>
Comment 4 HKS3 Tadeusz Sośnierz 2025-04-29 11:11:38 UTC
Created attachment 181644 [details] [review]
Bug 39771: Make `data` attribute nullable for jobs

This patch makes the `data` attribute nullable. This happens when
`$job->finish()` is called with no parameters.

To test:
1. Apply the regression tests patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/jobs.t
=> FAIL: Tests don't pass, a 500 is returned when `finish()` was called
with no data
3. Apply this patch
4. Rebuild the API spec:
  k$ yarn api:bundle
5. Repeat 2
=> SUCCESS: Tests pass! The API doesn't explode
6. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Tadeusz „tadzik” Sośnierz <tadeusz@sosnierz.com>
Comment 5 Tomás Cohen Arazi (tcohen) 2025-04-29 11:13:42 UTC
Some testing notes.

To reproduce the user facing error, I staged a file for import, then set the `data` column the string `null`. And the UI effectively failed to render the table with an API error 500.

After following the test plan and restarting Plack, the table loaded correctly.

The UI itself doesn’t use the `data` attribute.
Comment 6 HKS3 Tadeusz Sośnierz 2025-04-29 11:17:51 UTC
I've also verified this manually with a job that does finish() with no arguments. The API 500s and the page errors before the patch, works fine after it.
Comment 7 David Nind 2025-04-29 13:44:48 UTC
(In reply to HKS3 Tadeusz Sośnierz from comment #6)
> I've also verified this manually with a job that does finish() with no
> arguments. The API 500s and the page errors before the patch, works fine
> after it.

Please change the status to signed off so that you get the credit on the dashboard 8-)
Comment 8 Martin Renvoize (ashimema) 2025-05-08 12:51:33 UTC
Created attachment 182079 [details] [review]
Bug 39771: Regression tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Tadeusz „tadzik” Sośnierz <tadeusz@sosnierz.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 9 Martin Renvoize (ashimema) 2025-05-08 12:51:36 UTC
Created attachment 182080 [details] [review]
Bug 39771: Make `data` attribute nullable for jobs

This patch makes the `data` attribute nullable. This happens when
`$job->finish()` is called with no parameters.

To test:
1. Apply the regression tests patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/jobs.t
=> FAIL: Tests don't pass, a 500 is returned when `finish()` was called
with no data
3. Apply this patch
4. Rebuild the API spec:
  k$ yarn api:bundle
5. Repeat 2
=> SUCCESS: Tests pass! The API doesn't explode
6. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Tadeusz „tadzik” Sośnierz <tadeusz@sosnierz.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 10 Katrin Fischer 2025-05-09 07:14:28 UTC
Pushed for 25.05!

Well done everyone, thank you!