Created attachment 58142 [details] [review] Bug 17755: Introduce Koha::Object::Library::Limit This patch introduces a new class for extending Koha::Object using multiple inheritance. It cannot be used standalone, it needs to be used in Koha::Object implementations like this: use base qw( Koha::Object Koha::Object::Library::Limit ); Its goal is to provide a single way and place to deal with this common pattern in Koha's codebase. As it happened with Koha::Object, that needed to be tested in a real object class, this work was done on top of Koha::Patron::Attribute::Type implementation and it is fully covered by the tests that are introduced for it.
Created attachment 58143 [details] [review] Bug 17755: Introduce Koha::Patron::Attribute::Type(s) This patch introduces the Koha::Object-based classes for handling patron attribute types. It also adds branch limitation handling to the Koha::Patron::Attribute::Type class. It is built on top of the new Koha::Object::Library::Limit class that extends Koha::Object so it handles library limits.
Created attachment 58144 [details] [review] Bug 17755: Unit tests This patch introduces unit tests for Koha::Object::Library::Limit. It is done this way because it needs to be instantiated to be usable. To test: - Run: $ prove t/db_dependent/Koha/Patron/Attribute/Types.t => SUCCESS: Tests pass - Sign off :-D
Created attachment 58145 [details] [review] Bug 17755: Introduce Koha::Patron::Attribute::Type(s) This patch introduces the Koha::Object-based classes for handling patron attribute types. It also adds branch limitation handling to the Koha::Patron::Attribute::Type class. It is built on top of the new Koha::Object::Library::Limit class that extends Koha::Object so it handles library limits.
Created attachment 58167 [details] [review] Bug 17755: (followup) Rename ::Library::Limit for ::Limit::Library It makes sense to rename it, as there might be other limits defined, not just library-wise.
Error: "Can't locate object method 'throw' via package 'Koha::Exceptions::CannotAddLibraryLimit' (perhaps you forgot to load 'Koha::Exceptions::CannotAddLibraryLimit'?) at Koha/Object/Limit/Library.pm"
(In reply to Chris Kirby from comment #6) > Error: "Can't locate object method 'throw' via package > 'Koha::Exceptions::CannotAddLibraryLimit' (perhaps you forgot to load > 'Koha::Exceptions::CannotAddLibraryLimit'?) at Koha/Object/Limit/Library.pm" Chris, please find me on IRC or email to check what's going on. Things work fine for me.
Created attachment 58495 [details] [review] Bug 17755: Introduce Koha::Object::Library::Limit This patch introduces a new class for extending Koha::Object using multiple inheritance. It cannot be used standalone, it needs to be used in Koha::Object implementations like this: use base qw( Koha::Object Koha::Object::Library::Limit ); Its goal is to provide a single way and place to deal with this common pattern in Koha's codebase. As it happened with Koha::Object, that needed to be tested in a real object class, this work was done on top of Koha::Patron::Attribute::Type implementation and it is fully covered by the tests that are introduced for it. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 58496 [details] [review] Bug 17755: Unit tests This patch introduces unit tests for Koha::Object::Library::Limit. It is done this way because it needs to be instantiated to be usable. To test: - Run: $ prove t/db_dependent/Koha/Patron/Attribute/Types.t => SUCCESS: Tests pass - Sign off :-D Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 58497 [details] [review] Bug 17755: Introduce Koha::Patron::Attribute::Type(s) This patch introduces the Koha::Object-based classes for handling patron attribute types. It also adds branch limitation handling to the Koha::Patron::Attribute::Type class. It is built on top of the new Koha::Object::Library::Limit class that extends Koha::Object so it handles library limits. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 58498 [details] [review] Bug 17755: (followup) Rename ::Library::Limit for ::Limit::Library It makes sense to rename it, as there might be other limits defined, not just library-wise. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Would not it make more sense to make library_limits returns Koha::Libraries instead of \@branchcodes?
(In reply to Jonathan Druart from comment #12) > Would not it make more sense to make library_limits returns Koha::Libraries > instead of \@branchcodes? I'm not sure. It takes its data from borrower_attribute_types_branch Maybe you can check how it is used and see if the overhead is worth. Once I get to the office we can discuss it, but I'm not against this. I'm not specially attached to this api.
(In reply to Jonathan Druart from comment #12) > Would not it make more sense to make library_limits returns Koha::Libraries > instead of \@branchcodes? What behaviour would you expect when assigning library_limits?
To return $self I think.
Created attachment 59354 [details] [review] Bug 17755: (QA followup) Return $self when appropriate As failure situations raise exceptions that should be handled outside the object class, methods should return $self so successive calls can be chained nicely. This patch makes methods return $self and adjusts the tests to reflect this change. Make sure tests pass: - Run: $ prove t/db_dependent/Koha/Patron/Attribute/Types.t => SUCCESS: Tests return green - Sign off :-D
FAIL Koha/Object/Library/Limit.pm OK critic OK forbidden patterns FAIL git manipulation The file has been added and deleted in the same patchset OK pod SKIP spelling OK valid FAIL Koha/Object/Limit/Library.pm FAIL valid Can't locate Data/Printer.pm in @INC Debugging leftover ?
Created attachment 60989 [details] [review] Bug 17755: Introduce Koha::Object::Limit::Library This patch introduces a new class for extending Koha::Object using multiple inheritance. It cannot be used standalone, it needs to be used in Koha::Object implementations like this: use base qw( Koha::Object Koha::Object::Limit::Library ); Its goal is to provide a single way and place to deal with this common pattern in Koha's codebase. As it happened with Koha::Object, that needed to be tested in a real object class, this work was done on top of Koha::Patron::Attribute::Type implementation and it is fully covered by the tests that are introduced for it. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 60990 [details] [review] Bug 17755: Unit tests This patch introduces unit tests for Koha::Object::Library::Limit. It is done this way because it needs to be instantiated to be usable. To test: - Run: $ prove t/db_dependent/Koha/Patron/Attribute/Types.t => SUCCESS: Tests pass - Sign off :-D Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 60991 [details] [review] Bug 17755: Introduce Koha::Patron::Attribute::Type(s) This patch introduces the Koha::Object-based classes for handling patron attribute types. It also adds branch limitation handling to the Koha::Patron::Attribute::Type class. It is built on top of the new Koha::Object::Library::Limit class that extends Koha::Object so it handles library limits. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 60992 [details] [review] Bug 17755: (QA followup) Return $self when appropriate As failure situations raise exceptions that should be handled outside the object class, methods should return $self so successive calls can be chained nicely. This patch makes methods return $self and adjusts the tests to reflect this change. Make sure tests pass: - Run: $ prove t/db_dependent/Koha/Patron/Attribute/Types.t => SUCCESS: Tests return green - Sign off :-D
Created attachment 60995 [details] [review] Bug 17755: (followup) Override ->search to allow filtering by branchcode Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 61000 [details] [review] Bug 17755: (QA followup) Return $self when appropriate As failure situations raise exceptions that should be handled outside the object class, methods should return $self so successive calls can be chained nicely. This patch makes methods return $self and adjusts the tests to reflect this change. Make sure tests pass: - Run: $ prove t/db_dependent/Koha/Patron/Attribute/Types.t => SUCCESS: Tests return green - Sign off :-D Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 61001 [details] [review] Bug 17755: (followup) Override ->search to allow filtering by branchcode Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 61548 [details] [review] Bug 17755: Introduce Koha::Object::Limit::Library This patch introduces a new class for extending Koha::Object using multiple inheritance. It cannot be used standalone, it needs to be used in Koha::Object implementations like this: use base qw( Koha::Object Koha::Object::Limit::Library ); Its goal is to provide a single way and place to deal with this common pattern in Koha's codebase. As it happened with Koha::Object, that needed to be tested in a real object class, this work was done on top of Koha::Patron::Attribute::Type implementation and it is fully covered by the tests that are introduced for it. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 61549 [details] [review] Bug 17755: Unit tests This patch introduces unit tests for Koha::Object::Library::Limit. It is done this way because it needs to be instantiated to be usable. To test: - Run: $ prove t/db_dependent/Koha/Patron/Attribute/Types.t => SUCCESS: Tests pass - Sign off :-D Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 61550 [details] [review] Bug 17755: Introduce Koha::Patron::Attribute::Type(s) This patch introduces the Koha::Object-based classes for handling patron attribute types. It also adds branch limitation handling to the Koha::Patron::Attribute::Type class. It is built on top of the new Koha::Object::Library::Limit class that extends Koha::Object so it handles library limits. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 61551 [details] [review] Bug 17755: (QA followup) Return $self when appropriate As failure situations raise exceptions that should be handled outside the object class, methods should return $self so successive calls can be chained nicely. This patch makes methods return $self and adjusts the tests to reflect this change. Make sure tests pass: - Run: $ prove t/db_dependent/Koha/Patron/Attribute/Types.t => SUCCESS: Tests return green - Sign off :-D Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 61552 [details] [review] Bug 17755: (followup) Override ->search to allow filtering by branchcode Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
QAed along withbug 17792 and bug 13757. Please push them altogether.
Pushed to master for 17.05, thanks Tomas!
This won't get ported back to 16.11.x as it is an enhancement.