When the source is EDITED and then is re-uploaded any target that is NOT_CURRENT should be changed to PENDING but it remains the same.

LingotekContentTranslationService::updateDocument() the status of all targets were set to STATUS_REQUEST and then when the form was being built there is logic for if there is a translation for that langcode AND the status is set to STATUS_REQUEST then set status to STATUS_UNTRACKED because it means there is already a translation and this situation would and should mean that we are not handling the translation. In LingotekContentTranslationService::updateDocument() we should NOT be setting the status to STATUS_REQUEST but rather to STATUS_PENDING. And a target should only be moved to UNTRACKED if it is disassociated.

CommentFileSizeAuthor
#55 interdiff.txt3.62 KBt.murphy
#54 target_set_to_untracked-2850534-54.patch13.42 KBt.murphy
#53 target_set_to_untracked-2850534-53.patch13.34 KBt.murphy
#52 target_set_to_untracked-2850534-52.patch10.69 KBt.murphy
#50 target_set_to_untracked-2850534-50.patch10.68 KBt.murphy
#48 target_set_to_untracked-2850534-48.patch10.82 KBt.murphy
#45 target_set_to_untracked-2850534-45.patch6.04 KBt.murphy
#40 interdiff.txt3.33 KBpenyaskito
#37 target_set_to_untracked-2850534-37.patch28.05 KBt.murphy
#37 interdiff.txt4.71 KBt.murphy
#35 interdiff.txt1.81 KBpenyaskito
#35 target_set_to_untracked-2850534-35.patch28.91 KBpenyaskito
#33 target_set_to_untracked-2850534-33.patch27.34 KBt.murphy
#31 target_set_to_untracked-2850534-31.patch25 KBt.murphy
#29 target_set_to_untracked-2850534-29.patch25.01 KBt.murphy
#27 target_set_to_untracked-2850534-27.patch25.49 KBt.murphy
#25 target_set_to_untracked-2850534-25.patch25.49 KBt.murphy
#23 target_set_to_untracked-2850534-23.patch25.54 KBt.murphy
#21 target_set_to_untracked-2850534-21.patch24.5 KBt.murphy
#19 target_set_to_untracked-2850534-19.patch23.68 KBt.murphy
#17 target_set_to_untracked-2850534-17.patch12.35 KBt.murphy
#15 target_set_to_untracked-2850534-15.patch12.34 KBt.murphy
#12 target_set_to_untracked-2850534-12.patch12.01 KBt.murphy
#11 target_set_to_untracked-2850534-11.patch12.01 KBt.murphy
#9 target_set_to_untracked-2850534-9.patch12 KBt.murphy
#8 target_set_to_untracked-2850534-8.patch5.54 KBt.murphy
#6 target_set_to_untracked-2850534-6.patch3.52 KBt.murphy
#4 target_set_to_untracked-2850534-4.patch3.12 KBt.murphy
#2 target_set_to_untracked-2850534-2.patch1.42 KBt.murphy

Comments

t.murphy created an issue. See original summary.

t.murphy’s picture

Status: Active » Needs review
StatusFileSize
new1.42 KB

Status: Needs review » Needs work

The last submitted patch, 2: target_set_to_untracked-2850534-2.patch, failed testing.

t.murphy’s picture

Status: Needs work » Needs review
StatusFileSize
new3.12 KB

Status: Needs review » Needs work

The last submitted patch, 4: target_set_to_untracked-2850534-4.patch, failed testing.

t.murphy’s picture

Status: Needs work » Needs review
StatusFileSize
new3.52 KB

Status: Needs review » Needs work

The last submitted patch, 6: target_set_to_untracked-2850534-6.patch, failed testing.

t.murphy’s picture

Status: Needs work » Needs review
StatusFileSize
new5.54 KB
t.murphy’s picture

StatusFileSize
new12 KB

Status: Needs review » Needs work

The last submitted patch, 9: target_set_to_untracked-2850534-9.patch, failed testing.

