Bug 20307 - Language overlay for authorized values
Summary: Language overlay for authorized values
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low new feature (vote)
Assignee: Jonathan Druart
QA Contact: Tomás Cohen Arazi
URL:
Keywords: rel_20_05_target, RM_priority
Depends on: 14100
Blocks: 11815 22610
  Show dependency treegraph
 
Reported: 2018-02-27 21:34 UTC by Jonathan Druart
Modified: 2019-10-28 09:36 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 20307: Add DB column localization.interface (1.47 KB, patch)
2019-03-14 18:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20307: DBIC Schema changes (1.61 KB, patch)
2019-03-14 18:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20307: Update the localization unique key (1.57 KB, patch)
2019-03-14 18:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20307: DBIC Schema changes 2 (1.44 KB, patch)
2019-03-14 18:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20307: Language overlay for authorised values (20.83 KB, patch)
2019-03-14 18:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20307: Some more fixes (15.63 KB, patch)
2019-03-14 18:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20307: (QA follow-up) Move DB update to perl (1.87 KB, patch)
2019-05-10 10:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20307: Add DB column localization.interface (1.53 KB, patch)
2019-07-19 12:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20307: DBIC Schema changes (1.67 KB, patch)
2019-07-19 12:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20307: Update the localization unique key (1.63 KB, patch)
2019-07-19 12:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20307: DBIC Schema changes 2 (1.50 KB, patch)
2019-07-19 12:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20307: Language overlay for authorised values (20.83 KB, patch)
2019-07-19 12:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20307: Some more fixes (15.56 KB, patch)
2019-07-19 12:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20307: (QA follow-up) Move DB update to perl (1.93 KB, patch)
2019-07-19 12:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20307: (QA follow-up) Move DB update to perl (1.93 KB, patch)
2019-07-19 12:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20307: Some more fixes (15.53 KB, patch)
2019-08-14 16:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20307: Remove debug statement Carp::Always (659 bytes, patch)
2019-08-14 16:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20307: Disable the translate links if the object has not been saved yet (4.55 KB, patch)
2019-08-19 20:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20307: Use 'auhorised_values' instead of 'avs' to avoid confusion (6.26 KB, patch)
2019-08-19 20:21 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20307: Code for AVs must be category and value (4.31 KB, patch)
2019-08-19 20:21 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20307: Add DB column localization.interface (1.53 KB, patch)
2019-10-16 07:42 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20307: DBIC Schema changes (1.67 KB, patch)
2019-10-16 07:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20307: Update the localization unique key (1.63 KB, patch)
2019-10-16 07:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20307: DBIC Schema changes 2 (1.50 KB, patch)
2019-10-16 07:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20307: Language overlay for authorised values (20.63 KB, patch)
2019-10-16 07:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20307: (QA follow-up) Move DB update to perl (1.93 KB, patch)
2019-10-16 07:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20307: Some more fixes (15.51 KB, patch)
2019-10-16 07:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20307: Remove debug statement Carp::Always (659 bytes, patch)
2019-10-16 07:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20307: Disable the translate links if the object has not been saved yet (4.55 KB, patch)
2019-10-16 07:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20307: Use 'auhorised_values' instead of 'avs' to avoid confusion (6.25 KB, patch)
2019-10-16 07:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20307: Code for AVs must be category and value (4.30 KB, patch)
2019-10-16 07:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20307: Few more fixes due to bad rebase conflict resolutions (4.36 KB, patch)
2019-10-16 07:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20307: Add DB column localization.interface (1.57 KB, patch)
2019-10-24 21:44 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 20307: DBIC Schema changes (1.72 KB, patch)
2019-10-24 21:44 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 20307: Update the localization unique key (1.68 KB, patch)
2019-10-24 21:44 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 20307: DBIC Schema changes 2 (1.55 KB, patch)
2019-10-24 21:44 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 20307: Language overlay for authorised values (20.68 KB, patch)
2019-10-24 21:44 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 20307: (QA follow-up) Move DB update to perl (1.98 KB, patch)
2019-10-24 21:44 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 20307: Some more fixes (15.55 KB, patch)
2019-10-24 21:44 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 20307: Remove debug statement Carp::Always (707 bytes, patch)
2019-10-24 21:44 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 20307: Disable the translate links if the object has not been saved yet (4.60 KB, patch)
2019-10-24 21:44 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 20307: Use 'auhorised_values' instead of 'avs' to avoid confusion (6.30 KB, patch)
2019-10-24 21:44 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 20307: Code for AVs must be category and value (4.35 KB, patch)
2019-10-24 21:44 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 20307: Few more fixes due to bad rebase conflict resolutions (4.40 KB, patch)
2019-10-24 21:45 UTC, Michal Denar
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2018-02-27 21:34:39 UTC

    
Comment 1 Katrin Fischer 2018-06-22 12:49:20 UTC
Is there any work yet planned or done on this?
Comment 2 Jonathan Druart 2018-06-22 13:09:57 UTC
(In reply to Katrin Fischer from comment #1)
> Is there any work yet planned or done on this?

I have started something, but we need bug 15799 first (move the last C4::Koha::GetAuthorisedValues to Koha::AuthorisedValues)

There are local patches (very wip) on my laptop, but I have no idea when I will get back to it. The rewrite/move patches are to slow to get pushed and need too much work to rebase.
Comment 3 Katrin Fischer 2018-07-01 18:55:18 UTC
Ok, a little confused by the graph - didn't spot any patch with patches still needing work. Can you point me the right direction?
Comment 4 Jonathan Druart 2018-07-02 13:52:00 UTC
(In reply to Katrin Fischer from comment #3)
> Ok, a little confused by the graph - didn't spot any patch with patches
> still needing work. Can you point me the right direction?

I do not plan to get back to this soon.
Comment 5 Jonathan Druart 2019-03-02 21:19:19 UTC
Should we continue on the same way of item types (translation from the UI), or extract the strings from the DB then use the new Koha::I18 module (translation from POs)?
Comment 6 Katrin Fischer 2019-03-05 06:48:35 UTC
(In reply to Jonathan Druart from comment #5)
> Should we continue on the same way of item types (translation from the UI),
> or extract the strings from the DB then use the new Koha::I18 module
> (translation from POs)?

If I am not mistaken the translation from POs would always require to run jobs on the server, maintain your own po files... put them in the right place. I think we need something that can be done in the GUI to manage the translations. I am not sure if that could be achieved differently than what you have done for the itemtypes.
Comment 7 Stefan Berndtsson 2019-03-13 13:05:07 UTC
(In reply to Jonathan Druart from comment #5)
> Should we continue on the same way of item types (translation from the UI),
> or extract the strings from the DB then use the new Koha::I18 module
> (translation from POs)?

I think going the item type route is the proper way to do it. Doing it through PO:s wouldn't really work. Libraries within the same language area (sv-SE in my case) will not use the same names for values like LOC, and most likely not have the same values at all, and you can't have locally edited PO files for each library.

Translations for item types should be fairly similar to translations for authorised values. Patron categories and library names should also benefit from such a system.
Comment 8 Jonathan Druart 2019-03-14 18:02:20 UTC
Created attachment 86622 [details] [review]
Bug 20307: Add DB column localization.interface
Comment 9 Jonathan Druart 2019-03-14 18:02:25 UTC
Created attachment 86623 [details] [review]
Bug 20307: DBIC Schema changes
Comment 10 Jonathan Druart 2019-03-14 18:02:29 UTC
Created attachment 86624 [details] [review]
Bug 20307: Update the localization unique key
Comment 11 Jonathan Druart 2019-03-14 18:02:35 UTC
Created attachment 86625 [details] [review]
Bug 20307: DBIC Schema changes 2
Comment 12 Jonathan Druart 2019-03-14 18:02:41 UTC
Created attachment 86626 [details] [review]
Bug 20307: Language overlay for authorised values
Comment 13 Jonathan Druart 2019-03-14 18:02:45 UTC
Created attachment 86627 [details] [review]
Bug 20307: Some more fixes
Comment 14 Jonathan Druart 2019-03-14 18:06:16 UTC
I have cleaned and rebased my local patches, as well as fixing obvious problems. I have no idea how big is the remaining work to make it ready to be pushed.

However it can be tested and having a list of what is ok and what is not will help (and motivate) me to finish this enhancement.
Comment 15 Martin Renvoize 2019-04-24 20:29:17 UTC
Oh, sorry, I hadn't spotted that this one had come to life.. I'll try hard to find a moment to have a look at it.
Comment 16 Jonathan Druart 2019-05-09 15:53:34 UTC
Updating the status to Needs Signoff (to get more love).
Comment 17 Martin Renvoize 2019-05-10 10:13:11 UTC
Created attachment 89538 [details] [review]
Bug 20307: (QA follow-up) Move DB update to perl
Comment 18 Martin Renvoize 2019-05-10 10:14:46 UTC
This works, and I'm not against it.. but I can't help but feel there must be a simpler approach.

I'm going to go away and work on an alternative implementation as a way of getting my head around this code as part of a SO/QA effort.

I'll be using some of this as foundation work.. so see it as a golfing effort.
Comment 19 Jonathan Druart 2019-05-10 12:54:25 UTC
Hello Martin,

I do not know how easier it could be, what do you have in mind? :)
We already have a way to translate stuffs (only item types so far), we must reuse it. In those patches I am just mimicking what we already have for item types.
Comment 20 Martin Renvoize 2019-05-10 17:47:53 UTC
Testers do please go ahead and test this however..

The work I'm doing is more as preparation for doing a QA.. I felt working through the thought process involved in thinking about an alternative would be the best way for me to get up to speed with this implementation.
Comment 21 Hayley Mapley 2019-06-12 01:43:37 UTC
Hi,

Is there a test plan for this?
Comment 22 Jonathan Druart 2019-06-20 01:34:06 UTC
It's hard to provide a complete test plan, you will need to show imagination to find bugs :)

The idea is to translate authorised values and confirm that they are displayed translated for translated templates.
Comment 23 Martin Renvoize 2019-07-19 12:00:15 UTC
Created attachment 91645 [details] [review]
Bug 20307: Add DB column localization.interface

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 24 Martin Renvoize 2019-07-19 12:00:19 UTC
Created attachment 91646 [details] [review]
Bug 20307: DBIC Schema changes

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 25 Martin Renvoize 2019-07-19 12:00:22 UTC
Created attachment 91647 [details] [review]
Bug 20307: Update the localization unique key

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 26 Martin Renvoize 2019-07-19 12:00:26 UTC
Created attachment 91648 [details] [review]
Bug 20307: DBIC Schema changes 2

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 27 Martin Renvoize 2019-07-19 12:00:29 UTC
Created attachment 91649 [details] [review]
Bug 20307: Language overlay for authorised values

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 28 Martin Renvoize 2019-07-19 12:00:33 UTC
Created attachment 91650 [details] [review]
Bug 20307: Some more fixes

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 29 Martin Renvoize 2019-07-19 12:00:36 UTC
Created attachment 91651 [details] [review]
Bug 20307: (QA follow-up) Move DB update to perl

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 30 Martin Renvoize 2019-07-19 12:09:02 UTC
Rebased on master and signing off here..

I couldn't come up with a cleaner way and don't want to stop this one in it's tracks..

I'm minded to go ahead and push it and deal with consequences over the rest of the cycle as it feels pretty solid, but it would be nice to get a QA eye on it really..
Comment 31 Martin Renvoize 2019-07-19 12:18:33 UTC
Created attachment 91653 [details] [review]
Bug 20307: (QA follow-up) Move DB update to perl

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 32 Martin Renvoize 2019-07-19 13:30:21 UTC
Needs unit tests ;)
Comment 33 Katrin Fischer 2019-08-09 08:15:52 UTC
Hi Jonathan, 

starting QA here - not only a RM priority, but one of mine as well. I will try to come back at this as long as necessary:

1) QA test tools:

There are some about POD, but I think those are valid:

 FAIL	admin/authorised_values.pl
   FAIL	  valid
		Can't locate Carp/Always.pm in @INC (you may need to install the Carp::Always module) (@INC contains: /home/vagrant/kohaclone /home/vagrant/qa-test-tools /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.24.1 /usr/local/share/perl/5.24.1 /usr/lib/x86_64-linux-gnu/perl5/5.24 /usr/share/perl5 /usr/lib/x86_64-linux-gnu/perl/5.24 /usr/share/perl/5.24 /usr/local/lib/site_perl /usr/lib/x86_64-linux-gnu/perl-base /var/lib/koha/kohadev/plugins) 

 FAIL	Koha/AuthorisedValues.pm

   FAIL	  valid
		Name "Koha::Schema::Result::AuthorisedValue::LANGUAGE" used only once: possible typo 

2) Code review

