$page->update() fills missing fields with empty values or copies main language values, ignoring blueprint settings

[hdodov]

The $page->update() method fills in the missing fields from its input with empty values or duplicates those of the main language file, ignoring translate: false.

The $file->update() method has the same problem.

Steps to reproduce the behavior:

1. Create a page with page.en.txt:

Title: Hello World

----

Description: Testing

----

Color: #FACFAC

and blueprint:

title: Test Page
preset: page
fields:
  title:
    type: text
  description:
    type: text
  color:
    type: text
    translate: false

2. Update the title value of that page in another language:

$page = site()->page('hello-world');
$page->update([
  'title' => 'Нов Свят'
], 'bg');

3. Open page.bg.txt (the secondary language file) and you'll see the title field updated, but also the description and color (which has translate: false) are copied from the main language file:

Title: Нов Свят

----

Description: Testing

----

Color: #FACFAC

This problem occurs even if I provide null for description and color and/or use the third parameter of update() for validation.

Expected behavior
The update() method should create the secondary language file, but only populate the title field:

Title: Нов Свят

Why?
Because if description exists in page.bg.txt, it will no longer be synced with the English version as it should. Since the panel admin did not provide any value and the call to update() as well, it should be the same as the default language value. If non-existent in page.bg.txt, Kirby will use the value in page.en.txt as expected.

The same problem exists for color except it's worse. Since the field has translate: false, you can't change the value in the panel and because there's a value in page.bg.txt, any changes to page.en.txt will not be reflected. This means that the value in both languages will be different, defeating the purpose of translate: false.

Kirby Version
3.0.3

Desktop (please complete the following information):

• OS: Windows 10
• Browser Chrome
• Version 72.0.3626.121 (Official Build) (64-bit)

[lukaskleinschmidt]

I can confirm this and I needed it to be fixed asap because I currently rely on this to work. I found the problem to be within the Kirby\Cms\Form class.

For anyone in need of a quick fix:
https://github.com/getkirby/kirby/blob/master/src/Cms/ModelWithContent.php#L407

$form = Form::for($this, [
    'input'          => $input,
    'ignoreDisabled' => $validate === false,
+   'languageCode'   => $languageCode,
]);

https://github.com/getkirby/kirby/blob/master/src/Cms/Form.php#L24

- $isDefaultLanguage = $kirby->language()->isDefault();
+ $languageCode      = $props['languageCode'] ?? $kirby->language()->code();
+ $isDefaultLanguage = $languageCode === $kirby->defaultLanguage()->code();

@bastianallgeier I can prepare a PR if you like. At least on first sight it seems to be an easy fix.

[bastianallgeier]

@lukaskleinschmidt that would be great!

[hdodov]

This lukaskleinschmidt@b02b7ff fixes the issue partly. I just tried it and the translate: false values are indeed not copied, but all other fields are copied, even if they have empty values.

This means that in my example above, title and description would still be copied, just color won't.

If I provide null for description in my update() call, the values are still copied.

[hdodov]

That same thing happens if I use the panel too, by the way. If I edit a page in Bulgarian that does not have a page.bg.txt, change only the description, for example, and save my changes, I get a file with the modified description and all translatable values copied. Basically, the panel and $page->update() seem to have the same behavior with the same issue now.

I don't know that's the intended behavior by the devs, but it might not be the expected one. If I edit a field in another language, and see that the other fields have the same values (Kirby panel visualizes missing translated values by showing the default language ones), I expect those values to continue to be synchronized.

If not that, in the API, I definitely expect $page->update() to update only the stuff I requested it to update.

[lukaskleinschmidt]

As far as I can tell the copy of all values is currently intended and therefore expected behaviour.

[hdodov]

The copy of values in the panel - yes, I think that can be expected. You edit the page in the default language, then switch to another language and by editing and then saving those changes, you commit that the translation should no longer be synchronized with the default values.

But in the API, I think that this behavior should be different. At least it should be controlled via an argument. It would open more possibilities for plugin devs. I currently am developing a plugin that would benefit greatly from a functionality like that. If I have a translation for a given value, I want to use it. If I don't, I want a fallback to the latest default value.

[lukaskleinschmidt]

I did not try it but does the same happen if you use the $page->save() method? I think it should work.

[hdodov]

If I use $page->save(), I get the same behavior as I did without the fix you did above, except I can pass null as a value for a field and it will not be saved. Meaning, if in my example above, I call:

$page->save([
  'title' => 'New Title',
  'description' => null
]);

I would get the following in my page.bg.txt:

Title: New Title

----

Color: #FACFAC

[hdodov]

I get the desired behavior if I use $page->writeContent() though. So I guess @lukaskleinschmidt's fix is enough for the first part of this issue and for the second one, the docs for update, save and writeContent should better describe the differences between those methods.

[bastianallgeier]

✅

[hdodov]

We should reopen this issue because now, there are issues in structures. Non-translatable fields should be skipped, but not when in structures.

I have the following site.yml:

fields:
  text:
    type: text
  textConst:
    type: text
    translate: false
  test:
    type: structure
    fields:
      text:
        type: text
      textConst:
        type: text
        translate: false

...and the following site.en.txt:

Text: Foo

----

Textconst: Bar

----

Test:

- 
  text: foo
  textconst: bar

Then, I update the Bulgarian translation with the values Text = FooBG and Test/text = fooBG. That's the resulting site.bg.txt:

Text: FooBG

----

Test:

- 
  text: fooBG

As you can see, the Textconst field is missing, and it should be - this way Kirby will use the default translation value. However, the textconst field in the structure is also missing. This means that I get the value null for it in the template:

var_dump($site->test()->toStructure()->first()->text()->value()); // "fooBG"
var_dump($site->test()->toStructure()->first()->textConst()->value()); // null

At the same time, that value is grayed out in the panel and you can't fill it:

What's interesting is that even if I add textconst in site.bg.txt manually with my text editor, refresh the panel, and update the translation, it gets removed. (1)

---

I see two solutions for that:

1. In the toStructure() call, Kirby also loads the default translation values and does array_replace_recursive() with the current translation values
2. Kirby should leave values in structures as they are, so that (1) doesn't happen and plugins can handle the synchronization of values across translations

I'm currently working on a plugin that traverses the page blueprints and synchronizes structures according to them. For the most part, it works great, but it fails when it tries to copy values from the default translation - they are lost in the $page->update() call. If you implement the second solution I proposed, my plugin would be able to solve the problem.

[afbora]

@hdodov Can you create a new one by referencing this issue please?