Problem/Motivation

Fifth step for the #2464445: [meta] Jobless / continuous translators.

Proposed resolution

Add a new interface for translators, ContinuousTranslatorInterface, with a requestJobItemTranslation(JobItemInterface $job_item) method. Limit the allowed translators for continuous jobs to that interface. Implement it in the test translator. information like language and translator settings can be accessed through getJob().

Remaining tasks

User interface changes

API changes

Data model changes

Comments

edurenye created an issue. See original summary.

edurenye’s picture

edurenye’s picture

Status: Active » Needs review
StatusFileSize
new8.27 KB

Added the interface.
Make test_translator implement the interface.
Just can assign a continuous job to a ContinuousTranslator.
List of continuous jobs now don't show non continuous jobs.

berdir’s picture

Status: Needs review » Needs work
  1. +++ b/src/ContinuousTranslatorInterface.php
    @@ -0,0 +1,25 @@
    +   * @param JobItemInterface $job_item
    +   *   The JobItem we want to translate.
    +   */
    +  public function requestJobItemTranslation(JobItemInterface $job_item);
    

    as discussed, make this plural, an array of job items.

  2. +++ b/tmgmt.module
    @@ -471,6 +473,9 @@ function tmgmt_translator_labels_flagged(JobInterface $job = NULL) {
    +    if ($job->isContinuous() && !($translator->getPlugin() instanceof ContinuousTranslatorInterface)) {
    +      continue;
    

    $job is aparrently an optionl argument here, so we should check that one is passed in?

  3. +++ b/tmgmt_test/src/Plugin/tmgmt/Translator/TestTranslator.php
    @@ -152,4 +153,32 @@ class TestTranslator extends TranslatorPluginBase implements TranslatorRejectDat
    +  public function requestJobItemTranslation(JobItemInterface $job_item) {
    +    // Add a debug message.
    +    $job_item->addMessage('Test translator called.', array(), 'debug');
    

    can you add a (continuous) or something to the message here?

  4. +++ b/translators/tmgmt_file/src/Tests/FileTranslatorTest.php
    @@ -452,6 +452,11 @@ class FileTranslatorTest extends TMGMTTestBase {
     
    +    // Ensure this Job is not listed as continuous jobs as FileTranslator is not
    +    // a ContinuousTranslator.
    +    $this->drupalGet('admin/tmgmt/continuous_jobs');
    +    $this->assertText('No jobs available.');
    

    That's not what I meant. It wouldn't be listed there even if file would support that. because it's not a continuous job.

    You need to extend TMGMTUitest instead, around line 717, on the add form.

The last submitted patch, 3: add_an_interface_for-2670258-3.patch, failed testing.

edurenye’s picture

Status: Needs work » Needs review
StatusFileSize
new8.24 KB
new5.4 KB

Done.

Status: Needs review » Needs work

The last submitted patch, 6: add_an_interface_for-2670258-6.patch, failed testing.

The last submitted patch, 6: add_an_interface_for-2670258-6.patch, failed testing.

berdir’s picture

+++ b/src/Tests/TMGMTUiTest.php
@@ -712,6 +712,11 @@ class TMGMTUiTest extends TMGMTTestBase {
+    // Ensure this Jobs are not listed in the continuous jobs list as
+    // they are not continuous jobs.
+    $this->drupalGet('admin/tmgmt/continuous_jobs');
+    $this->assertText('No jobs available.');
+

Still no.

You need to check available translators, not jobs.

edurenye’s picture

Status: Needs work » Needs review
StatusFileSize
new8.61 KB
new2.13 KB

Done.

Status: Needs review » Needs work

The last submitted patch, 10: add_an_interface_for-2670258-10.patch, failed testing.

edurenye’s picture

Status: Needs work » Needs review
StatusFileSize
new9.11 KB
new924 bytes

Now I'm just installing tmgmt_file in the test where I need it.

berdir’s picture

Status: Needs review » Needs work
+++ b/src/Tests/TMGMTUiTest.php
@@ -740,11 +741,12 @@ class TMGMTUiTest extends TMGMTTestBase {
     $this->drupalGet('admin/tmgmt/continuous_jobs/continuous_add');
+    $this->assertNoText('File translator (auto created)');

Yeah, the problem with this is that this will no longer work properly when we change the translator label. See #2662498: Rename Translator to Provider. So it's very easy to break it so that it will no longer test anything useful.

For now, lets just shorten it to just checking for no "File". That will still be there, even after the the rename.

Almost there.

edurenye’s picture

Status: Needs work » Needs review
StatusFileSize
new9.41 KB
new1.35 KB

Done.

The last submitted patch, 10: add_an_interface_for-2670258-10.patch, failed testing.

  • Berdir committed 1e23993 on 8.x-1.x authored by edurenye
    Issue #2670258 by edurenye: Add an interface for continuous translators
    
berdir’s picture

Status: Needs review » Fixed

Yes, that works. Committed.

Status: Fixed » Closed (fixed)

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