a) C4/Search.pm
+                                $facet_label_value = $av->count ? $av->next->opac_translated_description : ''; # FIXME: Why are we displaying the opac description for staff?

I believe you are right here - this is a bug. Could be fixed here or later. Why are we not dealing with this in the templates?

b) Koha/AuthorisedValue.pm

opac_translated_description and tranlated_description seem to have a lot of duplicated code. Coudl it make sense to merge them with an interface parameter?

+    my $descriptions = {
+        lib              => $av->translated_description,     # Rename with opac_translated_description to avoid confusion
+        opac_description => $av->opac_translated_description # Same with translated_description
+    };

Not sure what you meant with your comments here?

3) Testing

The admin page currently explodes:

Could not compile /home/vagrant/kohaclone/admin/authorised_values.pl: Can't locate Carp/Always.pm in @INC (you may need to install the Carp::Always module) (@INC contains: /home/vagrant/kohaclone /home/vagrant/kohaclone/installer /home/vagrant/kohaclone/lib/installer /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.24.1 /usr/local/share/perl/5.24.1 /usr/lib/x86_64-linux-gnu/perl5/5.24 /usr/share/perl5 /usr/lib/x86_64-linux-gnu/perl/5.24 /usr/share/perl/5.24 /usr/local/lib/site_perl /usr/lib/x86_64-linux-gnu/perl-base /var/lib/koha/kohadev/plugins) at /home/vagrant/kohaclone/admin/authorised_values.pl line 27.
BEGIN failed--compilation aborted at /home/vagrant/kohaclone/admin/authorised_values.pl line 27.
 at /usr/share/perl5/CGI/Compile.pm line 132


