Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug in translateBehavior #9688

Closed
1 of 3 tasks
blackfalck opened this issue Nov 1, 2016 · 6 comments
Closed
1 of 3 tasks

Bug in translateBehavior #9688

blackfalck opened this issue Nov 1, 2016 · 6 comments

Comments

@blackfalck
Copy link

blackfalck commented Nov 1, 2016

This is a :

  • bug

  • enhancement

  • feature-discussion (RFC)

  • CakePHP Version: 3.3.7

  • Platform and Target: Mysql 5.7.13, Ubuntu, Apache, PHP7, the problem is in our REST Api. Issue exists when editing an emailtemplate entity in Angular or sending the data with Postman.

Default Locale = nl_NL
Locale = nl_NL

What you did

The json i am sending to my API:
{
	"id": "74410fdf-83be-4636-adb1-6cb96a1159fb",
	"name": "forgot_password",
	"system": 1,
	"description": "Forgot Password nl",
	"subject": "you forgot your password",
	"default_cc": null,
	"default_bcc": "support@xseeding.nl",
	"message": "<p class=\"p1\"><span id=\"selectionBoundary_1478005451302_6997624159938602\" class=\"rangySelectionBoundary\">&#65279;</span>Beste __NAME__,</p><p>Helaas bent u niet zo handig en uw wachtwoord vergeten. Gelukkig kunnen we dit voor u fixen.</p><p>Hopelijk lukt het u om onderstaande link te volgen.</p><p>http://rdmparts.xseeding.nl/wachtwoord-vergeten/__HASH__<span id=\"selectionBoundary_1478005451301_015738660491383394\" class=\"rangySelectionBoundary\">&#65279;</span></p>",
	"template": null,
	"layout": null,
	"created": "2016-08-29T11:38:31+00:00",
	"modified": "2016-11-01T13:48:22+00:00",
	"deleted": null,
	"xtranslations": {
		"de_DE": {
			"subject": "Forgot Password 3",
			"description": "Forgot Password de",
			"message": "message de",
			"locale": "de_DE"
		},
		"en_US": {
			"message": "message en",
			"description": "Forgot Password us",
			"subject": "Forgot Password 2",
			"locale": "en_US"
		}
	}
}

We have called it xtranslations for reasons unknown, however we got this in our beforesave:

 public function beforeSave(Event $event, $entity, \ArrayObject $options)
    {
        if (isset($entity->xtranslations)) {
            foreach ($entity->xtranslations as $locale => $data) {
                $entity->translation($locale)->set($data, ['guard' => false]);
            }
        }
    }

Which sets all the translations.

The API functions looks like this:

public function edit($id = null)
    {
        $this->request->allowMethod(['post', 'put']);
        $emailtemplate = $this->Emailtemplates->get($id);

        $emailtemplate = $this->Emailtemplates->patchEntity($emailtemplate, $this->request->data);
        $success = (bool)$this->Emailtemplates->save($emailtemplate);

        $this->set([
            'success' => $success,
            'data' => $emailtemplate,
            'errors' => $emailtemplate->errors()
        ]);
    }

I tried an older version of the translateBehavior, before this pull-request was merged:
#9640

I reckon the problem is in line 282 - translateBehavior:

$values = $entity->extract($this->_config['fields'], true);

Since this returns empty. However there are fields in the entity that can and are translated.

However I am not very certain what the problem is. Since I also don't really get what is tried to fix in the Pull-request.

What happened

Because my default Locale is nl_NL I expect de_DE and en_US to be saved in the I18N table, and the the regular entity fields in the emailtemplates table. Since they are the default language.

However I get nl_NL, de_DE and en_US in the I18N table and the values in emailtemplates table aren't updated.

Expected Behavior

See What Happened.

Thanks for your assistance.

  • Mitchell Izelaar
@markstory
Copy link
Member

What makes you think it is those specific lines? #9640 was intended to fix saving entities when records are created and only translated records are included in the entity. Previously that scenario would result in no records being saved.

How is TranslateBehavior attached to your EmailTemplates model?

@blackfalck
Copy link
Author

Hi Mark,

Well, Because the $values variable stays empty, and I reckon it has to be filled with the fields that need translations.
These fields:

$this->addBehavior('Translate', [
            'fields' => [
                'description',
                'subject',
                'default_cc',
                'default_bcc',
                'message',
            ]
        ]);

That's also how i added the behavior to my EmailtemplatesTable.
And in the Emailtemplate Entity:

 use TranslateTrait;

@Xety
Copy link
Contributor

Xety commented Nov 2, 2016

I have exactly the same problem. My default locale fr_FR is saved in the I18n table and the fields in my table are blank.
Can't say since when because i didn't coded since February... I will try to look into it.

PHP 7.0.8
Cake 3.3.7
MySQL 5.7.16

Edit : I confirm that the bug has been introduced in the 3.3.7, work well with the 3.3.6.

@Xety
Copy link
Contributor

Xety commented Nov 2, 2016

I get it worked again by adding 2 conditions in https://github.com/cakephp/cakephp/blob/master/src/ORM/Behavior/TranslateBehavior.php#L327:

$new = array_diff_key($values, $modified);
        foreach ($new as $field => $content) {
            // We don't need to create an Entity for the defaultLocale
            if ($locale !== $this->config('defaultLocale')) {
                $new[$field] = new Entity(compact('locale', 'field', 'content', 'model'), [
                    'useSetters' => false,
                    'markNew' => true
                ]);
            }
        }

        $entity->set('_i18n', array_merge($bundled, array_values($modified + $new)));
        $entity->set('_locale', $locale, ['setter' => false]);
        $entity->dirty('_locale', false);

        foreach ($fields as $field) {
            // We also don't need to make the fields dirty for the defaultLocale
            if ($locale !== $this->config('defaultLocale')) {
                $entity->dirty($field, false);
            }
        }

But i didn't run the tests to see if it break something, i don't have the environment ready to do it.

@markstory markstory self-assigned this Nov 2, 2016
@markstory
Copy link
Member

Thanks for the additional information, I'll take a look and get a fix together.

markstory added a commit that referenced this issue Nov 2, 2016
The default locale should be saved onto the host table and not into the
translations tables. By returning early when both the default locale and
bundled translations are present we can fix the regression introduced in #9640

Refs #9688
@markstory
Copy link
Member

Pull request up now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants