kitodo / kitodo-presentation

Kitodo.Presentation is a feature-rich framework for building a METS- or IIIF-based digital library. It is part of the Kitodo Digital Library Suite.
https://kitodo.github.io/kitodo-presentation/
GNU General Public License v3.0
39 stars 45 forks source link

[BUG] Metadata and structure translation entries #1147

Closed csidirop closed 8 months ago

csidirop commented 9 months ago

Description

There is something off with the metadata created by the new tenant controller. I first stumbled across this, when I tried to edit a metadata entry and TYPO3 gave me an info message that : The value of the field "index_name" has been changed from "opac_id" to "opac_id0" as it is required to be unique. And this holds for every metadata entry.

So it seams, that in between the v3.3.x and now something broke. Here in comparison some screenshots:

Overview: (note the indentations indicating a translation of the entry instead of a separate one) overview

Example: title

Example translation: (apart from the obvious difference, you can see that the translation in v4.x has the same index_name as the original) title-translation

This are exemplary DB entries for Type/Strukturtyp:

# v3.3.x:
INSERT INTO `tx_dlf_metadata` VALUES (28,3,1706263174,1706263174,2,0,0,0,'a:16:{s:5:\"label\";N;s:10:\"index_name\";N;s:15:\"index_tokenized\";N;s:12:\"index_stored\";N;s:13:\"index_indexed\";N;s:11:\"index_boost\";N;s:11:\"is_sortable\";N;s:8:\"is_facet\";N;s:9:\"is_listed\";N;s:18:\"index_autocomplete\";N;s:6:\"format\";N;s:13:\"default_value\";N;s:4:\"wrap\";N;s:16:\"sys_language_uid\";N;s:6:\"hidden\";N;s:6:\"status\";N;}',1,1026,NULL,'Strukturtyp','type',0,'','key.wrap = <dt class=\"tx-dlf-type\">|</dt>\nvalue.required = 1\nvalue.wrap = <dd class=\"tx-dlf-type\">|</dd>',0,1,1,1,0,0,1,0,0);
INSERT INTO `tx_dlf_metadata` VALUES (29,3,1706263174,1706263174,2,0,1,30,'a:3:{s:16:\"sys_language_uid\";N;s:11:\"l18n_parent\";N;s:6:\"format\";N;}',0,1025,NULL,'Context','context0',1,'','key.wrap = <dt>|</dt><dd>\nvalue.required = 1\nvalue.noTrimWrap = || &gt; |\nall.substring = 0,-6\nall.wrap = |</dd>',0,1,1,1,0,0,1,0,0);
# v4.x:
INSERT INTO `tx_dlf_metadata` VALUES (1,3,1706264096,1706264096,0,0,1,'',0,0,2,'Type','type',0,'','key.wrap = <dt>|</dt>\nvalue.required = 1\nvalue.wrap = <dd>|</dd>',0,0,0,1,0,0,0,0,0);
INSERT INTO `tx_dlf_metadata` VALUES (2,3,1706264096,1706264096,0,0,0,'',0,0,0,'Strukturtyp','type',2,'','key.wrap = <dt>|</dt>\nvalue.required = 1\nvalue.wrap = <dd>|</dd>',0,1,0,1,1,1,1,0,0);

And here an overview where you can clearly see the difference: collage What you also can see is, that the Data Formats differ extremely! On some cases there are multiple MODS data format entries and on most there different entries for the translations.

stweil commented 9 months ago

Please add the milestone Kitodo.Presentation 5.0.0.

beatrycze-volk commented 9 months ago

First thanks for description of the error!

I have suspicion what can be the cause for this error.

This are exemplary DB entries for Type/Strukturtyp:

# v3.3.x:
INSERT INTO `tx_dlf_metadata` VALUES (28,3,1706263174,1706263174,2,0,0,0,'a:16:{s:5:\"label\";N;s:10:\"index_name\";N;s:15:\"index_tokenized\";N;s:12:\"index_stored\";N;s:13:\"index_indexed\";N;s:11:\"index_boost\";N;s:11:\"is_sortable\";N;s:8:\"is_facet\";N;s:9:\"is_listed\";N;s:18:\"index_autocomplete\";N;s:6:\"format\";N;s:13:\"default_value\";N;s:4:\"wrap\";N;s:16:\"sys_language_uid\";N;s:6:\"hidden\";N;s:6:\"status\";N;}',1,1026,NULL,'Strukturtyp','type',0,'','key.wrap = <dt class=\"tx-dlf-type\">|</dt>\nvalue.required = 1\nvalue.wrap = <dd class=\"tx-dlf-type\">|</dd>',0,1,1,1,0,0,1,0,0);
INSERT INTO `tx_dlf_metadata` VALUES (29,3,1706263174,1706263174,2,0,1,30,'a:3:{s:16:\"sys_language_uid\";N;s:11:\"l18n_parent\";N;s:6:\"format\";N;}',0,1025,NULL,'Context','context0',1,'','key.wrap = <dt>|</dt><dd>\nvalue.required = 1\nvalue.noTrimWrap = || &gt; |\nall.substring = 0,-6\nall.wrap = |</dd>',0,1,1,1,0,0,1,0,0);

It would be very helpful if you could you give me correct log for insert to database. It looks here as there are copied lines for two different metadata - type and context. Already basing on it I assume that currently Kitodo.Presentation inserts double the same label instead label with suffix 0 (in your example context0 looks like translation for context, so basing on it type should get translation with type0) for translated records.

I will make necessary change and report back here :)

beatrycze-volk commented 9 months ago

https://github.com/kitodo/kitodo-presentation/pull/1169

csidirop commented 9 months ago

Here are some database dumps for you (renamed to .txt to allow upload).

-> "3.x" is from presentation 3.3.4, while "4.x" from a recent version. -> "Trimmed" contain only the tables tx_dlf_metadata, tx_dlf_metadataformat and tx_dlf_structures.

I have a setup with DFG-Viewer if that makes any difference.

Here two examples with year and place: presentation 3.3.4:

#tx_dlf_metadata:
INSERT INTO `tx_dlf_metadata` VALUES (1,3,1707475705,1707475705,2,0,1,17,'a:2:{s:16:\"sys_language_uid\";N;s:11:\"l18n_parent\";N;}',0,2304,'{\"wrap\":\"parent\"}','Year of Publication','year0',0,'','key.wrap = <dt>|</dt>\nvalue.required = 1\nvalue.wrap = <dd>|</dd>',0,0,1,1,0,0,0,0,0);
INSERT INTO `tx_dlf_metadata` VALUES (2,3,1707475705,1707475705,2,0,1,18,'a:3:{s:16:\"sys_language_uid\";N;s:11:\"l18n_parent\";N;s:6:\"format\";N;}',0,2176,NULL,'Place of Publication','place0',1,'','key.wrap = <dt>|</dt>\nvalue.ifEmpty.field = parentPlace\nvalue.required = 1\nvalue.wrap = <dd>|</dd>',0,1,1,1,0,0,1,0,0);
[...]
INSERT INTO `tx_dlf_metadata` VALUES (17,3,1707475705,1707475705,2,0,0,0,'a:1:{s:6:\"hidden\";N;}',0,1540,NULL,'Erscheinungsjahr','year',0,'','key.wrap = <dt>|</dt>\nvalue.required = 1\nvalue.wrap = <dd>|</dd>',0,0,1,1,0,0,0,0,0);
INSERT INTO `tx_dlf_metadata` VALUES (18,3,1707475705,1707475705,2,0,0,0,'a:1:{s:6:\"format\";N;}',0,1538,NULL,'Erscheinungsort','place',1,'','key.wrap = <dt>|</dt>\nvalue.ifEmpty.field = parentPlace\nvalue.required = 1\nvalue.wrap = <dd>|</dd>',0,1,1,1,0,0,1,0,0);