As I understand it, this is just the 'base' and then we will have to work through the different AV categories... that about right?
Comment 34 Jonathan Druart 2019-08-14 16:31:33 UTC
(In reply to Katrin Fischer from comment #33)
> Hi Jonathan, 

Hi Katrin, thanks for your feedback!

> starting QA here - not only a RM priority, but one of mine as well. I will
> try to come back at this as long as necessary:
> 
> 1) QA test tools:
> 
> There are some about POD, but I think those are valid:
> 
>  FAIL	admin/authorised_values.pl
>    FAIL	  valid
> 		Can't locate Carp/Always.pm in @INC (you may need to install the
> Carp::Always module) (@INC contains: /home/vagrant/kohaclone
> /home/vagrant/qa-test-tools /etc/perl
> /usr/local/lib/x86_64-linux-gnu/perl/5.24.1 /usr/local/share/perl/5.24.1
> /usr/lib/x86_64-linux-gnu/perl5/5.24 /usr/share/perl5
> /usr/lib/x86_64-linux-gnu/perl/5.24 /usr/share/perl/5.24
> /usr/local/lib/site_perl /usr/lib/x86_64-linux-gnu/perl-base
> /var/lib/koha/kohadev/plugins) 

All issues related with Carp::Always will be resolved by a follow-up patch.


