In bug 14407 we introduced a new system preference to allow limiting the online self checkout system to an IP or IP Range. The preference introduced, however, is called in_ipset which can be confusing (as for those in the know there is a tool in linux called ipset which isn't related to the original bug). We should therefore rename the system preference to in_iprange to more accurately describe it's function.
I think it's a function name rather than a system preference, but otherwise I totally agree. I think it is confusingly named.
Created attachment 90100 [details] [review] Bug 22943: Renames the in_ipset function to in_iprange in C4::Auth Bug 14407 introduced a new system preference to allow limiting the online self checkout system to an IP or IP Range. The function that handles this is called in_ipset, which is the name of a linux tool. To stop confusion, this patch renames the function to 'in_iprange', and the variable 'ipset' within it to 'iprange'. To test, follow the test plans outlined in Bug 14407 and confirm that everything works as expected. Sponsored-by: Catalyst IT
I am wondering if the reason the name in_ipset was chosen was because multiple, disjoined IP ranges are allowed, not just a single one?
(In reply to M. Tompsett from comment #3) > I am wondering if the reason the name in_ipset was chosen was because > multiple, disjoined IP ranges are allowed, not just a single one? This probably actually say "in_ipranges" that's true. If you look at the code in https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14407, you'll see lots of mentions of ranges instead of set.
Thanks for your comments Mark and David. Patch still needs signoff
Created attachment 100323 [details] [review] Bug 22943: Renames the in_ipset function to in_iprange in C4::Auth Bug 14407 introduced a new system preference to allow limiting the online self checkout system to an IP or IP Range. The function that handles this is called in_ipset, which is the name of a linux tool. To stop confusion, this patch renames the function to 'in_iprange', and the variable 'ipset' within it to 'iprange'. To test, follow the test plans outlined in Bug 14407 and confirm that everything works as expected. Sponsored-by: Catalyst IT Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 100324 [details] [review] Bug 22943: (QA follow-up) Add new cases A few additional cases were introduced since the original bug was authored. This patch simply catches those cases and makes the same change. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
My apologies, I'd missed that you had submitted a patch here Hayley... Signing off :)
Created attachment 100702 [details] [review] Bug 22943: Renames the in_ipset function to in_iprange in C4::Auth Bug 14407 introduced a new system preference to allow limiting the online self checkout system to an IP or IP Range. The function that handles this is called in_ipset, which is the name of a linux tool. To stop confusion, this patch renames the function to 'in_iprange', and the variable 'ipset' within it to 'iprange'. To test, follow the test plans outlined in Bug 14407 and confirm that everything works as expected. Sponsored-by: Catalyst IT Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 100703 [details] [review] Bug 22943: (QA follow-up) Add new cases A few additional cases were introduced since the original bug was authored. This patch simply catches those cases and makes the same change. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 100704 [details] [review] Bug 22943: Remove unused line from Auth.t Removes an unnecessary variable declaration from Auth.t Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Nice work everyone! Pushed to master for 20.05
Backported to 19.11.x branch for 19.11.05
when applied to 19.05.x QA tool yells at me: FAIL opac/sco/printslip.pl OK critic OK forbidden patterns OK git manipulation OK pod OK spelling FAIL valid "in_ipset" is not exported by the C4::Auth module Can't continue after import errors
Ignore my last comment, I failed to add the QA follow-up. Backported to 19.05.x for 19.05.10
Dependency not in 18.11.x, not backporting to 18.11.x series