|
Description
Laura Escamilla
2023-03-08 12:23:14 UTC
Created attachment 148029 [details] [review] Bug 33167: Cleanup staff detail page This patch begins reduing some of the extra things we are doing in detail.pl that could easily be handled in the templates - fetching authorised values and branches etc It also removes a loop to find items that should be hidden, and instead uses a searh parameter The template changes either use item object rather than passed variables, or utilize plugins to fetch authorised valued Template also includes addition of chomp to the template directives, this reduces the size of the items table by several megabytes on records with many items WIP Created attachment 148082 [details] [review] Bug 33167: Cleanup staff detail page This patch begins reduing some of the extra things we are doing in detail.pl that could easily be handled in the templates - fetching authorised values and branches etc It also removes a loop to find items that should be hidden, and instead uses a searh parameter The template changes either use item object rather than passed variables, or utilize plugins to fetch authorised valued Template also includes addition of chomp to the template directives, this reduces the size of the items table by several megabytes on records with many items WIP Created attachment 148083 [details] [review] Bug 33167: Add 'host_items' param to Koha::Biblio->items This patch adds an option to the $biblio->items method to allow retrieving the items and analytic items for a record. This is intended to allow fetching a single Items object, and related object, rather than having to fetch the items, and the host items, and push them together To test: 1 - prove -v t/db_dependent/Koha/Biblio.t Created attachment 148084 [details] [review] Bug 33167: Use 'host_items' param to fetch all items at once This patch adjusts the detail page to fetch items and host items together, and prefetches transfers and checkouts To test: 1 - Enabl easy analytics 2 - Attach some items to a bib 3 - Checkout an item on the bib 4 - View the details page 5 - Apply patch 6 - View details page, confirm nothign has changed Created attachment 148085 [details] [review] Bug 33167: Add a filter rleationship for branchtransfer Prefetching is useful, but we don't want to fetch all the inactive transfers for an item. This patch adds a filtered relationship and updates get_transfer to use this Is this work in progress or ready for testing? Created attachment 148431 [details] [review] Bug 33167: Cleanup staff detail page This patch begins reduing some of the extra things we are doing in detail.pl that could easily be handled in the templates - fetching authorised values and branches etc It also removes a loop to find items that should be hidden, and instead uses a searh parameter The template changes either use item object rather than passed variables, or utilize plugins to fetch authorised valued Created attachment 148432 [details] [review] Bug 33167: Add 'host_items' param to Koha::Biblio->items This patch adds an option to the $biblio->items method to allow retrieving the items and analytic items for a record. This is intended to allow fetching a single Items object, and related object, rather than having to fetch the items, and the host items, and push them together This is step towards being able to fetch items using API/DataTables directly To test: 1 - prove -v t/db_dependent/Koha/Biblio.t Created attachment 148433 [details] [review] Bug 33167: Use 'host_items' param to fetch all items at once This patch adjusts the detail page to fetch items and host items together, and prefetches transfers and checkouts To test: 1 - Enable easy analytics 2 - Attach some items to a bib 3 - Checkout an item on the bib 4 - View the details page 5 - Apply patch 6 - View details page, confirm nothign has changed Created attachment 148434 [details] [review] Bug 33167: Add a filter relationship for branchtransfers Prefetching is useful, but we don't want to fetch all the inactive transfers for an item. This patch adds a filtered relationship and updates get_transfer to use this (In reply to David Cook from comment #6) > Is this work in progress or ready for testing? This can now be tested, template chomping removed in favor of bug 20657 (In reply to Nick Clemens from comment #11) > (In reply to David Cook from comment #6) > > Is this work in progress or ready for testing? > > This can now be tested, template chomping removed in favor of bug 20657 It won't completely resolve slowness, but it should offer an improvement where there are many items on a bib I'm not sure what is required to test this one - apply the patch, and repeat the tests for bug 33493 and 33497? (In reply to David Nind from comment #13) > I'm not sure what is required to test this one - apply the patch, and repeat > the tests for bug 33493 and 33497? This one shouldn't cause any visible changes to the detail page - best test plan would be to add items with varying statuses: lost damage withdrawn in transit checked out to a record, and confirm the page looks the same before and after I noticed these differences from my testing (see 'How I tested' at the end), and have changed the status to Failed QA: 1. Checked out items: - Before the patch, a checked out item shows in the status column as "Checked out to Mary Burton (23529000651225) : due 04/18/2023" - After the patch, the checked out item shows as "Checked out to A patron from library : due 04/19/2023" 2. For a record that is linked to a host item (the child item - in this case http://127.0.0.1:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=262) there is this text after the DataTable (after the 'Showing 1 to 2 of 2 entries'): "Show all items (-1 hidden)". The text is an active link (in this case to http://127.0.0.1:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=262&showallitems=1). Let me know if you would like me to add some screenshots. How I tested (using KTD): 1. Setup some analytics using easy analytics: - Enable Easy Analytics (EasyAnalyticalRecords system preference) - Attach some items to a host record - Checkout an item on the host record - View the record details page 2. For an existing record: - Duplicate items so that there are at least 6 items - Mark separate items as lost, damaged, and withdrawn - Check out an item - Place a hold on an item - Transfer an item to another library - View the details page for the record 3. Apply the patch. 4. Review the details pages for the records in another browser - they should look the same before and after. 5. The tests should pass: prove -v t/db_dependent/Koha/Biblio.t + [% SET item_locatoin = AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.location', authorised_value => item.location ) %] There is a typo here. Created attachment 149677 [details] [review] Bug 33167: Cleanup staff detail page This patch begins reduing some of the extra things we are doing in detail.pl that could easily be handled in the templates - fetching authorised values and branches etc It also removes a loop to find items that should be hidden, and instead uses a searh parameter The template changes either use item object rather than passed variables, or utilize plugins to fetch authorised valued Created attachment 149704 [details] [review] Bug 33167: Cleanup staff detail page This patch begins reduing some of the extra things we are doing in detail.pl that could easily be handled in the templates - fetching authorised values and branches etc It also removes a loop to find items that should be hidden, and instead uses a searh parameter The template changes either use item object rather than passed variables, or utilize plugins to fetch authorised valued Signed-off-by: David Nind <david@davidnind.com> item.object.itemnotes.replace('\n','<br />')
Is this replace relevant? How do we test it?
(In reply to Jonathan Druart from comment #19) > item.object.itemnotes.replace('\n','<br />') > > Is this replace relevant? How do we test it? 1. Enter an itemnote note longer than 100 characters 2. Save the item 3. Edit the item, note field should now be a text area 4. Enter: a b c d 5. Save the item 6. View details page 7. Confirm line breaks are preserved Created attachment 150075 [details] [review] Bug 33167: Cleanup staff detail page This patch begins reduing some of the extra things we are doing in detail.pl that could easily be handled in the templates - fetching authorised values and branches etc It also removes a loop to find items that should be hidden, and instead uses a searh parameter The template changes either use item object rather than passed variables, or utilize plugins to fetch authorised valued Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 150076 [details] [review] Bug 33167: Fix TT comment for better translatability We just figured out that multi-line TT comments show up weirdly in translations and fixed it on bug 33332. So this just follows the newly established pattern. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> I am also stumbling on this change:
[% IF ( itemdata_itemnotes ) %]
<td><div class="itemnotes">[% item.object.itemnotes.replace('\n','<br />') | $raw %]</div></td>
[% END %]
[% IF itemdata_nonpublicnotes %]
<td class="nonpublicnote">[% item.itemnotes_nonpublic | html %]</td>
[% END %]
Why do this for the one note and not the other? It was also not there before and we allow HTML in this field. It seems counterintiuitive to allow both <br> and manual line breaks maybe. But: Not a blocker for me.
Pushed to master for 23.05. Nice work everyone, thanks! Nice work everyone! Pushed to stable for 22.11.x (In reply to Matt Blenkinsop from comment #25) > Nice work everyone! > > Pushed to stable for 22.11.x hi Matt, this patchset depends on 33070, which is missing from 22.11.x we probably need to revert this from 22.11.06, as it breaks catalogue/detail.pl I can confirm this is causing issues for us on 22.11.x. Over the weekend 22.11.6 was installed and I began to get reports about 500 errors.
The have all been on catalogue/detail.pl line 411
Trace begun at /usr/share/koha/lib/Koha/Object.pm line 925
Koha::Object::AUTOLOAD('Koha::Patron=HASH(0x562a086602e8)', 'LET') called at /usr/share/koha/intranet/cgi-bin/catalogue/detail.pl line 411
My error went away after I upgraded the debian package to 22.11.06-3. I am still seeing this error, even after upgrading to 22.11.06-3. (In reply to Barry Cannon from comment #29) > I am still seeing this error, even after upgrading to 22.11.06-3. hi Barry can you paste your error message? it should be different from above also try rebuilding your languages, with koha-translate tool Missing dependencies for 22.05.x, no backport Backport to 22.11.x was reverted post release - updating bug to reflect this |