[Koha-bugs] [Bug 20307] Language overlay for authorized values

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20307

--- Comment #97 from Jonathan Druart <jonathan.druart at bugs.koha-community.org> ---
(In reply to Julian Maurice from comment #96)

Thanks Julian!

> A few complaints from QA script:
> 
>  FAIL   Koha/Schema/Result/AuthorisedValueLocalization.pm
>    FAIL   pod coverage
>                 POD coverage was greater before, try perl
> -MPod::Coverage=PackageName -e666

=> False positive, there is no sub there.

>  FAIL   installer/data/mysql/atomicupdate/bug_20307.sql
>    FAIL   git manipulation
>                 The file has been added and deleted in the same patchset
> 
> So, maybe patches should be squashed.

Different authors.

> Also:
> 1) The value stored in localization.code is a concatenation of the AV
> category code, a '_', and the AV code. Which means that if we have a
> category A with a value B_C, and a category A_B with a value C, they will
> share their translations.

I can pick something else than '_', but the problem will exist for any
characters.
% or $ would fit better?

> 2) It feels weird to have the ability to translate for all languages. I
> mean, the original description is already written in one of those languages,
> right ? So if I add a translation for each language, the original
> description will never be used. Not really a problem though, as it allows to
> have the original description in the language we want, and we can just not
> add a translation for that language.

This development basically mimic what is done for item types. So if there is a
change to do in the way they are translated, that should be done before or
after this development to affect both screen.
I do not think it's a problem anyway. The "default" (the one in the
authorised_value table) is used when there is no translation available.

> 3) There is some places where translated descriptions are not used (the
> items table in cataloguing/additem.pl for instance) but that can be added
> later (not blocking)

I can commit on fixing any bugs we are going to find after those patches are
pushed.
I will take a look at additem.

> 4) The new column 'interface' will be only useful for authorized value,
> right ? Why not append the interface to the entity name instead of adding a
> new column ("authorised_values:opac" for instance) ? Actually, the AV
> category code could be added there too to avoid (1)
> ("authorised_values:opac:CCODE").

I think it makes sense to have the interface. The idea is to provide an
interface in the admin module to have a global view of the different entities
that are translated. So we could imagine a filter to display only opac/staff.
Also it could make sense to have the interface for some of the system
preferences.
As the feature is here, I prefer to keep it and see how it could be useful that
add work to drop it then re-add it later. Do not you agree?

> 5) Please add a test plan :) Even if it's not exhaustive it really helps to
> have one.

As I said already, a test plan is hard to provide.
One could be:
- Pick an authorised value that is displayed in lot of places (LOST/DAMAGED for
instance)
- Provide a translation for some of the values
- Go to different pages where the AV is displayed (opac and staff) and make
sure the values you translated is the one that is displayed.


> 6) Koha::AuthorisedValues::search_with_localization does something really
> strange : it sets package variables
> $Koha::Schema::Result::AuthorisedValue::LANGUAGE and
> $Koha::Schema::Result::AuthorisedValue::INTERFACE. I understand that they
> are then used in the has_many subroutine for 'localization', and that the
> same thing is done for itemtypes, but that feels wrong to use global
> variables like that. Can't we use a "normal" join in
> search_with_localization ? And why has_many ? An authorised value should
> have only 1 (or 0) translation, given a particular language and interface.

They are used actually in the view
(Koha/Schema/Result/AuthorisedValueLocalization.pm).
Take a look at this commit where I tried to explain it
  commit 80018625cf03ccd00957035f99065ed5793b80e3
  Bug 17835: Create a ItemtypeLocalization view
I spent quite a lot of time to find a correct solution when I implemented bug
17835.
Maybe there is a better way to do it, but here I am following an existing
pattern, that works so far.

I think the description also answer why 1-N (has_many): 1 entry in
authorised_value has several entries in the view.

-- 
You are receiving this mail because:
You are watching all bug changes.