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 <pedro.amorim@openfifth.co.uk>
This commit is contained in:
Tomás Cohen Arazi 2026-05-12 10:11:33 -03:00 committed by Pedro Amorim
parent e51334604f
commit 713c5c3343
No known key found for this signature in database
GPG Key ID: 06B75BA7F3894D55
12 changed files with 60 additions and 27 deletions

View File

@ -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

View File

@ -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);
}

View File

@ -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

View File

@ -87,8 +87,7 @@ Return the I<Koha::ILL::Requests> 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

View File

@ -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

View File

@ -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;

View File

@ -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;

View File

@ -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;

View File

@ -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;

View File

@ -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;

View File

@ -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

View File

@ -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 {