Novactive / NovaeZSEOBundle

Novactive eZ Publish and Platform SEO Bundle
MIT License
25 stars 57 forks source link

Delete field data in case there is some before storing #64

Closed Balamung closed 7 years ago

Balamung commented 7 years ago

In some instances where you update a content throught the eZPublish API, a database exception was thrown because these data were already set when creating the contentUpdateDraft.

To prevent that, a call to deleteFieldData was added before storing any new data.

Plopix commented 7 years ago

hey @Balamung I have looked into that quickly, not in the condition to test like you.

Can you look why

    public function storeFieldData( VersionInfo $versionInfo, Field $field, array $context )
    {
        if ( empty( $field->value->externalData ) )
        {
            return;
        }

        /** @var LegacyStorage $gateway */
        $gateway = $this->getGateway( $context );

        $metas = $gateway->loadFieldData( $versionInfo, $field );
        if ($metas) {
            $gateway->deleteFieldData( $versionInfo, array($field->id) );
        }

        $gateway->storeFieldData( $versionInfo, $field );
    }

We should enter into

      $metas = $gateway->loadFieldData( $versionInfo, $field );
        if ($metas) {
            $gateway->deleteFieldData( $versionInfo, array($field->id) );
        }

Then that would mean the loadFieldData is not working?

Thank you for help!

Balamung commented 7 years ago

Hi ! :)

Where did you get the following block ?

$metas = $gateway->loadFieldData( $versionInfo, $field );
if ($metas) {
    $gateway->deleteFieldData( $versionInfo, array($field->id) );
}

It does not seem to be there for either the master or develop-6.5.x.

I tried it and loadFieldData causes an exception because it is a protected method (and requires different parameters). However, if I change it to

$metas = $gateway->getFieldData( $versionInfo, $field );

, it seems to resolve the issue :+1:

Plopix commented 7 years ago

was indeed fix in the develop-ezplatform branch but not in the legacy.

https://github.com/Novactive/NovaeZSEOBundle/commit/06913f73c888a80ed3b8416851133f94f7913a9f

Thank you!