A new media is created after update an old one from Admin Form

[anacona16]

Environment

Sonata packages

$ composer show sonata-project/*
sonata-project/admin-bundle              3.20.1 The missing Symfony Admin Generator
sonata-project/block-bundle              3.3.2  Symfony SonataBlockBundle
sonata-project/cache                     1.0.7  Cache library
sonata-project/core-bundle               3.4.0  Symfony SonataCoreBundle
sonata-project/datagrid-bundle           2.2.1  Symfony SonataDatagridBundle
sonata-project/doctrine-extensions       1.0.2  Doctrine2 behavioral extensions
sonata-project/doctrine-orm-admin-bundle 3.1.6  Symfony Sonata / Integrate Doctrine ORM into the SonataAdminBundle
sonata-project/easy-extends-bundle       2.2.0  Symfony SonataEasyExtendsBundle
sonata-project/exporter                  1.7.1  Lightweight Exporter library
sonata-project/media-bundle              3.6.0  Symfony SonataMediaBundle
sonata-project/notification-bundle       3.1.0  Symfony SonataNotificationBundle

Symfony packages

$ composer show symfony/*
symfony/assetic-bundle     v2.8.2  Integrates Assetic into Symfony2
symfony/monolog-bundle     v2.12.1 Symfony MonologBundle
symfony/phpunit-bridge     v3.3.6  Symfony PHPUnit Bridge
symfony/polyfill-intl-icu  v1.4.0  Symfony polyfill for intl's ICU-related data and classes
symfony/polyfill-mbstring  v1.4.0  Symfony polyfill for the Mbstring extension
symfony/polyfill-php56     v1.4.0  Symfony polyfill backporting some PHP 5.6+ features to lower PHP versions
symfony/polyfill-php70     v1.4.0  Symfony polyfill backporting some PHP 7.0+ features to lower PHP versions
symfony/polyfill-util      v1.4.0  Symfony utilities for portability of PHP codes
symfony/security-acl       v3.0.0  Symfony Security Component - ACL (Access Control List)
symfony/swiftmailer-bundle v2.6.3  Symfony SwiftmailerBundle
symfony/symfony            v3.3.6  The Symfony PHP framework

PHP version

$ php -v
PHP 5.6.30-12~ubuntu16.04.1+deb.sury.org+1 (cli) 
Copyright (c) 1997-2016 The PHP Group
Zend Engine v2.6.0, Copyright (c) 1998-2016 Zend Technologies
    with Zend OPcache v7.0.6-dev, Copyright (c) 1999-2016, by Zend Technologies

Subject

A new media is created after update an old one. There is not a way to set new_on_update to false

Steps to reproduce

Create a new media (image, file, YouTube, Vimeo, etc) then update the same media uploading a new file or image or url.

Expected results

The same record should be updated with new metadata information.

Actual results

A new media is created, and the metadata for the old one is updated too including the file or image or url.

Request log

####################################################################################################
#######################################  02/Aug/2017 08:05:47  #####################################
####################################################################################################
___ REQUEST ________________________________________________________________________________________
Matched route "admin_sonata_media_media_edit".
--> route: admin_sonata_media_media_edit
--> route_parameters:
      _controller: 'Sonata\MediaBundle\Controller\MediaAdminController::editAction'
      _sonata_admin: sonata.media.admin.media
      _sonata_name: admin_sonata_media_media_edit
      id: '28'
      _route: admin_sonata_media_media_edit
--> request_uri: 'http://symfony_test.dev/app_dev.php/admin/sonata/media/media/28/edit?context=default&hide_context=0&uniqid=s5981cd655cd95'
--> method: POST
___ (!) SECURITY ___________________________________________________________________________________
Populated the TokenStorage with an anonymous Token.
___ DOCTRINE _______________________________________________________________________________________
SELECT t0.name AS name_1, t0.description AS description_2, t0.enabled AS enabled_3,
  t0.provider_name AS provider_name_4, t0.provider_status AS provider_status_5,
  t0.provider_reference AS provider_reference_6, t0.provider_metadata AS provider_metadata_7,
  t0.width AS width_8, t0.height AS height_9, t0.length AS length_10, t0.content_type AS
  content_type_11, t0.content_size AS content_size_12, t0.copyright AS copyright_13, t0.author_name
  AS author_name_14, t0.context AS context_15, t0.cdn_is_flushable AS cdn_is_flushable_16,
  t0.cdn_flush_identifier AS cdn_flush_identifier_17, t0.cdn_flush_at AS cdn_flush_at_18,
  t0.cdn_status AS cdn_status_19, t0.updated_at AS updated_at_20, t0.created_at AS created_at_21,
  t0.id AS id_22 FROM media__media t0 WHERE t0.id = ?
--> 'query params': ['28']
___ DOCTRINE _______________________________________________________________________________________
"START TRANSACTION"
___ DOCTRINE _______________________________________________________________________________________
INSERT INTO media__media (name, description, enabled, provider_name, provider_status,
  provider_reference, provider_metadata, width, height, length, content_type, content_size,
  copyright, author_name, context, cdn_is_flushable, cdn_flush_identifier, cdn_flush_at, cdn_status,
  updated_at, created_at) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
--> 'query params': { 1: 'Conoce el framework PHP Symfony', 2: null, 3: false, 4: sonata.media.provider.youtube, 5: 1, 6: 2XFiTXbOPLk, 7: { thumbnail_height: 360, thumbnail_url: 'https://i.ytimg.com/vi/2XF [...]', provider_url: 'https://www.youtube.com/', width: 480, type: video, author_name: DesarrolloWeb.com, height: 270, version: '1.0', thumbnail_width: 480, title: 'Conoce el framework PHP Symfony', author_url: 'https://www.youtube.com/us [...]', provider_name: YouTube, html: '<iframe width="480" height [...]' }, 8: 480, 9: 270, 10: null, 11: video/x-flv, 12: null, 13: null, 14: DesarrolloWeb.com, 15: default, 16: null, 17: null, 18: null, 19: null, 20: '2017-08-02T08:05:48-05:00', 21: '2017-08-02T08:05:48-05:00' }
___ DOCTRINE _______________________________________________________________________________________
UPDATE media__media SET provider_reference = ?, cdn_is_flushable = ?, name = ?, provider_metadata
  = ?, author_name = ?, cdn_status = ?, updated_at = ? WHERE id = ?
--> 'query params': [2XFiTXbOPLk, true, 'Conoce el framework PHP Symfony', { author_url: 'https://www.youtube.com/us [...]', thumbnail_width: 480, html: '<iframe width="480" height [...]', author_name: DesarrolloWeb.com, title: 'Conoce el framework PHP Symfony', version: '1.0', thumbnail_url: 'https://i.ytimg.com/vi/2XF [...]', width: 480, provider_name: YouTube, height: 270, provider_url: 'https://www.youtube.com/', type: video, thumbnail_height: 360 }, DesarrolloWeb.com, 3, '2017-08-02T08:05:49-05:00', 28]
___ DOCTRINE _______________________________________________________________________________________
"COMMIT"


####################################################################################################
#######################################  02/Aug/2017 08:05:49  #####################################
####################################################################################################
___ REQUEST ________________________________________________________________________________________
Matched route "admin_sonata_media_media_edit".
--> route: admin_sonata_media_media_edit
--> route_parameters:
      _controller: 'Sonata\MediaBundle\Controller\MediaAdminController::editAction'
      _sonata_admin: sonata.media.admin.media
      _sonata_name: admin_sonata_media_media_edit
      id: '29'
      _route: admin_sonata_media_media_edit
--> request_uri: 'http://symfony_test.dev/app_dev.php/admin/sonata/media/media/29/edit?context=default&hide_context=0'
--> method: GET
___ (!) SECURITY ___________________________________________________________________________________
Populated the TokenStorage with an anonymous Token.
___ DOCTRINE _______________________________________________________________________________________
SELECT t0.name AS name_1, t0.description AS description_2, t0.enabled AS enabled_3,
  t0.provider_name AS provider_name_4, t0.provider_status AS provider_status_5,
  t0.provider_reference AS provider_reference_6, t0.provider_metadata AS provider_metadata_7,
  t0.width AS width_8, t0.height AS height_9, t0.length AS length_10, t0.content_type AS
  content_type_11, t0.content_size AS content_size_12, t0.copyright AS copyright_13, t0.author_name
  AS author_name_14, t0.context AS context_15, t0.cdn_is_flushable AS cdn_is_flushable_16,
  t0.cdn_flush_identifier AS cdn_flush_identifier_17, t0.cdn_flush_at AS cdn_flush_at_18,
  t0.cdn_status AS cdn_status_19, t0.updated_at AS updated_at_20, t0.created_at AS created_at_21,
  t0.id AS id_22 FROM media__media t0 WHERE t0.id = ?
--> 'query params': ['29']

As you can see an INSERT INTO media__media statement is executed, also an UPDATE media__media statement is executed later.

Problems found

BaseMediaAdmin class is using the ProviderDataTransformer class but is not setting value for new_on_update flag and this value is always true.

I don't see a way to set that value to false.

Proposed solution

In this method check if $media->getId() === null if is null then set 'new_on_update' => true if not then set 'new_on_update' => false, and pass a new array argument this class ProviderDataTransformer

This solution won't affect to sonata_media_type form type.

Issues related

#974 but using form type: sonata_media_type

[anacona16]

Proposed solution

If I'm updating a media from an admin form that's what I want to do: update it, right? So, I think the best option is pass 'new_on_update' => false always

[greg0ire]

new_on_update defaults to true, but it looks like you should be able to set it: https://github.com/sonata-project/SonataMediaBundle/blob/fabd14b939267ab9ef622963f14aed1cf26caa4c/Resources/doc/reference/form.rst .

[anacona16]

@greg0ire yes it's possible but only if you use the form sonata_media_type, but is not possible to set it from admin form.

[greg0ire]

Have you checked the contents of this variable at runtime?

[anacona16]

It's always true because none option is passed from here https://github.com/sonata-project/SonataMediaBundle/blob/3.x/Admin/BaseMediaAdmin.php#L197

[greg0ire]

Yeah but options are not passed on construct, they are passed when calling getOptions. You can throw an exception in that method, that will give you a stack trace, helpful to see where $options comes from.

[anacona16]

This is the stack trace:

Exception:

  at vendor/sonata-project/media-bundle/Form/DataTransformer/ProviderDataTransformer.php:151
  at Sonata\MediaBundle\Form\DataTransformer\ProviderDataTransformer->getOptions(array())
     (vendor/sonata-project/media-bundle/Form/DataTransformer/ProviderDataTransformer.php:48)
  at Sonata\MediaBundle\Form\DataTransformer\ProviderDataTransformer->__construct(object(Pool), 'Application\\Sonata\\MediaBundle\\Entity\\Media')
     (vendor/sonata-project/media-bundle/Admin/BaseMediaAdmin.php:197)
  at Sonata\MediaBundle\Admin\BaseMediaAdmin->configureFormFields(object(FormMapper))
     (app/cache/dev/classes.php:3228)
  at Sonata\AdminBundle\Admin\AbstractAdmin->defineFormBuilder(object(FormBuilder))
     (app/cache/dev/classes.php:3222)
  at Sonata\AdminBundle\Admin\AbstractAdmin->getFormBuilder()
     (app/cache/dev/classes.php:4227)
  at Sonata\AdminBundle\Admin\AbstractAdmin->buildForm()
     (app/cache/dev/classes.php:3261)
  at Sonata\AdminBundle\Admin\AbstractAdmin->getForm()
     (vendor/sonata-project/admin-bundle/Controller/CRUDController.php:255)
  at Sonata\AdminBundle\Controller\CRUDController->editAction('67')
  at call_user_func_array(array(object(MediaAdminController), 'editAction'), array('67'))
     (app/cache/dev/classes.php:18855)
  at Symfony\Component\HttpKernel\HttpKernel->handleRaw(object(Request), 1)
     (app/cache/dev/classes.php:18810)
  at Symfony\Component\HttpKernel\HttpKernel->handle(object(Request), 1, true)
     (vendor/symfony/symfony/src/Symfony/Component/HttpKernel/Kernel.php:171)
  at Symfony\Component\HttpKernel\Kernel->handle(object(Request))
     (web/app_dev.php:12)

As you can see, options is empty.

[greg0ire]

Indeed. Please have a look at the calling function to know why it is empty there, and so on.

[anacona16]

The option in the __construct method for ProviderDataTransformer is by default empty: https://github.com/sonata-project/SonataMediaBundle/blob/3.x/Form/DataTransformer/ProviderDataTransformer.php#L45

From here: the transformers is being calling without options: https://github.com/sonata-project/SonataMediaBundle/blob/3.x/Admin/BaseMediaAdmin.php#L197

I know you said that options are not passed on construct, but here https://github.com/sonata-project/SonataMediaBundle/blob/3.x/Form/Type/MediaType.php#L73 in the sonata_media_type form they are passing the options on construct.

[greg0ire]

You're right, reading your stack trace, it looks like getOptions is called in the constructor.

[greg0ire]

I think the right fix would be to have a property on the admin to configure that, and expose it in the configuration.

[greg0ire]

Or maybe it worked as you expected before? Can you check previous versions of the media bundle?

[anacona16]

Maybe you're right, but my question is, if you have access to the admin form (sonata admin interface) and you choose to update a media why a new media is created?

[greg0ire]

I don't know or use this bundle, but the answer seems to be in the docs

then the media will be overwritten and related entities can be affected by this change.

[anacona16]

Of course, by relationships with other entities. But, I think they're talking about the asset not about the entity.

Here: http://demo.sonata-project.org/ it's working as expected, can you confirm which version are installed for that demo?