We need an accessor to fetch the Koha::Patron for the basket creator (a.k.a. authorisedby on the DB).
Created attachment 97631 [details] [review] Bug 24464: Unit tests
Created attachment 97632 [details] [review] Bug 24464: Add K::A::Basket->creator This patch adds a ->creator method to Koha::Acquisition::Basket to retrieve the related Koha::Patron object. Note: this should be re-done once 22658 is pushed, but at the time being, there's not patch submitted, and it is not a trivial task because of the column type missmatch. That said, the implementation of ->creator in this class is simple enough to just rewrite it once 22658 is submitted, and the tests will reflect the expected/unchanged behavior. To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/db_dependent/Koha/Acquisition/Basket.t => SUCCESS: Tests pass! 3. Sign off :-D
Created attachment 98272 [details] [review] Bug 24463: Add a basket_group relationship for baskets This patch aims to bring consistency between the accessor name and the underlying relationship to ease things when using it from the API. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Acquisition/Basket.t => SUCCESS: Tests pass! No behavior change! 3. Sign off :-D
Hi Tomás. I think I've somehow mucked up the sign-off - I've somehow added Bug 24463 patch to it as well.. If you could let me know how to fix it up, or reset it so I can sign-off again, that would be great!
Hi David, I fixed it, the way is: Details > Edit details > Obsolete. Do you want to try again?
Thanks Katrin!
Created attachment 98275 [details] [review] Bug 24464: Unit tests Signed-off-by: David Nind <david@davidnind.com>
Created attachment 98276 [details] [review] Bug 24464: Add K::A::Basket->creator This patch adds a ->creator method to Koha::Acquisition::Basket to retrieve the related Koha::Patron object. Note: this should be re-done once 22658 is pushed, but at the time being, there's not patch submitted, and it is not a trivial task because of the column type missmatch. That said, the implementation of ->creator in this class is simple enough to just rewrite it once 22658 is submitted, and the tests will reflect the expected/unchanged behavior. To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/db_dependent/Koha/Acquisition/Basket.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com>
Changes to to_api_mapping should not be part of this patchset.
Also, I think the method should reflect the lack of FK, something like: sub creator { my ($self) = @_; my $borrowernumber = $self->authorisedby; # FIXME missing FK here return unless $borrowernumber; return Koha::Patrons->find( $self->authorisedby ); }
(In reply to Jonathan Druart from comment #9) > Changes to to_api_mapping should not be part of this patchset. Would you accept a follow-up patch here?
(In reply to Jonathan Druart from comment #10) > Also, I think the method should reflect the lack of FK, something like: > sub creator { > my ($self) = @_; > my $borrowernumber = $self->authorisedby; # FIXME missing FK here > return unless $borrowernumber; > return Koha::Patrons->find( $self->authorisedby ); > } Ok, will do.
Created attachment 101316 [details] [review] Bug 24464: Add K::A::Basket->creator This patch adds a ->creator method to Koha::Acquisition::Basket to retrieve the related Koha::Patron object. Note: this should be re-done once 22658 is pushed, but at the time being, there's not patch submitted, and it is not a trivial task because of the column type missmatch. That said, the implementation of ->creator in this class is simple enough to just rewrite it once 22658 is submitted, and the tests will reflect the expected/unchanged behavior. To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/db_dependent/Koha/Acquisition/Basket.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com>
Created attachment 101317 [details] [review] Bug 24464: (QA follow-up) Make the method reflect the lack of FK This patch makes it super obvious to understand by reading the code that there's originally no foreign key. To test: 1. Run the tests and notice they pass 2. Spot the # FIXME 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 101409 [details] [review] Bug 24464: Unit tests Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 101410 [details] [review] Bug 24464: Add K::A::Basket->creator This patch adds a ->creator method to Koha::Acquisition::Basket to retrieve the related Koha::Patron object. Note: this should be re-done once 22658 is pushed, but at the time being, there's not patch submitted, and it is not a trivial task because of the column type missmatch. That said, the implementation of ->creator in this class is simple enough to just rewrite it once 22658 is submitted, and the tests will reflect the expected/unchanged behavior. To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/db_dependent/Koha/Acquisition/Basket.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 101411 [details] [review] Bug 24464: (QA follow-up) Make the method reflect the lack of FK This patch makes it super obvious to understand by reading the code that there's originally no foreign key. To test: 1. Run the tests and notice they pass 2. Spot the # FIXME 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Nice work everyone! Pushed to master for 20.05
enhancement not backported to 19.11.x