From 713c5c33431d6c6f52f21bb2fae71fb671ae3b9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Cohen=20Arazi?= Date: Tue, 12 May 2026 10:11:33 -0300 Subject: [PATCH] Bug 41950: (follow-up) Add DBIC relationship aliases for +count embeds Adds DBIC relationship aliases so that +count embeds advertised in the API specs are backed by actual database relationships, making them sortable via the SQL COUNT subquery infrastructure. New aliases: - Borrower: checkouts (issues) - Aqbookseller: baskets (aqbaskets), invoices (aqinvoices) - Ticket: updates (ticket_updates) - ErmEholdingsPackage: resources (erm_eholdings_resources) - Biblio: holds (reserves) Koha methods are updated to use the new relationship names directly, following the one-liner style: Koha::Foo->_new_from_dbic( scalar ... ) Note: checkouts on Borrower and subscriptions on Aqbookseller already had matching DBIC relationships. comments on Illrequest, requests on IllBatch, and items on Biblio also already existed. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/Koha/REST/Plugin/Query.t \ t/db_dependent/api/v1/patrons.t \ t/db_dependent/api/v1/acquisitions_vendors.t \ t/db_dependent/Koha/REST/Plugin/Objects.t \ t/db_dependent/Koha/Biblio.t => SUCCESS: All tests pass 3. Verify sorting works: GET /api/v1/patrons x-koha-embed: checkouts+count _order_by: -me.checkouts_count => SUCCESS: Patrons sorted by checkout count 4. Sign off :-D Assisted-by: Sonnet 4.6 (Anthropic) Signed-off-by: Pedro Amorim --- Koha/Acquisition/Bookseller.pm | 9 +++------ Koha/Biblio.pm | 2 +- Koha/ERM/EHoldings/Package.pm | 3 +-- Koha/ILL/Batch.pm | 3 +-- Koha/Patron.pm | 3 +-- Koha/Schema/Result/Aqbookseller.pm | 14 ++++++++++++++ Koha/Schema/Result/Biblio.pm | 7 +++++++ Koha/Schema/Result/Borrower.pm | 7 +++++++ Koha/Schema/Result/ErmEholdingsPackage.pm | 7 +++++++ Koha/Schema/Result/Ticket.pm | 8 +++++++- Koha/Ticket.pm | 4 +--- t/Koha/REST/Plugin/Query.t | 20 ++++++++++---------- 12 files changed, 60 insertions(+), 27 deletions(-) diff --git a/Koha/Acquisition/Bookseller.pm b/Koha/Acquisition/Bookseller.pm index 373888f1eb..24b452f55b 100644 --- a/Koha/Acquisition/Bookseller.pm +++ b/Koha/Acquisition/Bookseller.pm @@ -46,8 +46,7 @@ Returns the list of baskets for the vendor sub baskets { my ($self) = @_; - my $baskets_rs = $self->_result->aqbaskets; - return Koha::Acquisition::Baskets->_new_from_dbic($baskets_rs); + return Koha::Acquisition::Baskets->_new_from_dbic( scalar $self->_result->baskets ); } =head3 contacts @@ -112,8 +111,7 @@ Returns the list of subscriptions for the vendor sub subscriptions { my ($self) = @_; - my $rs = $self->_result->subscriptions; - return Koha::Subscriptions->_new_from_dbic($rs); + return Koha::Subscriptions->_new_from_dbic( scalar $self->_result->subscriptions ); } =head3 aliases @@ -198,8 +196,7 @@ Returns the list of invoices for the vendor sub invoices { my ($self) = @_; - my $invoices_rs = $self->_result->aqinvoices; - return Koha::Acquisition::Invoices->_new_from_dbic($invoices_rs); + return Koha::Acquisition::Invoices->_new_from_dbic( scalar $self->_result->invoices ); } =head3 to_api_mapping diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm index 4919c8baa3..833b9bbb80 100644 --- a/Koha/Biblio.pm +++ b/Koha/Biblio.pm @@ -799,7 +799,7 @@ return the current holds placed on this record sub holds { my ( $self, $params, $attributes ) = @_; $attributes->{order_by} = 'priority' unless exists $attributes->{order_by}; - my $hold_rs = $self->_result->reserves->search( $params, $attributes ); + my $hold_rs = $self->_result->holds->search( $params, $attributes ); return Koha::Holds->_new_from_dbic($hold_rs); } diff --git a/Koha/ERM/EHoldings/Package.pm b/Koha/ERM/EHoldings/Package.pm index 55f3a7d889..e4a5cec12c 100644 --- a/Koha/ERM/EHoldings/Package.pm +++ b/Koha/ERM/EHoldings/Package.pm @@ -67,8 +67,7 @@ Returns the resources from this package sub resources { my ($self) = @_; - my $rs = $self->_result->erm_eholdings_resources; - return Koha::ERM::EHoldings::Resources->_new_from_dbic($rs); + return Koha::ERM::EHoldings::Resources->_new_from_dbic( scalar $self->_result->resources ); } =head3 vendor diff --git a/Koha/ILL/Batch.pm b/Koha/ILL/Batch.pm index 76ad38149e..15a8799aa7 100644 --- a/Koha/ILL/Batch.pm +++ b/Koha/ILL/Batch.pm @@ -87,8 +87,7 @@ Return the I for this batch sub requests { my ($self) = @_; - my $requests = $self->_result->requests; - return Koha::ILL::Requests->_new_from_dbic($requests); + return Koha::ILL::Requests->_new_from_dbic( scalar $self->_result->requests ); } =head3 create_and_log diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 97528b07e0..37d1e63b96 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -1728,8 +1728,7 @@ my $checkouts = $patron->checkouts sub checkouts { my ($self) = @_; - my $checkouts = $self->_result->issues; - return Koha::Checkouts->_new_from_dbic($checkouts); + return Koha::Checkouts->_new_from_dbic( scalar $self->_result->checkouts ); } =head3 ill_requests diff --git a/Koha/Schema/Result/Aqbookseller.pm b/Koha/Schema/Result/Aqbookseller.pm index 431594841e..cb79bfdf4a 100644 --- a/Koha/Schema/Result/Aqbookseller.pm +++ b/Koha/Schema/Result/Aqbookseller.pm @@ -627,4 +627,18 @@ sub koha_objects_class { 'Koha::Acquisition::Booksellers'; } +__PACKAGE__->has_many( + "baskets", + "Koha::Schema::Result::Aqbasket", + { "foreign.booksellerid" => "self.id" }, + { cascade_copy => 0, cascade_delete => 0 }, +); + +__PACKAGE__->has_many( + "invoices", + "Koha::Schema::Result::Aqinvoice", + { "foreign.booksellerid" => "self.id" }, + { cascade_copy => 0, cascade_delete => 0 }, +); + 1; diff --git a/Koha/Schema/Result/Biblio.pm b/Koha/Schema/Result/Biblio.pm index 1793f2335c..b4222316a1 100644 --- a/Koha/Schema/Result/Biblio.pm +++ b/Koha/Schema/Result/Biblio.pm @@ -692,4 +692,11 @@ __PACKAGE__->add_columns( "+serial" => { is_boolean => 1 }, ); +__PACKAGE__->has_many( + "holds", + "Koha::Schema::Result::Reserve", + { "foreign.biblionumber" => "self.biblionumber" }, + { cascade_copy => 0, cascade_delete => 0 }, +); + 1; diff --git a/Koha/Schema/Result/Borrower.pm b/Koha/Schema/Result/Borrower.pm index 37e7f7cf75..081a4b57f3 100644 --- a/Koha/Schema/Result/Borrower.pm +++ b/Koha/Schema/Result/Borrower.pm @@ -2311,4 +2311,11 @@ sub koha_object_class { 'Koha::Patron'; } +__PACKAGE__->has_many( + "checkouts", + "Koha::Schema::Result::Issue", + { "foreign.borrowernumber" => "self.borrowernumber" }, + { cascade_copy => 0, cascade_delete => 0 }, +); + 1; diff --git a/Koha/Schema/Result/ErmEholdingsPackage.pm b/Koha/Schema/Result/ErmEholdingsPackage.pm index 939e85849c..f71d02f33c 100644 --- a/Koha/Schema/Result/ErmEholdingsPackage.pm +++ b/Koha/Schema/Result/ErmEholdingsPackage.pm @@ -253,4 +253,11 @@ sub koha_objects_class { 'Koha::ERM::EHoldings::Packages'; } +__PACKAGE__->has_many( + "resources", + "Koha::Schema::Result::ErmEholdingsResource", + { "foreign.package_id" => "self.package_id" }, + { cascade_copy => 0, cascade_delete => 0 }, +); + 1; diff --git a/Koha/Schema/Result/Ticket.pm b/Koha/Schema/Result/Ticket.pm index abc639a055..e1453a6639 100644 --- a/Koha/Schema/Result/Ticket.pm +++ b/Koha/Schema/Result/Ticket.pm @@ -267,5 +267,11 @@ __PACKAGE__->has_many( # DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:8bWiSb7hXPFYRzRzrVG3kw -# You can replace this text with custom code or comments, and it will be preserved on regeneration +__PACKAGE__->has_many( + "updates", + "Koha::Schema::Result::TicketUpdate", + { "foreign.ticket_id" => "self.id" }, + { cascade_copy => 0, cascade_delete => 0 }, +); + 1; diff --git a/Koha/Ticket.pm b/Koha/Ticket.pm index ed61892d83..24183a6a14 100644 --- a/Koha/Ticket.pm +++ b/Koha/Ticket.pm @@ -94,9 +94,7 @@ Return any updates attached to this ticket sub updates { my ($self) = @_; - my $rs = $self->_result->ticket_updates; - return unless $rs; - return Koha::Ticket::Updates->_new_from_dbic($rs) if $rs; + return Koha::Ticket::Updates->_new_from_dbic( scalar $self->_result->updates ); } =head2 Actions diff --git a/t/Koha/REST/Plugin/Query.t b/t/Koha/REST/Plugin/Query.t index 68d30cb72f..d175b14f2b 100755 --- a/t/Koha/REST/Plugin/Query.t +++ b/t/Koha/REST/Plugin/Query.t @@ -209,11 +209,11 @@ get '/dbic_merge_prefetch_count_sorted' => sub { get '/dbic_merge_prefetch_count_unsortable' => sub { my $c = shift; - my $attributes = { order_by => [ { '-desc' => 'me.checkouts_count' } ] }; + my $attributes = { order_by => [ { '-desc' => 'me.no_dbic_rel_count' } ] }; my $result_set = Koha::Patrons->new; - # checkouts is a Koha-only method, not a DBIC relationship - $c->stash( 'koha.embed', { "checkouts_count" => { "is_count" => 1 } } ); + # no_dbic_rel is not a DBIC relationship + $c->stash( 'koha.embed', { "no_dbic_rel_count" => { "is_count" => 1 } } ); try { $c->dbic_merge_prefetch( @@ -237,8 +237,8 @@ get '/dbic_merge_prefetch_count_unsortable_display' => sub { my $attributes = {}; my $result_set = Koha::Patrons->new; - # checkouts is a Koha-only method — no order_by, so no error - $c->stash( 'koha.embed', { "checkouts_count" => { "is_count" => 1 } } ); + # no_dbic_rel is not a DBIC relationship — no order_by, so no error + $c->stash( 'koha.embed', { "no_dbic_rel_count" => { "is_count" => 1 } } ); $c->dbic_merge_prefetch( { @@ -281,13 +281,13 @@ get '/dbic_merge_prefetch_nested_count' => sub { get '/dbic_merge_prefetch_nested_count_unsortable' => sub { my $c = shift; - my $attributes = { order_by => [ { '-desc' => 'guarantee.checkouts_count' } ] }; + my $attributes = { order_by => [ { '-desc' => 'guarantee.no_dbic_rel_count' } ] }; my $result_set = Koha::Patron::Relationship->new; - # checkouts is NOT in Patron's prefetch_whitelist + # no_dbic_rel is NOT in Patron's prefetch_whitelist $c->stash( 'koha.embed', - { "guarantee" => { "children" => { "checkouts_count" => { "is_count" => 1 } } } } + { "guarantee" => { "children" => { "no_dbic_rel_count" => { "is_count" => 1 } } } } ); try { @@ -628,7 +628,7 @@ subtest '/dbic_merge_prefetch' => sub { $t->get_ok('/dbic_merge_prefetch_count_unsortable') ->status_is(400) ->json_is( '/error_code' => 'bad_parameter' ) - ->json_like( '/error' => qr/Cannot sort on checkouts_count/ ); + ->json_like( '/error' => qr/Cannot sort on no_dbic_rel_count/ ); # Top-level unsortable +count without order_by: no error (display-only fallback) $t->get_ok('/dbic_merge_prefetch_count_unsortable_display') @@ -647,7 +647,7 @@ subtest '/dbic_merge_prefetch' => sub { $t->get_ok('/dbic_merge_prefetch_nested_count_unsortable') ->status_is(400) ->json_is( '/error_code' => 'bad_parameter' ) - ->json_like( '/error' => qr/Cannot sort on guarantee\.checkouts_count/ ); + ->json_like( '/error' => qr/Cannot sort on guarantee\.no_dbic_rel_count/ ); }; subtest '/merge_q_params' => sub {