| Summary: | GET /items* broken if notforloan == 0 and itemtype.notforloan == NULL | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Tomás Cohen Arazi (tcohen) <tomascohen> |
| Component: | REST API | Assignee: | Tomás Cohen Arazi (tcohen) <tomascohen> |
| Status: | CLOSED FIXED | QA Contact: | Marcel de Rooy <m.de.rooy> |
| Severity: | major | ||
| Priority: | P5 - low | CC: | dcook, fridolin.somers, lucas, m.de.rooy, nick, tomascohen |
| Version: | unspecified | Keywords: | rel_22_11_candidate, rel_23_05_candidate, rel_23_11_candidate |
| 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: |
23.11.00,23.05.05,22.11.12
|
|
| Circulation function: | |||
| Bug Depends on: | 31128 | ||
| Bug Blocks: | |||
| Attachments: |
Bug 35167: Regression tests
Bug 35167: Make 'effective_not_for_loan_status' fallback to 0 if itype has it undef Bug 35167: Regression tests Bug 35167: Make 'effective_not_for_loan_status' fallback to 0 if itype has it undef Bug 35167: Regression tests Bug 35167: Make 'effective_not_for_loan_status' fallback to 0 if itype has it undef |
||
I understood now how it works on the UI and will provide a fix :-D. Thanks Nick and Kyle. Created attachment 157948 [details] [review] Bug 35167: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 157949 [details] [review] Bug 35167: Make 'effective_not_for_loan_status' fallback to 0 if itype has it undef This patch makes the effective not for loan status be set the item value if not defined at itype level. To test: 1. Apply the regressions tests patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/items.t => FAIL: Tests fail! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 157954 [details] [review] Bug 35167: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Created attachment 157955 [details] [review] Bug 35167: Make 'effective_not_for_loan_status' fallback to 0 if itype has it undef This patch makes the effective not for loan status be set the item value if not defined at itype level. To test: 1. Apply the regressions tests patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/items.t => FAIL: Tests fail! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Looking here Created attachment 157968 [details] [review] Bug 35167: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 157969 [details] [review] Bug 35167: Make 'effective_not_for_loan_status' fallback to 0 if itype has it undef This patch makes the effective not for loan status be set the item value if not defined at itype level. To test: 1. Apply the regressions tests patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/items.t => FAIL: Tests fail! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Shouldnt we at some point change the itemtypes table? `notforloan` smallint(6) DEFAULT NULL COMMENT '1 if the item is not for loan, 0 if the item is available for loan', Just inconsistent? Pushed to master for 23.11. Nice work everyone, thanks! Pushed to 23.05.x for 23.05.05 Nice work everyone! Pushed to oldstable for 22.11.x |
The Koha::Item->to_api method has this (modded for readability): $overrides->{effective_not_for_loan_status} = $self->notforloan ? $self->notforloan : $self->itemtype->notforloan; This is weird, because on the table definition, that column is defined as NOT NULL: `notforloan` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'authorized value defining why this item is not for loan (MARC21 952$7)', so, if the item has a value of '0', then it will look for the itemtype's default `notforloan` value, which can be (and most of the time is) NULL! I will provide a patch checking 'defined $self->notforloan' but it feels like adding `effective_not_for_loan_status` was a mistake.