Created attachment 169081 [details] [review]
Bug 37393: fix "In bundle:" link not showing for items in staff interface

The cause was erroneous check `[% IF bundlesEnabled %]` before the parent bundle information is requested+shown. The `bundlesEnabled` variable checks if the current biblio ITSELF is of collection type, so should only be used for whether to show the button to add new items to the bundle items, NOT for querying whether current item is part of a bundle, as the items that make up the bundle aren't of collection type themselves.

The second fixed problem was that `bundle_host` didn't contain `.biblio` subitem (which'd contain the actual host biblio title) as the JavaScript code assumed, and it wasn't possible to request it with the API either.

Test plan: please follow the reproduction instructions from bug and ensure that the described issue is gone.

If you run into an error 400 on the detail page, you need to refresh API definitions, in kts shell you'drun:

redocly bundle --ext json api/v1/swagger/swagger.yaml --output api/v1/swagger/swagger_bundle.json
koha-plack --restart kohadev

Created attachment 169162 [details] [review]
Bug 37393: fix "In bundle:" link not showing for items in staff interface

The cause was erroneous check `[% IF bundlesEnabled %]` before the parent bundle information is requested+shown. The `bundlesEnabled` variable checks if the current biblio ITSELF is of collection type, so should only be used for whether to show the button to add new items to the bundle items, NOT for querying whether current item is part of a bundle, as the items that make up the bundle aren't of collection type themselves.

The second fixed problem was that `bundle_host` didn't contain `.biblio` subitem (which'd contain the actual host biblio title) as the JavaScript code assumed, and it wasn't possible to request it with the API either.

Test plan: please follow the reproduction instructions from bug and ensure that the described issue is gone.

If you run into an error 400 on the detail page, you need to refresh API definitions, in kts shell you'drun:

redocly bundle --ext json api/v1/swagger/swagger.yaml --output api/v1/swagger/swagger_bundle.json
koha-plack --restart kohadev

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>

Please fix email address of commit author

The e-mail is configured as intended, this is GitHub e-mails that assign commit attribution to users (https://github.com/Koha-Community/Koha/commits?author=mkibp) on the site while providing privacy protection at the same time: https://docs.github.com/en/account-and-profile/setting-up-and-managing-your-personal-account-on-github/managing-email-preferences/setting-your-commit-email-address

I have previously contributed to Koha several times with this e-mail provided and there's nothing in the contribution guidelines that would hint against that, so I don't see a reason for this to be a hindrance.

Otherwise a "real" reachable e-mail may be extracted from commits by bots and then spammed to death, while real people would comment on the offending commit's Bug on the Bugzilla if they wanted to reach out to the author and CC'd people about the changes that were previously introduced, as does happen sometimes. E-mails from Bugzilla can also be nicely filtered by people and sorted into folders on their e-mail client/servers and assigned into "conversation history" view, while random personal e-mails wouldn't.

(In reply to Michał from comment #4)
> The e-mail is configured as intended, this is GitHub e-mails that assign
> commit attribution to users
> (https://github.com/Koha-Community/Koha/commits?author=mkibp) on the site
> while providing privacy protection at the same time:
> https://docs.github.com/en/account-and-profile/setting-up-and-managing-your-
> personal-account-on-github/managing-email-preferences/setting-your-commit-
> email-address
> 
> I have previously contributed to Koha several times with this e-mail
> provided and there's nothing in the contribution guidelines that would hint
> against that, so I don't see a reason for this to be a hindrance.
> 
> Otherwise a "real" reachable e-mail may be extracted from commits by bots
> and then spammed to death, while real people would comment on the offending
> commit's Bug on the Bugzilla if they wanted to reach out to the author and
> CC'd people about the changes that were previously introduced, as does
> happen sometimes. E-mails from Bugzilla can also be nicely filtered by
> people and sorted into folders on their e-mail client/servers and assigned
> into "conversation history" view, while random personal e-mails wouldn't.

Thanks for explaining.

(In reply to Michał from comment #0)

> The cause is erroneous check `[% IF bundlesEnabled %]` before the parent
> bundle information is requested+shown. The `bundlesEnabled` variable checks
> if the current biblio ITSELF is of collection type, so should only be used
> for whether to show the button to add new items to the bundle items, NOT for
> querying whether current item is part of a bundle, as the items that make up
> the bundle aren't of collection type themselves.

The way this bundlesEnabled variable is filled, is quite obscure. It is actually done here:
    $template->param( "$_" => defined $dat->{$_} ? $dat->{$_} : '' );
To me it read more like 'feature enabled' but it indeed depends on the leader.
Your code looks good to me.

Created attachment 174918 [details] [review]
Bug 37393: fix "In bundle:" link not showing for items in staff interface

The cause was erroneous check `[% IF bundlesEnabled %]` before the parent bundle information is requested+shown. The `bundlesEnabled` variable checks if the current biblio ITSELF is of collection type, so should only be used for whether to show the button to add new items to the bundle items, NOT for querying whether current item is part of a bundle, as the items that make up the bundle aren't of collection type themselves.

The second fixed problem was that `bundle_host` didn't contain `.biblio` subitem (which'd contain the actual host biblio title) as the JavaScript code assumed, and it wasn't possible to request it with the API either.

Test plan: please follow the reproduction instructions from bug and ensure that the described issue is gone.

If you run into an error 400 on the detail page, you need to refresh API definitions, in kts shell you'drun:

redocly bundle --ext json api/v1/swagger/swagger.yaml --output api/v1/swagger/swagger_bundle.json
koha-plack --restart kohadev

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Martin, care to have a quick look at this one?

(In reply to Marcel de Rooy from comment #8)
> Martin, care to have a quick look at this one?

*ping*

I had to stare at it for a while, but I think I get it now.

This removes the checks from parts, where we don't expect a set record as it's about displaying the bundled items, not the bundle item...

Pushed for 25.05!

Well done everyone, thank you!

Agreed, I finally found a moment and it looks solid.

Nice work everyone!

Pushed to 24.11.x for 24.11.01

Backported to 24.05.x for 24.05.06

Not for 23.11.x

Manual updates
--------------

The bundle feature is documented at https://koha-community.org/manual/latest/en/html/circulation.html#circulating-bundles

This is a bug that fixes something that is not working as it should, so normally no manual update would be required.

However, the screenshots need updating, so I have left the "Needs documenting" status.