Bug 39771 - The `data` attribute in job.yaml should be nullable
Summary: The `data` attribute in job.yaml should be nullable
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2025-04-29 10:38 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2025-05-09 07:14 UTC (History)
5 users (show)

See Also:
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 (2.44 KB, patch)
2025-04-29 10:55 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 39771: Make `data` attribute nullable for jobs (1.22 KB, patch)
2025-04-29 10:55 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 39771: Regression tests (2.60 KB, patch)
2025-04-29 11:11 UTC, HKS3 Tadeusz Sośnierz
Details | Diff | Splinter Review
Bug 39771: Make `data` attribute nullable for jobs (1.38 KB, patch)
2025-04-29 11:11 UTC, HKS3 Tadeusz Sośnierz
Details | Diff | Splinter Review
Bug 39771: Regression tests (2.66 KB, patch)
2025-05-08 12:51 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 39771: Make `data` attribute nullable for jobs (1.44 KB, patch)
2025-05-08 12:51 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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!