Bug 20590 introduced a way to create more useful and testable exception messages. But we should still be allowed to pass manual messages, because we might not have all the information we need to build the stringified version like it proposed. Notably on Koha::Object->store, in which we parse the DBI warning.
Created attachment 76571 [details] [review] Bug 21022: Unit tests
Created attachment 76572 [details] [review] Bug 21022: Use passed message if present This patch makes 'full_message' use the passed exception message instead of trying to build it from the parameters. This is particularly useful for some situations in which we don't have all the information but would like to keep using the same exception. To test: - Apply this patchset - Run: $ kshell k$ prove t/Koha/Exceptions.t => SUCCESS: Tests pass! - Sign off
Created attachment 76573 [details] [review] Bug 21022: (follow-up) Move overloaded full_message method to subclass We shouldn't have it overloaded on the base class, as it could get huge and difficult to find things, and read. This patch moves things to Koha::Exceptions::Object. We should overload the full_message method on each exceptions class, as needed. To test: - Run: $ kshell k$ prove t/Koha/Exceptions.t => SUCCESS: Tests still pass! - Sign off :-D
Created attachment 76889 [details] [review] Bug 21022: Unit tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 76890 [details] [review] Bug 21022: Use passed message if present This patch makes 'full_message' use the passed exception message instead of trying to build it from the parameters. This is particularly useful for some situations in which we don't have all the information but would like to keep using the same exception. To test: - Apply this patchset - Run: $ kshell k$ prove t/Koha/Exceptions.t => SUCCESS: Tests pass! - Sign off Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 76891 [details] [review] Bug 21022: (follow-up) Move overloaded full_message method to subclass We shouldn't have it overloaded on the base class, as it could get huge and difficult to find things, and read. This patch moves things to Koha::Exceptions::Object. We should overload the full_message method on each exceptions class, as needed. To test: - Run: $ kshell k$ prove t/Koha/Exceptions.t => SUCCESS: Tests still pass! - Sign off :-D Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Seems sane, code is reasonable and well structured, signing off :)
Created attachment 76893 [details] [review] Bug 21022: Unit tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 76894 [details] [review] Bug 21022: Use passed message if present This patch makes 'full_message' use the passed exception message instead of trying to build it from the parameters. This is particularly useful for some situations in which we don't have all the information but would like to keep using the same exception. To test: - Apply this patchset - Run: $ kshell k$ prove t/Koha/Exceptions.t => SUCCESS: Tests pass! - Sign off Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 76895 [details] [review] Bug 21022: (follow-up) Move overloaded full_message method to subclass We shouldn't have it overloaded on the base class, as it could get huge and difficult to find things, and read. This patch moves things to Koha::Exceptions::Object. We should overload the full_message method on each exceptions class, as needed. To test: - Run: $ kshell k$ prove t/Koha/Exceptions.t => SUCCESS: Tests still pass! - Sign off :-D Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Awesome work all! Pushed to master for 18.11
Pushed to 18.05.x for 18.05.02
Depends on Bug 20590 not in 17.11.x