Bug 29857 - We must stringify our exceptions correctly
Summary: We must stringify our exceptions correctly
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 21022
Blocks: 30007 30057
  Show dependency treegraph
 
Reported: 2022-01-12 09:22 UTC by Jonathan Druart
Modified: 2022-12-12 21:25 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.05.00


Attachments
Bug 29857: Make the exception classes inherit from the base class (14.94 KB, patch)
2022-01-12 09:34 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 29857: Provide a generic full_message method for our exceptions (1.37 KB, patch)
2022-01-12 09:34 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 29857: Make the exception classes inherit from the base class (14.95 KB, patch)
2022-01-31 15:17 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29857: Provide a generic full_message method for our exceptions (1.42 KB, patch)
2022-01-31 15:17 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29857: Rename base class as Koha::Exception (37.41 KB, patch)
2022-01-31 15:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29857: Unit tests (2.06 KB, patch)
2022-01-31 15:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29857: Fix behavior (2.10 KB, patch)
2022-01-31 15:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29857: Unit tests (2.36 KB, patch)
2022-01-31 15:21 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29857: Fix behavior (2.10 KB, patch)
2022-01-31 15:21 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29857: Unit tests (2.44 KB, patch)
2022-01-31 16:01 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29857: Fix behavior (2.15 KB, patch)
2022-01-31 16:01 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29857: (CONCEPT) Switch to singular, add a load method (4.28 KB, patch)
2022-02-04 10:32 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29857: (QA follow-up) Add license (1.24 KB, patch)
2022-02-04 10:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29857: (CONCEPT) Switch to singular, add a load method (4.28 KB, patch)
2022-02-04 10:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29857: (QA follow-up) Add license (1.26 KB, patch)
2022-02-09 12:48 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29857: Make the exception classes inherit from the base class (15.02 KB, patch)
2022-02-25 12:08 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29857: Provide a generic full_message method for our exceptions (1.48 KB, patch)
2022-02-25 12:08 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29857: Rename base class as Koha::Exception (37.48 KB, patch)
2022-02-25 12:08 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29857: Unit tests (2.50 KB, patch)
2022-02-25 12:08 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29857: Fix behavior (2.21 KB, patch)
2022-02-25 12:09 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29857: (QA follow-up) Add license (1.32 KB, patch)
2022-02-25 12:09 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29857: (QA follow-up) Fix unit test Object.t (1.28 KB, patch)
2022-03-02 07:06 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 29857: (follow-up) Fix Barcode_transform_hooks.t (1.57 KB, patch)
2022-03-02 12:29 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29857: (follow-up) Fix t/Koha/Plugins/Tab.t (1.15 KB, patch)
2022-03-02 12:40 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29857: (follow-up) Fix t/db_dependent/Koha/Biblio.t (1.32 KB, patch)
2022-03-02 12:49 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29857: (follow-up) Fix t/Koha/Object/Message.t (874 bytes, patch)
2022-03-02 12:52 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29857: (follow-up) Fix t/Koha/Email.t (3.67 KB, patch)
2022-03-02 12:57 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29857: (follow-up) Fix t/db_dependent/Auth.t (909 bytes, patch)
2022-03-02 14:37 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29857: (follow-up) Fix t/db_dependent/Koha/Patron/Modifications.t (1.43 KB, patch)
2022-03-02 14:43 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29857: (follow-up) Fix t/Koha/REST/Plugin/Query.t (1.22 KB, patch)
2022-03-02 18:09 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 29857: (follow-up) Fix t/db_dependent/Koha/Plugins/Plugins.t (1.95 KB, patch)
2022-03-02 18:09 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 29857: (follow-up) Fix t/db_dependent/Koha/Club/Hold.t (1.12 KB, patch)
2022-03-02 18:09 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 29857: (follow-up) Fix t/db_dependent/Koha/BackgroundJobs/BatchUpdateBiblio.t (1.02 KB, patch)
2022-03-02 18:31 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 29857: (follow-up) Fix t/db_dependent/api/v1/transfer_limits.t (979 bytes, patch)
2022-03-02 18:31 UTC, Fridolin Somers
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2022-01-12 09:22:55 UTC
We must have a generic full_message method in the Koha::Exceptions base class to display all the info we have about the exception.
The current state is bad as we are displaying the "description" of the exception but it does not contain useful information.
Comment 1 Jonathan Druart 2022-01-12 09:34:31 UTC
Created attachment 129329 [details] [review]
Bug 29857: Make the exception classes inherit from the base class

