Bug 21022

Summary: Exceptions should skip stringifying if message manually passed
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: Architecture, internals, and plumbingAssignee: Tomás Cohen Arazi <tomascohen>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: fridolin.somers, jonathan.druart, martin.renvoize, nick, tomascohen
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Who signed the patch off: Text to go in the release notes:
Version(s) released in:
Bug Depends on: 20590    
Bug Blocks: 20287    
Attachments: Bug 21022: Unit tests
Bug 21022: Use passed message if present
Bug 21022: (follow-up) Move overloaded full_message method to subclass
Bug 21022: Unit tests
Bug 21022: Use passed message if present
Bug 21022: (follow-up) Move overloaded full_message method to subclass
Bug 21022: Unit tests
Bug 21022: Use passed message if present
Bug 21022: (follow-up) Move overloaded full_message method to subclass

Description Tomás Cohen Arazi 2018-06-29 14:02:06 UTC
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.
Comment 1 Tomás Cohen Arazi 2018-06-29 14:18:02 UTC
Created attachment 76571 [details] [review]
Bug 21022: Unit tests
Comment 2 Tomás Cohen Arazi 2018-06-29 14:18:09 UTC
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
Comment 3 Tomás Cohen Arazi 2018-06-29 14:18:19 UTC
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
Comment 4 Martin Renvoize 2018-07-12 12:33:57 UTC
Created attachment 76889 [details] [review]
Bug 21022: Unit tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 5 Martin Renvoize 2018-07-12 12:34:00 UTC
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>
Comment 6 Martin Renvoize 2018-07-12 12:34:03 UTC
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>
Comment 7 Martin Renvoize 2018-07-12 12:34:36 UTC
Seems sane, code is reasonable and well structured, signing off :)
Comment 8 Jonathan Druart 2018-07-12 14:40:42 UTC
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>
Comment 9 Jonathan Druart 2018-07-12 14:40:46 UTC
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>
Comment 10 Jonathan Druart 2018-07-12 14:40:50 UTC
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>
Comment 11 Nick Clemens 2018-07-13 13:49:43 UTC
Awesome work all!

Pushed to master for 18.11
Comment 12 Martin Renvoize 2018-07-15 20:45:17 UTC
Pushed to 18.05.x for 18.05.02
Comment 13 Fridolin SOMERS 2018-07-30 14:28:13 UTC
Depends on Bug 20590 not in 17.11.x