>  FAIL	Koha/AuthorisedValues.pm
> 
>    FAIL	  valid
> 		Name "Koha::Schema::Result::AuthorisedValue::LANGUAGE" used only once:
> possible typo 

Not valid, it's what we are doing for Itemtype.pm

> 2) Code review
> 
> a) C4/Search.pm
> +                                $facet_label_value = $av->count ?
> $av->next->opac_translated_description : ''; # FIXME: Why are we displaying
> the opac description for staff?
> 
> I believe you are right here - this is a bug. Could be fixed here or later.
> Why are we not dealing with this in the templates?

Because we are in C4::Search :-/

> b) Koha/AuthorisedValue.pm
> 
> opac_translated_description and tranlated_description seem to have a lot of
> duplicated code. Coudl it make sense to merge them with an interface
> parameter?

IMO it's more readable as it, but could be discussed.


> +    my $descriptions = {
> +        lib              => $av->translated_description,     # Rename with
> opac_translated_description to avoid confusion
> +        opac_description => $av->opac_translated_description # Same with
> translated_description
> +    };
> 
> Not sure what you meant with your comments here?

They do not make sense, I guess it's a c/p failure from Koha/Template/Plugin/AuthorisedValues.pm

> 3) Testing
> 
> The admin page currently explodes:
> 
> Could not compile /home/vagrant/kohaclone/admin/authorised_values.pl: Can't
> locate Carp/Always.pm in @INC (you may need to install the Carp::Always
> module) (@INC contains: /home/vagrant/kohaclone
> /home/vagrant/kohaclone/installer /home/vagrant/kohaclone/lib/installer
> /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.24.1
> /usr/local/share/perl/5.24.1 /usr/lib/x86_64-linux-gnu/perl5/5.24
> /usr/share/perl5 /usr/lib/x86_64-linux-gnu/perl/5.24 /usr/share/perl/5.24
> /usr/local/lib/site_perl /usr/lib/x86_64-linux-gnu/perl-base
> /var/lib/koha/kohadev/plugins) at
> /home/vagrant/kohaclone/admin/authorised_values.pl line 27.
> BEGIN failed--compilation aborted at
> /home/vagrant/kohaclone/admin/authorised_values.pl line 27.
>  at /usr/share/perl5/CGI/Compile.pm line 132