We have a base class Koha::Exceptions::Exception but it's not used in
most of our exception classes
Comment 2 Jonathan Druart 2022-01-12 09:34:34 UTC
Created attachment 129330 [details] [review]
Bug 29857: Provide a generic full_message method for our exceptions

We pass useful information when throwing exceptions, but we don't
display them! We must display all the details we need when the exception
is stringified.

Test plan:

Use the following snippet:
  use Koha::Exceptions::Elasticsearch;
  Koha::Exceptions::Elasticsearch::BadResponse->throw( type => "a good type", details => "and some details" );

Execute it without and with the patch.
Comment 3 Jonathan Druart 2022-01-12 09:35:53 UTC
[Bad response received when submitting request to Elasticsearch]

vs

Exception 'Koha::Exceptions::Elasticsearch::BadResponse' thrown 'Bad response received when submitting request to Elasticsearch' with type => a good type, details => and some details
Comment 4 Martin Renvoize 2022-01-12 14:13:49 UTC
This seems a solid improvement to me.. honestly I've found our exceptions confusing to work with.. creating new ones and all the classes all over the place.. this works towards making them all consistent at least.
Comment 5 Martin Renvoize 2022-01-19 15:12:56 UTC
What are the next steps here, discussion wise. I'm on board with the change myself and would happily sign off as is.. were there any particular points you wanted to raise/question Jonathan?
Comment 6 Jonathan Druart 2022-01-19 15:54:42 UTC
Ready for signoff then :)
Comment 7 Tomás Cohen Arazi 2022-01-31 15:17:47 UTC
Created attachment 130007 [details] [review]
Bug 29857: Make the exception classes inherit from the base class

We have a base class Koha::Exceptions::Exception but it's not used in
most of our exception classes

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 8 Tomás Cohen Arazi 2022-01-31 15:17:54 UTC
Created attachment 130008 [details] [review]
Bug 29857: Provide a generic full_message method for our exceptions

We pass useful information when throwing exceptions, but we don't
display them! We must display all the details we need when the exception
is stringified.

Test plan:

Use the following snippet:
  use Koha::Exceptions::Elasticsearch;
  Koha::Exceptions::Elasticsearch::BadResponse->throw( type => "a good type", details => "and some details" );

Execute it without and with the patch.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 9 Tomás Cohen Arazi 2022-01-31 15:18:02 UTC
Created attachment 130009 [details] [review]
Bug 29857: Rename base class as Koha::Exception

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 10 Tomás Cohen Arazi 2022-01-31 15:18:09 UTC Comment hidden (obsolete)
Comment 11 Tomás Cohen Arazi 2022-01-31 15:18:16 UTC Comment hidden (obsolete)
Comment 12 Tomás Cohen Arazi 2022-01-31 15:21:28 UTC Comment hidden (obsolete)
Comment 13 Tomás Cohen Arazi 2022-01-31 15:21:36 UTC Comment hidden (obsolete)
Comment 14 Tomás Cohen Arazi 2022-01-31 15:24:41 UTC
I renamed the 'base' exception class to Koha::Exception as it is something I wanted to do for a long time already. Beware the package/file name doesn't need to match the exception name in Exception::Class, so we could've left Koha::Exceptions::Exception and still use Koha::Exception->throw if we named it like that. But... this is certainly better :-D

