Bug 24464

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