Because of missing Carp::Always, but I removed it now.

> As I understand it, this is just the 'base' and then we will have to work
> through the different AV categories... that about right?

Nope, it should work for any AV.
Comment 35 Jonathan Druart 2019-08-14 16:32:11 UTC
Created attachment 92192 [details] [review]
Bug 20307: Some more fixes

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 36 Jonathan Druart 2019-08-14 16:32:15 UTC
Created attachment 92193 [details] [review]
Bug 20307: Remove debug statement Carp::Always
Comment 37 Katrin Fischer 2019-08-16 05:54:16 UTC
Back here... :)
Comment 38 Katrin Fischer 2019-08-16 06:03:53 UTC
Sorry Jonathan, not getting far again: 

When I click on a category in authorised values to edit existing values:
Template process failed: undef error - The method Koha::AuthorisedValue->translated_descriptions is not covered by tests!
Comment 39 Katrin Fischer 2019-08-16 06:08:42 UTC
Sorry, my fault... forgot a restart_all.
Comment 40 Katrin Fischer 2019-08-16 06:26:17 UTC
I have problems with adding end editing of translations:

1) Adding translations the same time as adding a new authorised value doesn't work. The first time you try, it seems to save, but doesn't display the values later. And the second time you try, it claims values already exist. 
I think the easiest way for now would be to not display the translate links when adding, only on editing.

2) The entity shown in the table of translations is "avs" - not sure if this is a code or should be translatable? It might not be understood well in other context.

3) After adding some values and translations to TERM I have switched to DEPARTMENT. Adding an OPAC description translation worked, but now I get an error when I try to add a staff translation:

DBD::mysql::st execute failed: Duplicate entry 'avs-Department1-de-DE' for key 'entity_code_lang' [for Statement "INSERT INTO `localization` ( `code`, `entity`, `interface`, `lang`, `translation`) VALUES ( ?, ?, ?, ?, ? )" with ParamValues: 0="Department1", 1="avs", 2="intranet", 3="de-DE", 4="depds"] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1836.
Duplicate ID at /usr/share/perl5/Exception/Class/Base.pm line 88
	Exception::Class::Base::throw('Koha::Exceptions::Object::DuplicateID', 'error', 'Duplicate ID', 'duplicate_id', 'entity_code_lang') called at /home/vagrant/kohaclone/Koha/Object.pm line 160

