Description
Tomás Cohen Arazi (tcohen)
2017-12-19 19:42:26 UTC
Created attachment 69966 [details] [review] Bug 19841: Unit tests Created attachment 69967 [details] [review] Bug 19841: Make AddMember raise an exception if categorycode is invalid This patch makes C4::Members::AddMember raise a Koha::Exceptions::BadParameter exception. The current behaviour is to just explode. Code calling AddReturn should work as expected, unless they are looking for a specific failure message. It is just explodes in a way we know what happened, in the case the categorycode is invalid. To test: - Apply the Unit tests patch - Run: $ kshell k$ prove t/db_dependent/Members.t => FAIL: Tests fail because AddReturn doesn't raise an exception - Apply this patch - Run: $ kshell k$ prove t/db_dependent/Members.t => SUCCESS: Tests pass! - Sign off :-D Note: - If the code is not catching problems, it is already failing, and it will now fail in a different way. But no behaviour change. - If the code is catching problems (either Try::Tiny or just eval) the thing to test, is if the code handling the problem (in AddReturn) is expectig DBIC specific exceptions, die, etc. I grepped the code and didn't find places in which AddReturn is eval-ed and THEN the failure message is verified/inspected. This means the behaviour is not changed by this patches. But another set of eyes is welcome. Created attachment 69968 [details] [review] Bug 19841: Make AddMember raise an exception if categorycode is invalid This patch makes C4::Members::AddMember raise a Koha::Exceptions::BadParameter exception. The current behaviour is to just explode. Code calling AddReturn should work as expected, unless they are looking for a specific failure message. It is just explodes in a way we know what happened, in the case the categorycode is invalid. To test: - Apply the Unit tests patch - Run: $ kshell k$ prove t/db_dependent/Members.t => FAIL: Tests fail because AddReturn doesn't raise an exception - Apply this patch - Run: $ kshell k$ prove t/db_dependent/Members.t => SUCCESS: Tests pass! - Sign off :-D Created attachment 70010 [details] [review] Bug 19841: Unit tests This patch introduces unit tests for the new behaviour of AddMember (raising an exception if the passed categorycode is not valid. To test: - Apply this patch - Run: $ kshell k$ prove t/db_dependent/Members.t => FAIL: It should fail because the feature is still not implemented. Created attachment 70011 [details] [review] Bug 19841: Make AddMember raise an exception if categorycode is invalid This patch makes C4::Members::AddMember raise a Koha::Exceptions::BadParameter exception. The current behaviour is to just explode. Code calling AddReturn should work as expected, unless they are looking for a specific failure message. It is just explodes in a way we know what happened, in the case the categorycode is invalid. To test: - Apply the Unit tests patch - Run: $ kshell k$ prove t/db_dependent/Members.t => FAIL: Tests fail because AddReturn doesn't raise an exception - Apply this patch - Run: $ kshell k$ prove t/db_dependent/Members.t => SUCCESS: Tests pass! - Sign off :-D Created attachment 70138 [details] [review] Bug 19841: Make AddMember raise an exception if categorycode is invalid This patch makes C4::Members::AddMember raise a Koha::Exceptions::BadParameter exception. The current behaviour is to just explode. Code calling AddReturn should work as expected, unless they are looking for a specific failure message. It is just explodes in a way we know what happened, in the case the categorycode is invalid. To test: - Apply the Unit tests patch - Run: $ kshell k$ prove t/db_dependent/Members.t => FAIL: Tests fail because AddReturn doesn't raise an exception - Apply this patch - Run: $ kshell k$ prove t/db_dependent/Members.t => SUCCESS: Tests pass! - Sign off :-D Signed-off-by: dom <dominic.pichette@inlibro.com> Created attachment 70147 [details] [review] Bug 19841: Unit tests This patch introduces unit tests for the new behaviour of AddMember (raising an exception if the passed categorycode is not valid. To test: - Apply this patch - Run: $ kshell k$ prove t/db_dependent/Members.t => FAIL: It should fail because the feature is still not implemented. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 70148 [details] [review] Bug 19841: Make AddMember raise an exception if categorycode is invalid This patch makes C4::Members::AddMember raise a Koha::Exceptions::BadParameter exception. The current behaviour is to just explode. Code calling AddReturn should work as expected, unless they are looking for a specific failure message. It is just explodes in a way we know what happened, in the case the categorycode is invalid. To test: - Apply the Unit tests patch - Run: $ kshell k$ prove t/db_dependent/Members.t => FAIL: Tests fail because AddReturn doesn't raise an exception - Apply this patch - Run: $ kshell k$ prove t/db_dependent/Members.t => SUCCESS: Tests pass! - Sign off :-D Signed-off-by: dom <dominic.pichette@inlibro.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> (In reply to Josef Moravec from comment #9) > Signed-off-by: dom <dominic.pichette@inlibro.com> Dominic, please fix you git alias! Pushed to master for 18.05, thanks to everybody involved! Enhancement, skipping for 17.11.x. Awesome work everybody! |