| Summary: | Missing logging of $@ in onboarding.pl after eval block | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Slava Shishkin <slavashishkin> |
| Component: | Installation and upgrade (web-based installer) | Assignee: | Slava Shishkin <slavashishkin> |
| Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
| Severity: | trivial | ||
| Priority: | P5 - low | CC: | aleisha, gmcharlt, jonathan.druart, lucas, martin.renvoize, nugged, stalkernoid |
| 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: |
20.11.00, 20.05.03
|
|
| Circulation function: | |||
| Bug Depends on: | |||
| Bug Blocks: | 25790 | ||
| Attachments: |
Bug 25695: add a missing warning in onboarding.pl
Bug 25695: merge of similar code blocks after circulation_rules update Bug 25695: merge of similar code blocks after circulation_rules update Bug 25695: merge of similar code blocks after circulation_rules update |
||
|
Description
Slava Shishkin
2020-06-08 15:18:30 UTC
Created attachment 105650 [details] [review] Bug 25695: add a missing warning in onboarding.pl There is a forgotten warning in the second eval block just after the same eval block which has warning. This makes the error reason not logged. The solution is to add exactly the same 'warn $@' in consecutive eval block. Mentored-by: Andrew Nugged <nugged@gmail.com> Why are not we actually setting maxissueqty in the same set_rules call? It's coming from commit 3c4232e746cef574de9d1a346be7704dc3cd2e33 Bug 22489: Update onboarding tool for bug 18925 But now we have all our rules moved to circulation_rules, I think maxissueqty can be moved into the same call as the other rules. Yes, that was the second option but more "distant from the original". I agree, we will. Slava: let's do it. Created attachment 105998 [details] [review] Bug 25695: merge of similar code blocks after circulation_rules update In onboarding.pl we have second block with adding just one more rule parameter (maxissueqty) which can be merged to first one after all parameters now stored in circulation_rules table. Also this eliminates the need of adding forgotten "warn $@" after the second eval block. Mentored-by: Andrew Nugged <nugged@gmail.com> have we saved a "success" message? Seems it's lost :).
This one:
push @messages, { code => 'success_on_insert_circ_rule' };
let's update :).
Created attachment 105999 [details] [review] Bug 25695: merge of similar code blocks after circulation_rules update In onboarding.pl we have second block with adding just one more rule parameter (maxissueqty) which can be merged to first one after all parameters now stored in circulation_rules table. Also this eliminates the need of adding forgotten "warn $@" after the second eval block. Mentored-by: Andrew Nugged <nugged@gmail.com> Created attachment 106387 [details] [review] Bug 25695: merge of similar code blocks after circulation_rules update In onboarding.pl we have second block with adding just one more rule parameter (maxissueqty) which can be merged to first one after all parameters now stored in circulation_rules table. Also this eliminates the need of adding forgotten "warn $@" after the second eval block. Mentored-by: Andrew Nugged <nugged@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> A nice bit of tidying up.. works as expected, thanks! Going straight for a QA here.. PQA Pushed to master for 20.11, thanks to everybody involved! backported to 20.05.x for 20.05.03 Does not apply cleanly to 19.11.x. Please rebase if needed. |