I am not yet sure why this happens, my table looks like this:

+-----------------+-----------+-----------+-------------+-------+-------------+
| localization_id | entity    | interface | code        | lang  | translation |
+-----------------+-----------+-----------+-------------+-------+-------------+
|               1 | avs       | opac      |             | de-DE | 2019d       |
|               2 | avs       | opac      |             | en    | test        |
|               3 | avs       | intranet  | 2019        | de-DE | 2019d       |
|               5 | avs       | intranet  | 2019        | en    | t           |
|               6 | itemtypes | NULL      | BK          | de-DE | Bücher      |
|               8 | avs       | opac      | Department1 | de-DE | dep_do      |
+-----------------+-----------+-----------+-------------+-------+-------------+

4) Looking at the db structure: We know the code and that it is an authorised value, but we don't know the category! I think this is a mistake. Not always the tanslation for the same code is the same. Think about the numeric ones like Damaged, Lost...
Comment 41 Jonathan Druart 2019-08-19 20:20:58 UTC
Created attachment 92359 [details] [review]
Bug 20307: Disable the translate links if the object has not been saved yet

We do not have the code yet (or can be edited after the translation has
been done)
Comment 42 Jonathan Druart 2019-08-19 20:21:02 UTC
Created attachment 92360 [details] [review]
Bug 20307: Use 'auhorised_values' instead of 'avs' to avoid confusion

And so we need to extend the size of localization.entity
Comment 43 Jonathan Druart 2019-08-19 20:21:09 UTC
Created attachment 92361 [details] [review]
Bug 20307: Code for AVs must be category and value

And not only the authorized value, otherwise we could not handle the
case where 2 categories have an identical authorized value.

FIXME: When editing an AV, if you translate then change the AV's value,
we will have inconsistent data, we should add some JS code to disable
the translate link if the AV's value is modified. I would suggest to do
it on a follow-up bug.
Comment 44 Jonathan Druart 2019-08-19 20:22:51 UTC
(In reply to Katrin Fischer from comment #40)
> I have problems with adding end editing of translations:
> 
> 1) Adding translations the same time as adding a new authorised value
> doesn't work. The first time you try, it seems to save, but doesn't display
> the values later. And the second time you try, it claims values already
> exist. 
> I think the easiest way for now would be to not display the translate links
> when adding, only on editing.

Indeed, I removed the links on adding a new AV or itemtype.

> 2) The entity shown in the table of translations is "avs" - not sure if this
> is a code or should be translatable? It might not be understood well in
> other context.

It's an internal code, I have renamed it anyway.

> 3) After adding some values and translations to TERM I have switched to
> DEPARTMENT. Adding an OPAC description translation worked, but now I get an
> error when I try to add a staff translation:
> 
> DBD::mysql::st execute failed: Duplicate entry 'avs-Department1-de-DE' for
> key 'entity_code_lang' [for Statement "INSERT INTO `localization` ( `code`,
> `entity`, `interface`, `lang`, `translation`) VALUES ( ?, ?, ?, ?, ? )" with
> ParamValues: 0="Department1", 1="avs", 2="intranet", 3="de-DE", 4="depds"]
> at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1836.
> Duplicate ID at /usr/share/perl5/Exception/Class/Base.pm line 88
> 	Exception::Class::Base::throw('Koha::Exceptions::Object::DuplicateID',
> 'error', 'Duplicate ID', 'duplicate_id', 'entity_code_lang') called at
> /home/vagrant/kohaclone/Koha/Object.pm line 160
> 
> I am not yet sure why this happens, my table looks like this:
> 
> +-----------------+-----------+-----------+-------------+-------+------------
> -+
> | localization_id | entity    | interface | code        | lang  |
> translation |
> +-----------------+-----------+-----------+-------------+-------+------------
> -+
> |               1 | avs       | opac      |             | de-DE | 2019d     
> |
> |               2 | avs       | opac      |             | en    | test      
> |
> |               3 | avs       | intranet  | 2019        | de-DE | 2019d     
> |
> |               5 | avs       | intranet  | 2019        | en    | t         
> |
> |               6 | itemtypes | NULL      | BK          | de-DE | Bücher    
> |
> |               8 | avs       | opac      | Department1 | de-DE | dep_do    
> |
> +-----------------+-----------+-----------+-------------+-------+------------
> -+
> 
> 4) Looking at the db structure: We know the code and that it is an
> authorised value, but we don't know the category! I think this is a mistake.
> Not always the tanslation for the same code is the same. Think about the
> numeric ones like Damaged, Lost...

