Bug 39932 - Koha::Item->_status should return an array
Summary: Koha::Item->_status should return an array
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low major
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on: 37334
Blocks:
  Show dependency treegraph
 
Reported: 2025-05-17 12:21 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2025-05-22 14:24 UTC (History)
4 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
25.05.00
Circulation function:


Attachments
Bug 39932: Make Koha::Item->_status return an arrayref (6.78 KB, patch)
2025-05-19 12:15 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 39932: Add POD for Koha::Item->_status() (1.10 KB, patch)
2025-05-19 12:15 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 39932: Remove split in template (1.16 KB, patch)
2025-05-19 12:15 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 39932: Make Koha::Item->_status return an arrayref (6.82 KB, patch)
2025-05-19 13:59 UTC, David Nind
Details | Diff | Splinter Review
Bug 39932: Add POD for Koha::Item->_status() (1.14 KB, patch)
2025-05-19 13:59 UTC, David Nind
Details | Diff | Splinter Review
Bug 39932: Remove split in template (1.21 KB, patch)
2025-05-19 13:59 UTC, David Nind
Details | Diff | Splinter Review
Bug 39932: Make Koha::Item->_status return an arrayref (6.89 KB, patch)
2025-05-19 18:04 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 39932: Add POD for Koha::Item->_status() (1.21 KB, patch)
2025-05-19 18:04 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 39932: Remove split in template (1.27 KB, patch)
2025-05-19 18:04 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-05-17 12:21:49 UTC
The `_status` method has a couple problems:

* It doesn't have a POD
* It returns a string!

It should return a list instead, so handling on the UI doesn't require splitting the string back.

That said, the tests using it in the UI (Holdings_spec.ts) only test the `available` rendering use case. This is minor.
Comment 1 Tomás Cohen Arazi (tcohen) 2025-05-17 12:22:25 UTC
I think this should be fixed for the release.
Comment 2 Tomás Cohen Arazi (tcohen) 2025-05-19 12:15:10 UTC
Created attachment 182598 [details] [review]
Bug 39932: Make Koha::Item->_status return an arrayref

This patch makes the method return an actual array instead of a
comma-separated string. The API spec is adjusted so it also expects an
array in the response.

Tests are adjusted for the new return value.

To test:
1. Apply this patch
2. Run:
   $ ktd --shell
  k$ yarn api:bundle
  k$ prove t/db_dependent/Koha/Item.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Comment 3 Tomás Cohen Arazi (tcohen) 2025-05-19 12:15:12 UTC
Created attachment 182599 [details] [review]
Bug 39932: Add POD for Koha::Item->_status()

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 4 Tomás Cohen Arazi (tcohen) 2025-05-19 12:15:15 UTC
Created attachment 182600 [details] [review]
Bug 39932: Remove split in template
Comment 5 Tomás Cohen Arazi (tcohen) 2025-05-19 12:18:33 UTC
I marked it as Major to get more attention. I think the API spec is the most sensible issue here, as people could start using the 'string value' once it is published.
Comment 6 David Nind 2025-05-19 13:59:40 UTC
Created attachment 182605 [details] [review]
Bug 39932: Make Koha::Item->_status return an arrayref

This patch makes the method return an actual array instead of a
comma-separated string. The API spec is adjusted so it also expects an
array in the response.

Tests are adjusted for the new return value.

To test:
1. Apply this patch
2. Run:
   $ ktd --shell
  k$ yarn api:bundle
  k$ prove t/db_dependent/Koha/Item.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 David Nind 2025-05-19 13:59:43 UTC
Created attachment 182606 [details] [review]
Bug 39932: Add POD for Koha::Item->_status()

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Comment 8 David Nind 2025-05-19 13:59:45 UTC
Created attachment 182607 [details] [review]
Bug 39932: Remove split in template

Signed-off-by: David Nind <david@davidnind.com>
Comment 9 Martin Renvoize (ashimema) 2025-05-19 18:04:14 UTC
Created attachment 182632 [details] [review]
Bug 39932: Make Koha::Item->_status return an arrayref

This patch makes the method return an actual array instead of a
comma-separated string. The API spec is adjusted so it also expects an
array in the response.

Tests are adjusted for the new return value.

To test:
1. Apply this patch
2. Run:
   $ ktd --shell
  k$ yarn api:bundle
  k$ prove t/db_dependent/Koha/Item.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 10 Martin Renvoize (ashimema) 2025-05-19 18:04:18 UTC
Created attachment 182633 [details] [review]
Bug 39932: Add POD for Koha::Item->_status()

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 11 Martin Renvoize (ashimema) 2025-05-19 18:04:21 UTC
Created attachment 182634 [details] [review]
Bug 39932: Remove split in template

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 12 Katrin Fischer 2025-05-20 13:16:21 UTC
Pushed for 25.05!

Well done everyone, thank you!
Comment 13 Katrin Fischer 2025-05-22 13:36:39 UTC
This is blamed for failing tests, please fix!
Comment 14 Tomás Cohen Arazi (tcohen) 2025-05-22 14:24:50 UTC
(In reply to Katrin Fischer from comment #13)
> This is blamed for failing tests, please fix!

Pushed already (in 37334 context)