t.murphy’s picture

Status: Needs work » Needs review
StatusFileSize
new12.01 KB
t.murphy’s picture

StatusFileSize
new12.01 KB

The last submitted patch, 11: target_set_to_untracked-2850534-11.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 12: target_set_to_untracked-2850534-12.patch, failed testing.

t.murphy’s picture

Status: Needs work » Needs review
StatusFileSize
new12.34 KB

Status: Needs review » Needs work

The last submitted patch, 15: target_set_to_untracked-2850534-15.patch, failed testing.

t.murphy’s picture

Status: Needs work » Needs review
StatusFileSize
new12.35 KB
penyaskito’s picture

Status: Needs review » Needs work
+++ b/src/Tests/LingotekNodeTranslationFlowNotCurrentToPendingTest.php
@@ -0,0 +1,139 @@
+class LingotekNodeTranslationFlowNotCurrentToPendingTest extends LingotekTestBase {
...
+  public function testNodeTargetStatusAfterSourceEditAndUpload() {

Didn't look at this in detail yet, but at least we need more tests for config and config entities.

t.murphy’s picture

Status: Needs work » Needs review
StatusFileSize
new23.68 KB

Status: Needs review » Needs work

The last submitted patch, 19: target_set_to_untracked-2850534-19.patch, failed testing.

t.murphy’s picture

Status: Needs work » Needs review
StatusFileSize
new24.5 KB

Status: Needs review » Needs work

The last submitted patch, 21: target_set_to_untracked-2850534-21.patch, failed testing.

t.murphy’s picture

Status: Needs work » Needs review
StatusFileSize
new25.54 KB

Status: Needs review » Needs work

The last submitted patch, 23: target_set_to_untracked-2850534-23.patch, failed testing.

t.murphy’s picture

Status: Needs work » Needs review
StatusFileSize
new25.49 KB

Status: Needs review » Needs work

The last submitted patch, 25: target_set_to_untracked-2850534-25.patch, failed testing.

t.murphy’s picture

Status: Needs work » Needs review
StatusFileSize
new25.49 KB

Status: Needs review » Needs work

The last submitted patch, 27: target_set_to_untracked-2850534-27.patch, failed testing.

t.murphy’s picture

Status: Needs work » Needs review
StatusFileSize
new25.01 KB

Status: Needs review » Needs work

The last submitted patch, 29: target_set_to_untracked-2850534-29.patch, failed testing.

t.murphy’s picture

Status: Needs work » Needs review
StatusFileSize
new25 KB

Status: Needs review » Needs work

The last submitted patch, 31: target_set_to_untracked-2850534-31.patch, failed testing.

t.murphy’s picture

Status: Needs work » Needs review
StatusFileSize
new27.34 KB

Status: Needs review » Needs work

The last submitted patch, 33: target_set_to_untracked-2850534-33.patch, failed testing.

penyaskito’s picture

Status: Needs work » Needs review
StatusFileSize
new28.91 KB
new1.81 KB

Removed the ToDo and changed tests as now #2850545: Check All Translation Progress Action not updating status correctly has been committed.

penyaskito’s picture

  1. +++ b/src/LingotekConfigTranslationService.php
    @@ -258,15 +258,27 @@ class LingotekConfigTranslationService implements LingotekConfigTranslationServi
    -        if ($current_status === Lingotek::STATUS_PENDING && $status === Lingotek::STATUS_REQUEST) {
    +        if ($current_status === Lingotek::STATUS_PENDING &&
    +        $status === Lingotek::STATUS_REQUEST) {
    

    This change mades readability worse IMHO.

  2. +++ b/src/LingotekConfigTranslationService.php
    @@ -258,15 +258,27 @@ class LingotekConfigTranslationService implements LingotekConfigTranslationServi
    +        if (in_array( $current_status,
    +                      array(  Lingotek::STATUS_UNTRACKED,
    +                              Lingotek::STATUS_DISABLED
    +                      )) &&
    +        $status === Lingotek::STATUS_PENDING) {
    

    Same here, a single line if using short array syntax should be better.

  3. +++ b/src/LingotekConfigTranslationService.php
    @@ -258,15 +258,27 @@ class LingotekConfigTranslationService implements LingotekConfigTranslationServi
    +        elseif ($current_status == Lingotek::STATUS_EDITED && in_array($status,
    +        array(Lingotek::STATUS_CURRENT, Lingotek::STATUS_PENDING))) {
    

    Short array syntax here too.

  4. +++ b/src/Tests/LingotekConfigEntityTranslationEditedToPendingTest.php
    @@ -0,0 +1,116 @@
    +    // TODO: once the config is following the correct translation flow then the
    +    // following tests will work, but for now it's not
    

    What's wrong here then? Do we have an issue?

  5. +++ b/src/Tests/LingotekConfigTranslationEditedToPendingTest.php
    @@ -0,0 +1,133 @@
    +    // Set Site name back to original name
    +    $config2->set('name', $original_site_name)->save();
    

    Why is this needed?

t.murphy’s picture

StatusFileSize
new4.71 KB
new28.05 KB
t.murphy’s picture

The last patch has been updated to fit the comments. The only one that doesn't have a change is #4 because the test really needs to continue farther, but the module does not follow the translation flow for the config management and so the TODO is meant to wait for the module to function as expected and then the tests are already written and ready to be added to the end of the test.

penyaskito’s picture

Status: Needs review » Reviewed & tested by the community

This looks better, thanks! I will fix a couple of coding standards issues at commit (extra spaces on in_array and short syntax arrays)

penyaskito’s picture

StatusFileSize
new3.33 KB

Interdiff before commit.

  • penyaskito committed 6ec333e on 8.x-1.x authored by t.murphy
    Issue #2850534 by t.murphy, penyaskito: Target set to UNTRACKED
    
penyaskito’s picture

Status: Reviewed & tested by the community » Fixed

Committed 6ec333e and pushed to 8.x-1.x. Thanks!

  • penyaskito committed fa5906e on 8.x-2.x authored by t.murphy
    Issue #2850534 by t.murphy, penyaskito: Target set to UNTRACKED
    
penyaskito’s picture

Committed fa5906e and pushed to 8.x-2.x. Thanks!

t.murphy’s picture

Status: Fixed » Needs review
StatusFileSize
new6.04 KB

This changed made it so that when there are other targets that are in the REQUEST status they also move to PENDING even though they shouldn't. This updated patch fixes that.

Status: Needs review » Needs work

The last submitted patch, 45: target_set_to_untracked-2850534-45.patch, failed testing.

penyaskito’s picture

If our tests didn't detect that we definitely need tests. If this patch failed, is there a problem with requirement or a big in our tests?

t.murphy’s picture

Status: Needs work » Needs review
StatusFileSize
new10.82 KB

That last test added new tests and those new tests are what failed. So I have another version that hopefully will pass. The problem was in that this scenario was not tested.

Status: Needs review » Needs work

The last submitted patch, 48: target_set_to_untracked-2850534-48.patch, failed testing.

t.murphy’s picture

Status: Needs work » Needs review
StatusFileSize
new10.68 KB

Status: Needs review » Needs work

The last submitted patch, 50: target_set_to_untracked-2850534-50.patch, failed testing.

t.murphy’s picture

Status: Needs work » Needs review
StatusFileSize
new10.69 KB
t.murphy’s picture

StatusFileSize
new13.34 KB
t.murphy’s picture

StatusFileSize
new13.42 KB
t.murphy’s picture

StatusFileSize
new3.62 KB

penyaskito’s picture

Status: Needs review » Fixed

Committed 9039a4b and pushed to 8.x-1.x. Thanks!
Committed 3d3a26e and pushed to 8.x-2.x. Thanks!

Status: Fixed » Closed (fixed)

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