Indeed, big design issue. Thanks Katrin!
It should be fixed now.
Comment 45 Katrin Fischer 2019-08-20 21:10:17 UTC
I started with testing the DEPARTMENT again.

I added a de-DE translation for the DEPARTMENT. None for en.

1) The language de-DE is not activated, but it starts to show the translation I added for German in the English interface. dep-dei - internal staff description in the table below, shows in the English OPAC GUI. Staff/OPAC might be a template issue, but still wondering why it shows up in the English GUI?

+-----------------+-------------------+-----------+------------------------+-------+-------------+
| localization_id | entity            | interface | code                   | lang  | translation |
+-----------------+-------------------+-----------+------------------------+-------+-------------+
|               1 | itemtypes         | NULL      | BK                     | de-DE | Bücher      |
|               2 | authorised_values | intranet  | DEPARTMENT_Department1 | de-DE | Dep-dei     |
|               5 | authorised_values | opac      | DEPARTMENT_Department1 | de-DE | dep-deo     |
+-----------------+-------------------+-----------+------------------------+-------+-------------+

2) In staff admin:
My translation for OPAC description shows as description in the OPAC summary (the OPAC description column being empty). When I edit the OPAC description, the translation shows there. So maybe just something wrong on the list of values?
Comment 46 Jonathan Druart 2019-09-01 14:57:21 UTC
(In reply to Katrin Fischer from comment #45)
> I started with testing the DEPARTMENT again.
> 
> I added a de-DE translation for the DEPARTMENT. None for en.
> 
> 1) The language de-DE is not activated, but it starts to show the
> translation I added for German in the English interface. dep-dei - internal
> staff description in the table below, shows in the English OPAC GUI.
> Staff/OPAC might be a template issue, but still wondering why it shows up in
> the English GUI?

Where? When editing/adding a course reserve I see the default (untranslated) value.

> 2) In staff admin:
> My translation for OPAC description shows as description in the OPAC summary
> (the OPAC description column being empty). When I edit the OPAC description,
> the translation shows there. So maybe just something wrong on the list of
> values?

I do not get that one, sorry. Could you provide a screenshot?
Comment 47 Jonathan Druart 2019-10-16 07:42:56 UTC
Created attachment 94245 [details] [review]
Bug 20307: Add DB column localization.interface

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 48 Jonathan Druart 2019-10-16 07:43:00 UTC
Created attachment 94246 [details] [review]
Bug 20307: DBIC Schema changes

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 49 Jonathan Druart 2019-10-16 07:43:04 UTC
Created attachment 94247 [details] [review]
Bug 20307: Update the localization unique key

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 50 Jonathan Druart 2019-10-16 07:43:08 UTC
Created attachment 94248 [details] [review]
Bug 20307: DBIC Schema changes 2

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 51 Jonathan Druart 2019-10-16 07:43:13 UTC
Created attachment 94249 [details] [review]
Bug 20307: Language overlay for authorised values

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 52 Jonathan Druart 2019-10-16 07:43:17 UTC
Created attachment 94250 [details] [review]
Bug 20307: (QA follow-up) Move DB update to perl

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 53 Jonathan Druart 2019-10-16 07:43:21 UTC
Created attachment 94251 [details] [review]
Bug 20307: Some more fixes

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 54 Jonathan Druart 2019-10-16 07:43:25 UTC
Created attachment 94252 [details] [review]
Bug 20307: Remove debug statement Carp::Always
Comment 55 Jonathan Druart 2019-10-16 07:43:29 UTC
Created attachment 94253 [details] [review]
Bug 20307: Disable the translate links if the object has not been saved yet