#tx_dlf_structures: 'place' dont exists
INSERT INTO `tx_dlf_structures` VALUES (2,3,1707475705,1707475705,2,0,1,91,'a:2:{s:16:\"sys_language_uid\";N;s:11:\"l18n_parent\";N;}',0,NULL,1,'Year of Publication','year0','',0,0);
[...]
INSERT INTO `tx_dlf_structures` VALUES (91,3,1707475705,1707475705,2,0,0,0,'a:8:{s:8:\"toplevel\";N;s:5:\"label\";N;s:10:\"index_name\";N;s:8:\"oai_name\";N;s:9:\"thumbnail\";N;s:16:\"sys_language_uid\";N;s:6:\"hidden\";N;s:6:\"status\";N;}',0,NULL,1,'Jahr','year','',0,0);

presentation 4.1:

#tx_dlf_metadata:
INSERT INTO `tx_dlf_metadata` VALUES (29,3,1707475374,1707475374,0,0,1,'',0,0,30,'Place of Publication','place',0,'','key.wrap = <dt>|</dt>\nvalue.required = 1\nvalue.wrap = <dd>|</dd>',0,0,0,1,0,0,0,0,0);
INSERT INTO `tx_dlf_metadata` VALUES (30,3,1707475374,1707475374,0,0,0,'',0,0,0,'Erscheinungsort','place',3,'','key.wrap = <dt>|</dt>\nvalue.required = 1\nvalue.wrap = <dd>|</dd>',1,1,1,1,1,1,1,0,0);
[...]
INSERT INTO `tx_dlf_metadata` VALUES (31,3,1707475374,1707475374,0,0,1,'',0,0,32,'Year of Publication','year',0,'','key.wrap = <dt>|</dt>\nvalue.required = 1\nvalue.wrap = <dd>|</dd>',0,0,0,1,0,0,0,0,0);
INSERT INTO `tx_dlf_metadata` VALUES (32,3,1707475374,1707475374,0,0,0,'',0,0,0,'Erscheinungsjahr','year',4,'','key.wrap = <dt>|</dt>\nvalue.required = 1\nvalue.wrap = <dd>|</dd>',0,1,1,1,1,1,1,0,0);

#tx_dlf_structures: ('place' dont exists)
INSERT INTO `tx_dlf_structures` VALUES (183,3,1707475361,1707475361,0,0,0,'',0,0,1,'Jahr','year','',0,0);
INSERT INTO `tx_dlf_structures` VALUES (184,3,1707475361,1707475361,0,0,1,'',0,183,0,'Year','year','',0,0);
beatrycze-volk commented 9 months ago

Thanks for detailed description :)

So my changes are removing necessity of renaming the index_name field but there is still missing this part 'a:8:{s:8:\"toplevel\";N;s:5:\"label\";N;s:10:\"index_name\";N;s:8:\"oai_name\";N;s:9:\"thumbnail\";N;s:16:\"sys_language_uid\";N;s:6:\"hidden\";N;s:6:\"status\";N;}' . I need to figure out how this one was generated previously and why it has gone missing.

csidirop commented 9 months ago

Let me know if I can help in any way

csidirop commented 9 months ago

Not sure if this is related, but when changing the language the metadata still stays German: viewer-metadata Unfortunately @BFallert can't reproduce this error. Can you?

Ps.: I updated the original post, with additional informations.

BFallert commented 9 months ago

I get a different error: In my case, all metadata that I have edited or put in a different order / sequence disappears in the english version. When saving in typo3, the IndexName is of course also supplemented by "0" if it already exists for another language, as an IndexName may only occur once.

beatrycze-volk commented 8 months ago

Not sure if this is related, but when changing the language the metadata still stays German

It looks that the records which should be used as translation are not treated as translation, more like the separated records. So the both problems are very likely related to each other. Actually the reason must lie in this empty l18n_diffsource field.

chrode commented 8 months ago

The TCA is missing a parameter that handles the connection in the TYPO3 backend.

Please add

'transOrigPointerField' => 'l18n_parent',

to https://github.com/kitodo/kitodo-presentation/blob/master/Configuration/TCA/tx_dlf_metadata.php#L21 and the backend will show the connection as before.

This should be done inside any TCA field configuration file that wants this behaviour.

The "localize to" button is only showing when you have a translation of this page.

csidirop commented 8 months ago

Worked for me: grafik

