Bug 24464 - Add K::A::Basket->creator
Summary: Add K::A::Basket->creator
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact:
URL:
Keywords:
Depends on: 24463
Blocks: 24918
  Show dependency treegraph
 
Reported: 2020-01-20 18:58 UTC by Tomás Cohen Arazi
Modified: 2020-11-30 21:48 UTC (History)
5 users (show)

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


Attachments
Bug 24464: Unit tests (1.68 KB, patch)
2020-01-20 19:03 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 24464: Add K::A::Basket->creator (2.54 KB, patch)
2020-01-20 19:03 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 24463: Add a basket_group relationship for baskets (1.83 KB, patch)
2020-02-03 08:00 UTC, David Nind
Details | Diff | Splinter Review
Bug 24464: Unit tests (1.73 KB, patch)
2020-02-03 09:03 UTC, David Nind
Details | Diff | Splinter Review
Bug 24464: Add K::A::Basket->creator (2.59 KB, patch)
2020-02-03 09:03 UTC, David Nind
Details | Diff | Splinter Review
Bug 24464: Add K::A::Basket->creator (1.88 KB, patch)
2020-03-20 19:03 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 24464: (QA follow-up) Make the method reflect the lack of FK (1.12 KB, patch)
2020-03-20 19:03 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 24464: Unit tests (1.80 KB, patch)
2020-03-23 09:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24464: Add K::A::Basket->creator (1.95 KB, patch)
2020-03-23 09:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24464: (QA follow-up) Make the method reflect the lack of FK (1.19 KB, patch)
2020-03-23 09:55 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2020-01-20 18:58:51 UTC
We need an accessor to fetch the Koha::Patron for the basket creator (a.k.a. authorisedby on the DB).
Comment 1 Tomás Cohen Arazi 2020-01-20 19:03:22 UTC
Created attachment 97631 [details] [review]
Bug 24464: Unit tests
Comment 2 Tomás Cohen Arazi 2020-01-20 19:03:25 UTC
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
Comment 3 David Nind 2020-02-03 08:00:03 UTC
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
Comment 4 David Nind 2020-02-03 08:11:23 UTC
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!
Comment 5 Katrin Fischer 2020-02-03 08:47:56 UTC
Hi David, I fixed it, the way is: Details > Edit details > Obsolete. Do you want to try again?
Comment 6 David Nind 2020-02-03 09:00:03 UTC
Thanks Katrin!
Comment 7 David Nind 2020-02-03 09:03:27 UTC
Created attachment 98275 [details] [review]
Bug 24464: Unit tests

Signed-off-by: David Nind <david@davidnind.com>
Comment 8 David Nind 2020-02-03 09:03:30 UTC
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>
Comment 9 Jonathan Druart 2020-02-21 10:42:55 UTC
Changes to to_api_mapping should not be part of this patchset.
Comment 10 Jonathan Druart 2020-02-21 10:48:46 UTC
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 );
}
Comment 11 Tomás Cohen Arazi 2020-03-20 17:29:32 UTC
(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?
Comment 12 Tomás Cohen Arazi 2020-03-20 17:29:48 UTC
(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.
Comment 13 Tomás Cohen Arazi 2020-03-20 19:03:24 UTC
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>
Comment 14 Tomás Cohen Arazi 2020-03-20 19:03:29 UTC
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>
Comment 15 Jonathan Druart 2020-03-23 09:55:39 UTC
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>
Comment 16 Jonathan Druart 2020-03-23 09:55:43 UTC
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>
Comment 17 Jonathan Druart 2020-03-23 09:55:47 UTC
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>
Comment 18 Martin Renvoize 2020-03-24 09:30:10 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 19 Joy Nelson 2020-04-03 21:26:55 UTC
enhancement not backported to 19.11.x