We do not have the code yet (or can be edited after the translation has
been done)
Comment 56 Jonathan Druart 2019-10-16 07:43:33 UTC
Created attachment 94254 [details] [review]
Bug 20307: Use 'auhorised_values' instead of 'avs' to avoid confusion

And so we need to extend the size of localization.entity
Comment 57 Jonathan Druart 2019-10-16 07:43:37 UTC
Created attachment 94255 [details] [review]
Bug 20307: Code for AVs must be category and value

And not only the authorized value, otherwise we could not handle the
case where 2 categories have an identical authorized value.

FIXME: When editing an AV, if you translate then change the AV's value,
we will have inconsistent data, we should add some JS code to disable
the translate link if the AV's value is modified. I would suggest to do
it on a follow-up bug.
Comment 58 Jonathan Druart 2019-10-16 07:43:41 UTC
Created attachment 94256 [details] [review]
Bug 20307: Few more fixes due to bad rebase conflict resolutions
Comment 59 Michal Denar 2019-10-24 21:44:06 UTC
Created attachment 94700 [details] [review]
Bug 20307: Add DB column localization.interface

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 60 Michal Denar 2019-10-24 21:44:11 UTC
Created attachment 94701 [details] [review]
Bug 20307: DBIC Schema changes

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 61 Michal Denar 2019-10-24 21:44:15 UTC
Created attachment 94702 [details] [review]
Bug 20307: Update the localization unique key

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 62 Michal Denar 2019-10-24 21:44:20 UTC
Created attachment 94703 [details] [review]
Bug 20307: DBIC Schema changes 2

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 63 Michal Denar 2019-10-24 21:44:25 UTC
Created attachment 94704 [details] [review]
Bug 20307: Language overlay for authorised values

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 64 Michal Denar 2019-10-24 21:44:30 UTC
Created attachment 94705 [details] [review]
Bug 20307: (QA follow-up) Move DB update to perl

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 65 Michal Denar 2019-10-24 21:44:35 UTC
Created attachment 94706 [details] [review]
Bug 20307: Some more fixes

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 66 Michal Denar 2019-10-24 21:44:41 UTC
Created attachment 94707 [details] [review]
Bug 20307: Remove debug statement Carp::Always

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 67 Michal Denar 2019-10-24 21:44:45 UTC
Created attachment 94708 [details] [review]
Bug 20307: Disable the translate links if the object has not been saved yet

We do not have the code yet (or can be edited after the translation has
been done)

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 68 Michal Denar 2019-10-24 21:44:51 UTC
Created attachment 94709 [details] [review]
Bug 20307: Use 'auhorised_values' instead of 'avs' to avoid confusion

And so we need to extend the size of localization.entity

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 69 Michal Denar 2019-10-24 21:44:56 UTC
Created attachment 94710 [details] [review]
Bug 20307: Code for AVs must be category and value

And not only the authorized value, otherwise we could not handle the
case where 2 categories have an identical authorized value.

FIXME: When editing an AV, if you translate then change the AV's value,
we will have inconsistent data, we should add some JS code to disable
the translate link if the AV's value is modified. I would suggest to do
it on a follow-up bug.

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 70 Michal Denar 2019-10-24 21:45:01 UTC
Created attachment 94711 [details] [review]
Bug 20307: Few more fixes due to bad rebase conflict resolutions

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 71 Jonathan Druart 2019-10-25 09:00:21 UTC
QA: This is not a priority for 19.11, it's too late. But please keep it in mind for early 19.12 ;)
Comment 72 Katrin Fischer 2019-10-28 06:55:05 UTC
(In reply to Jonathan Druart from comment #71)
> QA: This is not a priority for 19.11, it's too late. But please keep it in
> mind for early 19.12 ;)

+1!
Comment 73 Martin Renvoize 2019-10-28 09:36:33 UTC
I've added a new keyword for things we should target early next cycle.. rel_20_05_target and christened it by adding it to this bug :)