https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32680
Nick Clemens <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |[email protected] Status|Signed Off |Failed QA --- Comment #28 from Nick Clemens <[email protected]> --- I like this idea, it expands upon the current: CustomCoverImagesURL system preference, and would allow for adding services that need authentication, or more specialized formats. There are a few issues I see: 1 - In both the staff and opac we have lines like below in the scripts: use Koha::Template::Plugin::KohaPlugins; ... my $intranet_cover_images_plugins = Koha::Template::Plugin::KohaPlugins->get_plugins_intranet_cover_images; if($intranet_cover_images_plugins){ $template->param( CoverImagesRequired => 1 ) } I don't think we should be calling template plugins in the scripts. We can do something like: [% SET CoverImagesRequired = KohaPlugins.get_plugins_intranet_cover_images %] early in the template, then add the code in later like: [% CoverImagesRequired | $raw %] 2 - I am not sure the code below is necessary, we add normalized isbn to the results, and the plugin seems to simply fetch the results with JS " const search_results_images = document.querySelectorAll('.cover_images_required');" Am I correct? + [% IF ( CoverImagesRequired ) %] + <script> + const search_results = {}; + [% FOREACH SEARCH_RESULT IN SEARCH_RESULTS %] + var cover_index = "[% loop.count | html %]"; + search_results[cover_index] = {}; + search_results[cover_index].isbn = "[% SEARCH_RESULT.normalized_isbn | html %]"; + search_results[cover_index].biblionumber = "[% SEARCH_RESULT.biblionumber | html %]"; + search_results[cover_index].processedBiblio = "[% PROCESS biblio_a_href biblionumber => SEARCH_RESULT.biblionumber | html %]"; + [% END %] + </script> + [% END %] 3 - CoverImagesRequired is misleading. I think it should be renamed like 'CoverImagePlugins' 4 - I would suggest squashing the patches, as the second rewrites the first, it would make for easier review -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