What still bothers me is the divergence in data format between the original and translation.

csidirop commented 8 months ago

I added the missing in tx_dlf_metadata.php and tx_dlf_structures, and that worked.

But maybe we need to add this also to tx_dlf_basket.php, tx_dlf_collections.php and tx_dlf_libraries.php. At least those had the languageField and transOrigDiffSourceField field too.

beatrycze-volk commented 8 months ago

The TCA is missing a parameter that handles the connection in the TYPO3 backend.

Thanks a lot for help! :)

Now the question is why was it removed.

@chrizzor could you explain what was a purpose of the change which you have made here https://github.com/kitodo/kitodo-presentation/commit/6a1ae83fca08bd970d74dc6731411035c7b32d24 ?

What still bothers me is the divergence in data format between the original and translation.

It is the next problem which I'm going to work on.

beatrycze-volk commented 8 months ago

Actually the main reason why the format is not synchronized is already mentioned above empty l18n_diffsource. It is caused by usage of Extbase in NewTenantController. Extbase directly writes to the database, and thus bypasses DataHandler. DataHandler is actually the one which is responsible for the localization of records.

It is not possible to assign the same format to both metadata records in the controller as the format record is first created on the save, so when I add something like: $translatedRecord->setFormat($newRecord->getFormat()); Extbase create new record which is attached to translated metadata and ignores original record. So in the current implementation new format record can be attached to original record or translated but not both.

Synchronization happens first after updating record in backend - l18n_diffsource is filled again. I have re-saved simply translation for Strukturtyp and then DataHandler has done its job:

INSERT INTO `tx_dlf_metadata`
VALUES (95,2,1708696735,1708696577,0,0,1,96,'a:17:{s:5:\"label\";s:11:\"Strukturtyp\";s:13:\"default_value\";s:0:\"\";s:16:\"sys_language_uid\";i:0;s:11:\"l18n_parent\";i:0;s:6:\"hidden\";i:0;s:4:\"wrap\";s:64:\"key.wrap = <dt>|</dt>\nvalue.required = 1\nvalue.wrap = <dd>|</dd>\";s:10:\"index_name\";s:4:\"type\";s:6:\"format\";i:1;s:15:\"index_tokenized\";i:0;s:12:\"index_stored\";i:1;s:13:\"index_indexed\";i:0;s:11:\"index_boost\";d:1;s:11:\"is_sortable\";i:1;s:8:\"is_facet\";i:1;s:9:\"is_listed\";i:1;s:18:\"index_autocomplete\";i:0;s:6:\"status\";i:0;}',0,0,'{\"wrap\":\"parent\"}','Typee','type',1,'','key.wrap = <dt>|</dt>\nvalue.required = 1\nvalue.wrap = <dd>|</dd>',0,1,0,1,1,1,1,0,0),
       (96,2,1708696577,1708696577,0,0,0,0,'',0,0,NULL,'Strukturtyp','type',1,'','key.wrap = <dt>|</dt>\nvalue.required = 1\nvalue.wrap = <dd>|</dd>',0,1,0,1,1,1,1,0,0),
       (97,2,1708696577,1708696577,0,0,1,98,'',0,0,NULL,'Title','title',0,'','key.wrap = <dt class=\"tx-dlf-metadata-title\">|</dt>\nvalue.required = 1\nvalue.wrap = <dd class=\"tx-dlf-metadata-title\">|</dd>',0,0,0,1,0,0,0,0,0),
       (98,2,1708696577,1708696577,0,0,0,0,'',0,0,NULL,'Titel','title',3,'','key.wrap = <dt class=\"tx-dlf-metadata-title\">|</dt>\nvalue.required = 1\nvalue.wrap = <dd class=\"tx-dlf-metadata-title\">|</dd>',1,1,1,2,1,0,1,1,0)
datahandler

Currently there is no way to force DataHandler to accept Extbase entities. We would need to transform those entities into data maps (so basically revert changes to the old state before update).

@sebastian-meyer should we revert to old saving method of data maps or accept it as bug and wait until TYPO3 provide someday possibility to localize Extbase entities with DataHandler?