[Koha-bugs] [Bug 24975] Refactor database translations

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

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

--- Comment #113 from Jonathan Druart <jonathan.druart+koha at gmail.com> ---
(In reply to Marcel de Rooy from comment #108)
> Some findings in random order:
> 
> I am missing an Add button on admin/localization. You can clear a record and
> remove it; but perhaps adding a Delete button would be nice too.

What would you like to add? You are not supposed to need it, at least for now.

> Not sure why we are using DBIC here instead of Koha objects. No performance
> figures are given to justify. Koha::Object should be our standard.

It avoids 2 new packages, Koha::I18N::L10nSource[s]?
I would go without them to start with this approach, please keep things as
simple as possible.

> opac/opac-search.pl
>      my $description = $itemtypes->{$itemtype}->{iscat}
>        ? $itemtypes->{$itemtype}->{description}
> -      : db_t('itemtype', Koha::ItemTypes->find($itemtype)->itemtype);
> +      : Koha::ItemTypes->find($itemtype)->translated_description;
>      $itemtypes->{$itemtype}->{description} = $description ||
> $itemtypes->{$itemtype}->{description} || q{};
> Weird code.

All this "itemtype category" code is ugly, but why is this part weird? (there
is a comment above about non-translatability of item type categories)

> package Koha::Template::Plugin::ItemTypes;
> sub GetDescription {
>     return $parent ? $self->t($parent->description) . "->" .
> $self->t($itemtype->description) : $self->t($itemtype->description);
> Omission for switching to itemtype.

Should be fixed now.

(In reply to Marcel de Rooy from comment #109)
> There is still something confusing in the table design:
> select * from l10n_source;
> +----------------+----------+-----+------------------------+
> | l10n_source_id | group    | key | text                   |
> +----------------+----------+-----+------------------------+
> |              1 | itemtype | BK  | Books                  |
> |              2 | itemtype | BKS | Books                  |
> |              3 | itemtype | CF  | Computer Files         |
> select * from l10n_target;
> +----------------+----------------+----------+-----------------+-------+
> | l10n_target_id | l10n_source_id | language | translation     | fuzzy |
> +----------------+----------------+----------+-----------------+-------+
> |              2 |              1 | nl-NL    | Boek            |     0 |
> |              3 |              2 | en       | BKS_EN          |     0 |
> |              4 |              2 | nl-NL    | Boek            |     0 |
> The source seems to be the English text. But I can translate it into EN
> again via target. Easiest seems to be: prevent having EN in the target table.

The source (l10n_source.text) also contain the default in case there is no
translation. We should keep it.

> Shouldnt we check if we are calling db_t btw for an English translation? We
> should not be looking for the English translation of itemtype if we have the
> description already in our object.

If we are not doing worst with this patch than before, I would not recommend to
add more to it. First step should be to have something pushed that is stable.
Then we can iterate and improve.

(In reply to Marcel de Rooy from comment #110)
> db_t could be made more flexible? Now we look for key and we removed
> description. But I think we should support both.

No, that's the whole point of the discussion earlier. We should go with the
key.
At least not for now.

> Not sure how the concept of localization group will grow. But it might be
> too big soon for loading all its sources and finding corresponding
> translations. (The code needs a join here.) For instance, thinking about AV,
> I am probably only interested in a particular category and not in all
> entries under 'authorised_values' if that was the group. Other areas may
> have other needs again.
> This is a quite fundamental issue on the design of db_t. It feels not
> complete.

Well, it's cached, I would not care much about performances here, that should
not be a problem if we are doing things correctly.

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