Description
Alexis Ripetti
2020-12-02 19:39:42 UTC
Created attachment 114107 [details] [review] Bug 27136 - Add languages and avoid displaying nothing in facet Created attachment 114119 [details] [review] Bug 27136 - Add languages and avoid displaying nothing in facet The update database here needs to be updated to use skeleton.perl as a base Moving the removal of blank facet to bug 29561 - we should not display anything as the facet cannot be searched. We don't include blank searches for any of the facets Changed status to Failed QA - see comment #3. Created attachment 137748 [details] [review] Bug 27136 - Add languages and avoid displaying nothing in facet Created attachment 137785 [details] [review] Bug 27136 - Add languages and avoid displaying nothing in facet Signed-off-by: Alexis Ripetti <alexis.ripetti@inLibro.com> Oups you also need to add languages in : installer/data/mysql/mandatory/subtag_registry.sql I suggest you split into 2 patches, one for languages and one for facet display Created attachment 139998 [details] [review] Bug 27136: Add additional languages for facet display This patch adds two missing languages, Cree & Afrikaans, as well as some necessary linguistic definitions for faceting: Multi language, undetermined, no linguistic content Signed-off-by: Alexis Ripetti <alexis.ripetti@inLibro.com> Created attachment 139999 [details] [review] Bug 27136: Avoid displaying nothing in facet This patch displays 'No information provided' if a facet value/description cannot be found, rather than displaying an empty/hidden facet Signed-off-by: Alexis Ripetti <alexis.ripetti@inLibro.com> Hi, thx for the patch! Some things: 1) The database update is not idempotent, meaning that you cannot run it a second time without it causing an error: DEV atomic update /kohadevbox/koha/installer/data/mysql/atomicupdate/bz27136-Add_languages_for_facet.pl [22:55:29]: Bug 27136 - Add missing language information for facets ERROR - {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Duplicate entry 'cr-language' for key 'uniq_lang' at /kohadevbox/koha/C4/Installer.pm line 739 As we had some issues with that in recent update (bug 30951), I think it would be better to implement some kind of check. 2) Next time it would be better to have these in 2 separate bugs: * Add languages * Avoid displaying nothing in facets Both are distinct issues and in separate patches it gives better advertising in release notes and better chances one or the other might get backported. A strategy of small granular steps usually also works to make your patches move faster. Maybe a suggestion: We could make this one about adding the languages only as the patches are nice and separate. I tihnk I agree with Nick about not displayin the empty ones, as they can't be searched for - but this discussion could be continued on bug 29561. OK? Created attachment 141239 [details] [review] Bug 27136: Add additional languages for facet display This patch adds two missing languages, Cree & Afrikaans, as well as some necessary linguistic definitions for faceting: Multi language, undetermined, no linguistic content Signed-off-by: Alexis Ripetti <alexis.ripetti@inLibro.com> Created attachment 141241 [details] [review] Bug 27136: (follow-up) Make update idempotent Created attachment 141518 [details] [review] Bug 27136: (QA follow-up) Remove reference to facets, add languages to update description The new languages will also effect other pages in Koha, like the language list on the advanced search page. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 141522 [details] [review] Bug 27136: (QA follow-up) Fix entries for multiple und undetermined language(s) While working on translations for bug 31715 I noticed some more little changes are needed here. (In reply to Katrin Fischer from comment #15) > While working on translations for bug 31715 I noticed some more little > changes are needed here. ... were needed: see second follow-up patch :) Pushed to master for 22.11. Nice work everyone, thanks! |