Summary: | Holds placed via the REST API should not be forced by default even if AllowHoldPolicyOverride is enabled | ||
---|---|---|---|
Product: | Koha | Reporter: | Kyle M Hall (khall) <kyle> |
Component: | REST API | Assignee: | Tomás Cohen Arazi (tcohen) <tomascohen> |
Status: | CLOSED FIXED | QA Contact: | Marcel de Rooy <m.de.rooy> |
Severity: | normal | ||
Priority: | P5 - low | CC: | 1joynelson, agustinmoyano, andrew, david, fridolin.somers, m.de.rooy, martin.renvoize, tomascohen, victor |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26072 | ||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: |
This patch disables AllowHoldPolicyOverride by default in the /holds REST API. It also adds tests for this behaviour, and adds a header that can be used to request the override explicitly.
|
Version(s) released in: |
21.05.00,20.11.03,20.05.09
|
Circulation function: | |||
Bug Depends on: | |||
Bug Blocks: | 27760, 27797 | ||
Attachments: |
Bug 26181: regression tests
Bug 26181: Disable override by default in /holds Bug 26181: Regression tests Bug 26181: Disable override by default in /holds Bug 26181: Regression tests Bug 26181: Disable override by default in /holds Bug 26181: Regression tests Bug 26181: Disable override by default in /holds |
Description
Kyle M Hall (khall)
2020-08-07 16:07:17 UTC
Created attachment 108016 [details] [review] Bug 26181: regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 108017 [details] [review] Bug 26181: Disable override by default in /holds This patch disables AllowHoldPolicyOverride by default in /holds. It also adds a header that can be used to request the override explicitly. Tests are added for this behaviour To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/holds.t => FAIL: Tests fail because the behaviour is not implemented 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> I left the AllowHoldPolicyOverride name in the x-koha-override header on purpose. To have less friction on this patches. But I really preferred something like 'holds-policy'. Food for thought. Patch no longer applies 8-( : root@kohadevbox:koha(bz26181)$ git bz apply 26181 Bug 26181 - Holds placed via the REST API should not be forced by default even if AllowHoldPolicyOverride is enabled 108016 - Bug 26181: regression tests 108017 - Bug 26181: Disable override by default in /holds Apply? [(y)es, (n)o, (i)nteractive] i Applying: Bug 26181: regression tests Using index info to reconstruct a base tree... M t/db_dependent/api/v1/holds.t Falling back to patching base and 3-way merge... Auto-merging t/db_dependent/api/v1/holds.t CONFLICT (content): Merge conflict in t/db_dependent/api/v1/holds.t error: Failed to merge in the changes. Patch failed at 0001 Bug 26181: regression tests The copy of the patch that failed is found in: .git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-26181-regression-tests-FLFBjo.patch Question: Do I need to post the message for the patch failing, or is it okay to just say that the patch no longer applies? Created attachment 116720 [details] [review] Bug 26181: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 116721 [details] [review] Bug 26181: Disable override by default in /holds This patch disables AllowHoldPolicyOverride by default in /holds. It also adds a header that can be used to request the override explicitly. Tests are added for this behaviour To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/holds.t => FAIL: Tests fail because the behaviour is not implemented 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 116757 [details] [review] Bug 26181: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 116758 [details] [review] Bug 26181: Disable override by default in /holds This patch disables AllowHoldPolicyOverride by default in /holds. It also adds a header that can be used to request the override explicitly. Tests are added for this behaviour To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/holds.t => FAIL: Tests fail because the behaviour is not implemented 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: Kyle M Hall <kyle@bywatersolutions.com> (In reply to Tomás Cohen Arazi from comment #3) > I left the AllowHoldPolicyOverride name in the x-koha-override header on > purpose. To have less friction on this patches. > But I really preferred something like 'holds-policy'. Food for thought. I'm tempted by the actual rule/policy failure names being in x-koha-override.. and in this case as the override is only available as a 'all or nothing' we should start with a special 'any' rule. So x-koha-override: [ any ] # Meaning, override whatever we failed for in the future x-koha-override: [ any, damaged_item, transfer_restriction, age_restriction, etc, etc ] # An override per policy violation error that can be thrown and individually override. Thoughts? QAing Created attachment 116786 [details] [review] Bug 26181: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 116787 [details] [review] Bug 26181: Disable override by default in /holds This patch disables AllowHoldPolicyOverride by default in /holds. It also adds a header that can be used to request the override explicitly. Tests are added for this behaviour To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/holds.t => FAIL: Tests fail because the behaviour is not implemented 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: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Pushed to master for 21.05, thanks to everybody involved! Pushed to 20.11.x for 20.11.03 Pushed to 20.05.x for 20.05.09 Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed. |