I tried to highlight the expected behaviors with the tests and found some glitches, that are fixed on the 'Fix behavior' patch.

Hope you agree with the proposed follow-ups.
Comment 15 Tomás Cohen Arazi 2022-01-31 16:01:38 UTC
Created attachment 130014 [details] [review]
Bug 29857: Unit tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 16 Tomás Cohen Arazi 2022-01-31 16:01:50 UTC
Created attachment 130015 [details] [review]
Bug 29857: Fix behavior

This patch makes:
- passed parameters be rendered alphabetically to allow testing.
- only the defined parameters be rendered in the string
- the method honor the fact that $self->message should be returned if
  passed to the constructor (i.e. if the developer called
Koha::Exceptions::MyException->throw( $string ) they will expect $string
to be rendered.

To test:
1. Apply the unit tests patch
2. Run:
   $ kshell
  k$ prove t/Koha/Exceptions.t
=> FAIL: Tests fail!
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 17 Martin Renvoize 2022-01-31 19:43:33 UTC
Dude, this is a great set of follow-ups. Will work through them properly and test it tomorrow
Comment 18 Marcel de Rooy 2022-02-04 08:01:52 UTC
Looking here
Comment 19 Marcel de Rooy 2022-02-04 08:42:30 UTC
(In reply to Jonathan Druart from comment #0)
> We must have a generic full_message method in the Koha::Exceptions base
> class to display all the info we have about the exception.
> The current state is bad as we are displaying the "description" of the
> exception but it does not contain useful information.

Exception 'Koha::Exception::TestMR' thrown 'Something went wrong!' with status => x, type => HASH(0x5653eb753278)

How useful is HASH(0x5653eb753278) ?
Comment 20 Marcel de Rooy 2022-02-04 10:09:24 UTC
Looks quite good to me. I am wondering if we should remove the confusion between singular and plural completely now.
We have a base class Koha::Exception.
And we could define subclasses in Koha/Exception/ like Koha/Exception/Generic.pm to contain those now in Exceptions.pm. And the rest already in Exceptions/.
What do you think?
Comment 21 Marcel de Rooy 2022-02-04 10:32:55 UTC Comment hidden (obsolete)
Comment 22 Marcel de Rooy 2022-02-04 10:33:11 UTC
(In reply to Marcel de Rooy from comment #21)
> Created attachment 130156 [details] [review] [review]
> Bug 29857: (CONCEPT) Switch to singular, add a load method
> 
> The load method calls the Class::Exception import. Allowing you
> to skip a use Class::Exception everywhere, and define 'nicer'
> exception lists?

Any thoughts ?
Comment 23 Marcel de Rooy 2022-02-04 10:39:42 UTC
Created attachment 130157 [details] [review]
Bug 29857: (QA follow-up) Add license
Comment 24 Marcel de Rooy 2022-02-04 10:39:46 UTC
Created attachment 130158 [details] [review]
Bug 29857: (CONCEPT) Switch to singular, add a load method

The load method calls the Class::Exception import. Allowing you
to skip a use Class::Exception everywhere, and define 'nicer'
exception lists?
Comment 25 Jonathan Druart 2022-02-08 16:09:13 UTC
IMO we are going too far here. Please provide patches on follow-up bug reports instead.

I don't understand why the "Virtualshelves exceptions" part is in Koha::Exception::Generic.
Comment 26 Jonathan Druart 2022-02-08 16:10:18 UTC
(In reply to Marcel de Rooy from comment #19)
> (In reply to Jonathan Druart from comment #0)
> > We must have a generic full_message method in the Koha::Exceptions base
> > class to display all the info we have about the exception.
> > The current state is bad as we are displaying the "description" of the
> > exception but it does not contain useful information.
> 
> Exception 'Koha::Exception::TestMR' thrown 'Something went wrong!' with
> status => x, type => HASH(0x5653eb753278)
> 
> How useful is HASH(0x5653eb753278) ?

We should call Dumper if it's a struct.
Comment 27 Jonathan Druart 2022-02-08 16:13:01 UTC
(In reply to Jonathan Druart from comment #26)
> (In reply to Marcel de Rooy from comment #19)
> > (In reply to Jonathan Druart from comment #0)
> > > We must have a generic full_message method in the Koha::Exceptions base
> > > class to display all the info we have about the exception.
> > > The current state is bad as we are displaying the "description" of the
> > > exception but it does not contain useful information.
> > 
> > Exception 'Koha::Exception::TestMR' thrown 'Something went wrong!' with
> > status => x, type => HASH(0x5653eb753278)
> > 
> > How useful is HASH(0x5653eb753278) ?
> 
> We should call Dumper if it's a struct.

Marcel, did you find an example in the code or it's theoretical?
Comment 28 Marcel de Rooy 2022-02-09 12:18:54 UTC
(In reply to Jonathan Druart from comment #27)
> (In reply to Jonathan Druart from comment #26)
> > (In reply to Marcel de Rooy from comment #19)
> > > (In reply to Jonathan Druart from comment #0)
> > > > We must have a generic full_message method in the Koha::Exceptions base
> > > > class to display all the info we have about the exception.
> > > > The current state is bad as we are displaying the "description" of the
> > > > exception but it does not contain useful information.
> > > 
> > > Exception 'Koha::Exception::TestMR' thrown 'Something went wrong!' with
> > > status => x, type => HASH(0x5653eb753278)
> > > 
> > > How useful is HASH(0x5653eb753278) ?
> > 
> > We should call Dumper if it's a struct.
> 
> Marcel, did you find an example in the code or it's theoretical?

I do not have a real example. So I am not sure about that. But it isn't far fetched if we are starting to use this more. Dumper could do it indeed.
Comment 29 Marcel de Rooy 2022-02-09 12:19:30 UTC
(In reply to Jonathan Druart from comment #25)
> IMO we are going too far here. Please provide patches on follow-up bug
> reports instead.
> 
> I don't understand why the "Virtualshelves exceptions" part is in
> Koha::Exception::Generic.

Because it is now in Exceptions.pm where it does not belong either :)
Comment 30 Marcel de Rooy 2022-02-09 12:23:32 UTC
(In reply to Jonathan Druart from comment #25)
> IMO we are going too far here. Please provide patches on follow-up bug
> reports instead.

Heh. I see this happen all the time :) As to singular, I think we do half of the work here and then leave it. And we should try more to finish what we started?

But you can remove the concept patch. Not sure if it will reach another report.
Comment 31 Tomás Cohen Arazi 2022-02-09 12:48:27 UTC
Created attachment 130355 [details] [review]
Bug 29857: (QA follow-up) Add license

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 32 Tomás Cohen Arazi 2022-02-09 12:57:35 UTC
(In reply to Marcel de Rooy from comment #22)
> (In reply to Marcel de Rooy from comment #21)
> > Created attachment 130156 [details] [review] [review] [review]
> > Bug 29857: (CONCEPT) Switch to singular, add a load method
> > 
> > The load method calls the Class::Exception import. Allowing you
> > to skip a use Class::Exception everywhere, and define 'nicer'
> > exception lists?
> 
> Any thoughts ?

I like it. It deserves its own bug IMHO.
Comment 33 Tomás Cohen Arazi 2022-02-14 12:17:35 UTC
(In reply to Jonathan Druart from comment #25)
> I don't understand why the "Virtualshelves exceptions" part is in
> Koha::Exception::Generic.

I filed bug 30057 and submitted patches. Test plz
Comment 34 Martin Renvoize 2022-02-25 12:08:47 UTC
Created attachment 131112 [details] [review]
Bug 29857: Make the exception classes inherit from the base class

We have a base class Koha::Exceptions::Exception but it's not used in
most of our exception classes

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 35 Martin Renvoize 2022-02-25 12:08:51 UTC
Created attachment 131113 [details] [review]
Bug 29857: Provide a generic full_message method for our exceptions

We pass useful information when throwing exceptions, but we don't
display them! We must display all the details we need when the exception
is stringified.

Test plan:

Use the following snippet:
  use Koha::Exceptions::Elasticsearch;
  Koha::Exceptions::Elasticsearch::BadResponse->throw( type => "a good type", details => "and some details" );

Execute it without and with the patch.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 36 Martin Renvoize 2022-02-25 12:08:55 UTC
Created attachment 131114 [details] [review]
Bug 29857: Rename base class as Koha::Exception

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 37 Martin Renvoize 2022-02-25 12:08:59 UTC
Created attachment 131115 [details] [review]
Bug 29857: Unit tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 38 Martin Renvoize 2022-02-25 12:09:04 UTC
Created attachment 131116 [details] [review]
Bug 29857: Fix behavior

This patch makes:
- passed parameters be rendered alphabetically to allow testing.
- only the defined parameters be rendered in the string
- the method honor the fact that $self->message should be returned if
  passed to the constructor (i.e. if the developer called
Koha::Exceptions::MyException->throw( $string ) they will expect $string
to be rendered.

To test:
1. Apply the unit tests patch
2. Run:
   $ kshell
  k$ prove t/Koha/Exceptions.t
=> FAIL: Tests fail!
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 39 Martin Renvoize 2022-02-25 12:09:08 UTC
Created attachment 131117 [details] [review]
Bug 29857: (QA follow-up) Add license

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 40 Martin Renvoize 2022-02-25 12:17:07 UTC
Glad to see the next stage put onto it's own bug... The code here works well and clear things up a lot. QA scripts are happy, tests passing.

Passing QA
Comment 41 Fridolin Somers 2022-03-02 07:06:09 UTC
Created attachment 131232 [details] [review]
Bug 29857: (QA follow-up) Fix unit test Object.t

Unit test failling with :
    #   Failed test 'Exception message correct'
    #   at /kohadevbox/koha/t/db_dependent/Koha/Object.t line 335.
     #          got: 'Exception 'Koha::Exception' thrown 'Asked to embed items but its return value doesn't implement to_api'
    # '
    #     expected: 'Asked to embed items but its return value doesn't implement to_api'
     # Looks like you failed 1 test of 29.

Fixed by using $@->message like in other places.

prove t/db_dependent/Koha/Object.t
Comment 42 Fridolin Somers 2022-03-02 07:29:46 UTC
There are other fails in test suite but I push master, we will fix them
Comment 43 Fridolin Somers 2022-03-02 08:48:56 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 44 Tomás Cohen Arazi 2022-03-02 12:29:01 UTC
Created attachment 131240 [details] [review]
Bug 29857: (follow-up) Fix Barcode_transform_hooks.t

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 45 Tomás Cohen Arazi 2022-03-02 12:40:18 UTC
Created attachment 131241 [details] [review]
Bug 29857: (follow-up) Fix t/Koha/Plugins/Tab.t

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 46 Tomás Cohen Arazi 2022-03-02 12:49:44 UTC
Created attachment 131242 [details] [review]
Bug 29857: (follow-up) Fix t/db_dependent/Koha/Biblio.t

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 47 Tomás Cohen Arazi 2022-03-02 12:52:46 UTC
Created attachment 131243 [details] [review]
Bug 29857: (follow-up) Fix t/Koha/Object/Message.t

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 48 Tomás Cohen Arazi 2022-03-02 12:57:55 UTC
Created attachment 131244 [details] [review]
Bug 29857: (follow-up) Fix t/Koha/Email.t

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 49 Tomás Cohen Arazi 2022-03-02 14:37:18 UTC
Created attachment 131256 [details] [review]
Bug 29857: (follow-up) Fix t/db_dependent/Auth.t

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 50 Tomás Cohen Arazi 2022-03-02 14:43:06 UTC
Created attachment 131258 [details] [review]
Bug 29857: (follow-up) Fix t/db_dependent/Koha/Patron/Modifications.t

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 51 Tomás Cohen Arazi 2022-03-02 14:44:00 UTC
Just in case: I'm submitting the fixes one by one just in case I need to leave and cannot complete the fixes. But this could just be pushed on a single commit.
Comment 52 Marcel de Rooy 2022-03-02 14:51:17 UTC
(In reply to Tomás Cohen Arazi from comment #51)
> Just in case: I'm submitting the fixes one by one just in case I need to
> leave and cannot complete the fixes. But this could just be pushed on a
> single commit.

These one line patches read so beautifully ;)
Comment 53 Fridolin Somers 2022-03-02 18:09:10 UTC
Created attachment 131287 [details] [review]
Bug 29857: (follow-up) Fix t/Koha/REST/Plugin/Query.t
Comment 54 Fridolin Somers 2022-03-02 18:09:15 UTC
Created attachment 131288 [details] [review]
Bug 29857: (follow-up) Fix t/db_dependent/Koha/Plugins/Plugins.t
Comment 55 Fridolin Somers 2022-03-02 18:09:20 UTC
Created attachment 131289 [details] [review]
Bug 29857: (follow-up) Fix t/db_dependent/Koha/Club/Hold.t
Comment 56 Fridolin Somers 2022-03-02 18:31:10 UTC
Created attachment 131290 [details] [review]
Bug 29857: (follow-up) Fix t/db_dependent/Koha/BackgroundJobs/BatchUpdateBiblio.t
Comment 57 Fridolin Somers 2022-03-02 18:31:16 UTC
Created attachment 131291 [details] [review]
Bug 29857: (follow-up) Fix t/db_dependent/api/v1/transfer_limits.t
Comment 58 Fridolin Somers 2022-03-02 18:32:46 UTC
Follow-ups pushed to master :

 > git log --oneline --no-decorate --grep=29857
be74dd2edf Bug 29857: (follow-up) Fix t/db_dependent/api/v1/transfer_limits.t
94220261f2 Bug 29857: (follow-up) Fix t/db_dependent/Koha/BackgroundJobs/BatchUpdateBiblio.t
b5560af25e Bug 29857: (follow-up) Fix t/db_dependent/Koha/Club/Hold.t
6ccf04661a Bug 29857: (follow-up) Fix t/db_dependent/Koha/Plugins/Plugins.t
e93de8268e Bug 29857: (follow-up) Fix t/Koha/REST/Plugin/Query.t
13a49f860f Bug 29857: (follow-up) Fix t/db_dependent/Koha/Patron/Modifications.t
d406051dcb Bug 29857: (follow-up) Fix t/db_dependent/Auth.t
8d2c00fb02 Bug 29857: (follow-up) Fix t/Koha/Email.t
e568e8ef5d Bug 29857: (follow-up) Fix t/Koha/Object/Message.t
42c592f3ad Bug 29857: (follow-up) Fix t/db_dependent/Koha/Biblio.t
bcd98589c1 Bug 29857: (follow-up) Fix t/Koha/Plugins/Tab.t
651a2228aa Bug 29857: (follow-up) Fix Barcode_transform_hooks.t
c287121471 Bug 29857: (QA follow-up) Fix unit test Object.t
6ece7185b0 Bug 29857: (QA follow-up) Add license
3951145cf7 Bug 29857: Fix behavior
c10640e8a7 Bug 29857: Unit tests
2b9fae5445 Bug 29857: Rename base class as Koha::Exception
7429f28cfc Bug 29857: Provide a generic full_message method for our exceptions
42096deea4 Bug 29857: Make the exception classes inherit from the base class