Problem/Motivation

We have worked through improving the consistency and cleanness of the APIs to handle translations and separated local and target languages more. A translator plugin now does not need to care about mappings.

However, the sources always just receive a job item to saveTranslation.
Based on this, all sources need to correctly lookup the job from the item and then identify the language by $this->getJob()->getTargetLanguage().
This similarly contains risk of doing it wrong.

Proposed resolution

Additionally pass the target language code (and the data) into the saveTranslation() as arguments.
That reduces the source plugin code by at least 2 lines and more explicitly tells the source plugin what to do.

API changes

Two new arguments on saveTranslation

CommentFileSizeAuthor
#6 interdiff-pass_target_language-2569643-2-6.txt6.03 KBedurenye
#6 pass_target_language-2569643-6.patch5.62 KBedurenye
#2 pass_target_language-2569643-2.patch6.11 KBedurenye

Comments

miro_dietiker created an issue. See original summary.

edurenye’s picture

edurenye commented
StatusFileSize
new pass_target_language-2569643-2.patch6.11 KB

Done.

edurenye’s picture

edurenye commented
Status: Active » Needs review
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Needs work 
  1. +++ b/sources/content/src/Plugin/tmgmt/Source/ContentEntitySource.php
@@ -177,10 +177,8 @@ class ContentEntitySource extends SourcePluginBase {
-    $data = $job_item->getData();
-    $this->doSaveTranslations($entity, $data, $job->getTargetLangcode());
+    $this->doSaveTranslations($entity, $data, $tarjet_langcode);

    type everywhere (tarjet).

    Wondering how a tar jet would look like now ;) 

  2. +++ b/src/SourcePluginInterface.php
@@ -32,11 +32,15 @@ interface SourcePluginInterface extends PluginInspectionInterface {
+   *   The tarjet language code.

    Well, at least you are consistent ;) 

  3. +++ b/src/SourcePluginInterface.php
@@ -32,11 +32,15 @@ interface SourcePluginInterface extends PluginInspectionInterface {
    *
-   * @return bool
+   * @return bool TRUE if the translation was saved successfully, FALSE otherwise.
    *   TRUE if the translation was saved successfully, FALSE otherwise.

    That's not correctly structured. And duplicated.

Also, not 100% convinced about this. I can kind of see the point on target language, since they need that and this makes that easily available. But data? Is passing that in as an argument really that much easier? It's just $job_item->getData(), where you can also look up the documentation for it IIRC, while we'd either have something useless or duplicated on the $data argument.

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented

OK you are right, i was possibly thinking too far..
I mean, you could also get the info from the item, that you should ask for the right target language... ;-)

I thought if we already pass the info to the API about what target language to save, we can also pass the data at the same time.
Then, if ever we make the job_item work different and possibly even support multiple target languages, the API would stay and the source can cleanly save the specific translation.
...But i'm pretty sure you will kill this direction of thought immediately. ;-P

edurenye’s picture

edurenye commented
Status: Needs work » Needs review
StatusFileSize
new pass_target_language-2569643-6.patch5.62 KB
new interdiff-pass_target_language-2569643-2-6.txt6.03 KB

Sorry for that misspelling error.
I changed that and deleted data in the calls.

giancarlosotelo’s picture

giancarlosotelo commented
Status: Needs review » Reviewed & tested by the community

Comments were addressed and the patch looks good. tmgmt its broken now but I think it could be rtbc.

  • Berdir committed 575b452 on 8.x-1.x authored by edurenye 
    Issue #2569643 by edurenye: Pass target language to the source...
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Title: Pass target language and data to the source saveTranslation » Pass target language to the source saveTranslation
Status: Reviewed & tested by the community » Fixed

Fixed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.