Bug 20590

Summary: Koha::Exceptions does not stringify the exceptions
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: ere.maijala, fridolin.somers, julian.maurice, kyle, m.de.rooy, martin.renvoize, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20580
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 21022    
Attachments: Bug 20590: Improve Koha::Exceptions in string context
Bug 20590: Koha::Exceptions::Exception
Bug 20590: Improve Koha::Exceptions in string context
Bug 20590: Koha::Exceptions::Exception
Bug 20590: Unit tests
Bug 20590: Include Koha::Exceptions::Exception
Bug 20590: Fix unit test
Bug 20590: Fix Members.t

Description Jonathan Druart 2018-04-16 15:38:08 UTC
I was working on bug 20580 and noticed that the exceptions are not stringified and so not really useful when displayed in string context.
Comment 1 Jonathan Druart 2018-04-16 15:45:11 UTC
Created attachment 74246 [details] [review]
Bug 20590: Improve Koha::Exceptions in string context

I was working on bug 20580 and noticed that the exceptions are not
stringified and so not really useful when displayed in string context.

This patch is for discussion and not ready for inclusion

Still need to fix:
Koha/REST/V1/Patrons.pm (The error is "Given xxx does not exist")
Koha/Object.pm: in ->store, Koha::Exceptions::Object::FKConstraint is
raised but we cannot set 'value'

Another note: In Koha::Exceptions::Exception I do not have access to
$self->line_number and $self->filename

See also
http://search.cpan.org/~drolsky/Exception-Class-1.44/lib/Exception/Class/Base.pm#OVERRIDING_THE_as_string_METHOD

Test plan:
Apply patch from bug 20580 and execute create_superlibrarian.pl with an
invalid categorycode
Comment 2 Jonathan Druart 2018-04-16 15:46:59 UTC
Hi guys,
It seems that our exception classes have few issues. Could you take a look at this patch and give feedbacks?
Comment 3 Jonathan Druart 2018-04-19 13:51:08 UTC
Created attachment 74556 [details] [review]
Bug 20590: Koha::Exceptions::Exception
Comment 4 Tomás Cohen Arazi 2018-04-19 14:26:18 UTC
Created attachment 74560 [details] [review]
Bug 20590: Improve Koha::Exceptions in string context

I was working on bug 20580 and noticed that the exceptions are not
stringified and so not really useful when displayed in string context.

This patch is for discussion and not ready for inclusion

Still need to fix:
Koha/REST/V1/Patrons.pm (The error is "Given xxx does not exist")
Koha/Object.pm: in ->store, Koha::Exceptions::Object::FKConstraint is
raised but we cannot set 'value'

Another note: In Koha::Exceptions::Exception I do not have access to
$self->line_number and $self->filename

See also
http://search.cpan.org/~drolsky/Exception-Class-1.44/lib/Exception/Class/Base.pm#OVERRIDING_THE_as_string_METHOD

Test plan:
Apply patch from bug 20580 and execute create_superlibrarian.pl with an
invalid categorycode

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 5 Tomás Cohen Arazi 2018-04-19 14:26:23 UTC
Created attachment 74561 [details] [review]
Bug 20590: Koha::Exceptions::Exception

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 6 Tomás Cohen Arazi 2018-04-19 14:26:31 UTC
Created attachment 74562 [details] [review]
Bug 20590: Unit tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 7 Tomás Cohen Arazi 2018-04-19 14:26:37 UTC
Created attachment 74563 [details] [review]
Bug 20590: Include Koha::Exceptions::Exception

This patch makes Koha::Exceptions::Object include
Koha::Exceptions::Exception so it is not required to include it
excplicitly for using this exceptions.

To test:
- Run:
  $ kshell
 $k prove t/Koha/Exceptions.t
=> FAIL: Tests fail because Koha::Exceptions::Exception is not
(implicitly or explicitly included)
- Apply this patch
- Run
 k$ prove t/Koha/Exceptions.t
=> SUCCESS: Tests pass!

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 8 Jonathan Druart 2018-04-19 15:46:34 UTC
Pushed to master for 18.05, thanks Tomas!
Comment 9 Kyle M Hall 2018-04-19 18:34:23 UTC
Created attachment 74583 [details] [review]
Bug 20590: Fix unit test
Comment 10 Jonathan Druart 2018-04-20 13:18:21 UTC
Created attachment 74639 [details] [review]
Bug 20590: Fix Members.t

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 11 Fridolin Somers 2018-07-30 14:27:44 UTC
I choose not to backport to 17.11.x, too big and complex