Summary: | Change holdallowed values from numbers to strings | ||
---|---|---|---|
Product: | Koha | Reporter: | Kyle M Hall (khall) <kyle> |
Component: | Architecture, internals, and plumbing | Assignee: | Tomás Cohen Arazi (tcohen) <tomascohen> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | agustinmoyano, fridolin.somers, jonathan.druart, kyle, martin.renvoize, nick, tomascohen |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
21.05.00
|
|
Circulation function: | |||
Bug Depends on: | |||
Bug Blocks: | 28169 | ||
Attachments: |
Bug 27069: Adjust tests
Bug 27069: Adapt uses of holdallowed Bug 27069: Update existing rules Bug 27069: Adjust tests Bug 27069: Adapt uses of holdallowed Bug 27069: Update existing rules Bug 27069: Adjust tests Bug 27069: Adapt uses of holdallowed Bug 27069: Update existing rules Bug 27069: (follow-up) Only display rules if one is set |
Description
Kyle M Hall (khall)
2020-11-20 17:54:45 UTC
Holds rules are complex enough to get lost fairly easily. No need to ALSO remember what each integer means :-D Created attachment 118252 [details] [review] Bug 27069: Adjust tests Created attachment 118253 [details] [review] Bug 27069: Adapt uses of holdallowed Created attachment 118254 [details] [review] Bug 27069: Update existing rules This patch adds an atomic update file that takes care of translating existing rules. It constrains the rules to translate to those currently expected by Koha and leaves any other value out. To test: 1. Have some manually added rules: $ koha-mysql kohadev > INSERT INTO circulation_rules (rule_name,rule_value) VALUES ('holdallowed', -1); > INSERT INTO circulation_rules (rule_name,rule_value) VALUES ('holdallowed', 1); > INSERT INTO circulation_rules (rule_name,rule_value) VALUES ('holdallowed', 2); > INSERT INTO circulation_rules (rule_name,rule_value) VALUES ('holdallowed', 3); > INSERT INTO circulation_rules (rule_name,rule_value) VALUES ('holdallowed', 0); > INSERT INTO circulation_rules (rule_name,rule_value) VALUES ('holdallowed', 4); 2. Apply this patch 3. Run: $ updatedatabase => SUCCESS: It doesn't explode 4. Verify the created rules were updated correctly: > SELECT * FROM circulation_rules WHERE rule_name='holdallowed'; 5: Verify all the tests that dealt with this rule still pass! $ kshell k$ git diff origin/master --name-only | grep -e '\.t$' | \ xargs prove => SUCCESS: Tests pass! 6. Verify the UI handles setting things correctly 7. Sign off :-D Created attachment 118324 [details] [review] Bug 27069: Adjust tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 118325 [details] [review] Bug 27069: Adapt uses of holdallowed Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 118326 [details] [review] Bug 27069: Update existing rules This patch adds an atomic update file that takes care of translating existing rules. It constrains the rules to translate to those currently expected by Koha and leaves any other value out. To test: 1. Have some manually added rules: $ koha-mysql kohadev > INSERT INTO circulation_rules (rule_name,rule_value) VALUES ('holdallowed', -1); > INSERT INTO circulation_rules (rule_name,rule_value) VALUES ('holdallowed', 1); > INSERT INTO circulation_rules (rule_name,rule_value) VALUES ('holdallowed', 2); > INSERT INTO circulation_rules (rule_name,rule_value) VALUES ('holdallowed', 3); > INSERT INTO circulation_rules (rule_name,rule_value) VALUES ('holdallowed', 0); > INSERT INTO circulation_rules (rule_name,rule_value) VALUES ('holdallowed', 4); 2. Apply this patch 3. Run: $ updatedatabase => SUCCESS: It doesn't explode 4. Verify the created rules were updated correctly: > SELECT * FROM circulation_rules WHERE rule_name='holdallowed'; 5: Verify all the tests that dealt with this rule still pass! $ kshell k$ git diff origin/master --name-only | grep -e '\.t$' | \ xargs prove => SUCCESS: Tests pass! 6. Verify the UI handles setting things correctly 7. Sign off :-D Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Nice improvement, works as expected.. Signing off Created attachment 119223 [details] [review] Bug 27069: Adjust tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 119224 [details] [review] Bug 27069: Adapt uses of holdallowed Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 119225 [details] [review] Bug 27069: Update existing rules This patch adds an atomic update file that takes care of translating existing rules. It constrains the rules to translate to those currently expected by Koha and leaves any other value out. To test: 1. Have some manually added rules: $ koha-mysql kohadev > INSERT INTO circulation_rules (rule_name,rule_value) VALUES ('holdallowed', -1); > INSERT INTO circulation_rules (rule_name,rule_value) VALUES ('holdallowed', 1); > INSERT INTO circulation_rules (rule_name,rule_value) VALUES ('holdallowed', 2); > INSERT INTO circulation_rules (rule_name,rule_value) VALUES ('holdallowed', 3); > INSERT INTO circulation_rules (rule_name,rule_value) VALUES ('holdallowed', 0); > INSERT INTO circulation_rules (rule_name,rule_value) VALUES ('holdallowed', 4); 2. Apply this patch 3. Run: $ updatedatabase => SUCCESS: It doesn't explode 4. Verify the created rules were updated correctly: > SELECT * FROM circulation_rules WHERE rule_name='holdallowed'; 5: Verify all the tests that dealt with this rule still pass! $ kshell k$ git diff origin/master --name-only | grep -e '\.t$' | \ xargs prove => SUCCESS: Tests pass! 6. Verify the UI handles setting things correctly 7. Sign off :-D Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Pushed to master for 21.05, thanks to everybody involved! Created attachment 119454 [details] [review] Bug 27069: (follow-up) Only display rules if one is set Testing that the rule is != 'hold_allowed' is true for each itemtype where the rule is not set. This means it appears that no holds are allowed for any item type if no rules are set To test: 1 - With a fresh testing DB or no holdallowed rule set, you can use SQl below to clear: DELETE from circuation_rules WHERE rule_name='holdallowed'; 2 - Load smart-rules.pl (Administration->Circulation and fines rules) 3 - Scroll to "Default holds policy by item type" 4 - Note all item types appear as 'No holds allowed' 5 - Apply patch 6 - Reload 7 - All clear 8 - Confirm adding rules displays correctly Follow-up pushed to master. Enhancement not pushed to 20.11.x |