-
-
Notifications
You must be signed in to change notification settings - Fork 495
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
A new media is created after update an old one from Admin Form #1273
Comments
Proposed solutionIf 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 |
|
@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. |
Have you checked the contents of this variable at runtime? |
It's always true because none option is passed from here https://github.com/sonata-project/SonataMediaBundle/blob/3.x/Admin/BaseMediaAdmin.php#L197 |
Yeah but options are not passed on construct, they are passed when calling |
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, |
Indeed. Please have a look at the calling function to know why it is empty there, and so on. |
The option in the 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 |
You're right, reading your stack trace, it looks like |
I think the right fix would be to have a property on the admin to configure that, and expose it in the configuration. |
Or maybe it worked as you expected before? Can you check previous versions of the media bundle? |
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? |
I don't know or use this bundle, but the answer seems to be in the docs
|
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? |
I don't know, but I think it is based on the sandbox, so you can have a look.
I don't get it. It's clearly written
not assets... |
You're right. I meant, I have a Product entity with: id -> integer
name -> string
image -> ManyToOne with `Sonata\MediaBundle\Entity\Media` --- Rows table_products --- Media Entity # 321 have and Image (asset) with a content X. If I updated the registry and I upload a new image (asset) with a content Y, of course the related entity will be affected, but the change will be only the asset content. I'm installing the sandbox project on my local. |
If it works as you expect in the sandbox, then it must be a bug. Please use |
I tried to test different versions but was almost impossible, there are a lot of dependencies and incompatibilities between bundles. I think I'll override that method in the Application/SonataBundle. |
I found the problem, is not an issue from media-bundle. A recent commit on admin-bundle is causing this issue: see here sonata-project/SonataAdminBundle@9655c1e#commitcomment-22646949 They know about the issue but I don't see any solution at the moment. |
Oh right... any idea how to fix this? |
The quick solution maybe is this: #1273 (comment) But I'm not sure if is the best solution. |
Sorry I meant "fixing sonata-project/SonataAdminBundle@9655c1e#commitcomment-22646949" |
I got temporary solution for this problem. To solve this problem (Stop creating new one, and delete old file/thumbnail and update) , I modified ProviderDataTransformer.php as // create a new media to avoid erasing other media or not ...
// $newMedia = $this->options['new_on_update'] ? new $this->class() : $media;
// $newMedia->setProviderName($media->getProviderName());
// $newMedia->setContext($media->getContext());
// $newMedia->setBinaryContent($binaryContent);
// if (!$newMedia->getProviderName() && $this->options['provider']) {
// $newMedia->setProviderName($this->options['provider']);
// }
//
// if (!$newMedia->getContext() && $this->options['context']) {
// $newMedia->setContext($this->options['context']);
// }
$provider = $this->pool->getProvider($media->getProviderName());
try {
$provider->transform($media);
} catch (\Exception $e) { // NEXT_MAJOR: When switching to PHP 7+, change this to \Throwable
// #1107 We must never throw an exception here.
// An exception here would prevent us to provide meaningful errors through the Form
// Error message inspired from Monolog\ErrorHandler
$this->logger->error(
sprintf('Caught Exception %s: "%s" at %s line %s', get_class($e), $e->getMessage(), $e->getFile(), $e->getLine()),
['exception' => $e]
);
}
return $media; This solves my problem now. At the same time the thumbnails are not deleted (This is a new problem). public function preRemove(MediaInterface $media) {
if ($this->requireThumbnails()) {
$this->thumbnail->delete($this, $media);
}
}
public function postUpdate(MediaInterface $media) {
if (!$media->getBinaryContent() instanceof \SplFileInfo) {
return;
}
// Delete the current file from the FS
$oldMedia = clone $media;
// if no previous reference is provided, it prevents
// Filesystem from trying to remove a directory
if ($media->getPreviousProviderReference() !== null) {
$oldMedia->setProviderReference($media->getPreviousProviderReference());
$path = $this->getReferenceImage($oldMedia);
if ($this->getFilesystem()->has($path)) {
$this->getFilesystem()->delete($path);
}
if ($this->requireThumbnails()) {
$this->thumbnail->delete($this, $media);
}
}
$this->fixBinaryContent($media);
$this->setFileContents($media);
$this->generateThumbnails($media);
$media->resetBinaryContent();
} There is already a problem regarding removing thumbnail in 2.x. But 3.x missing this line. |
A possible solution: removing the ProviderDataTransformer and add it with new options (e.g. new_on_update) to the FormBuilder. Working example: <?php
namespace ExtendedBundles\MediaBundle\Admin;
use Sonata\AdminBundle\Form\FormMapper;
use Sonata\MediaBundle\Admin\BaseMediaAdmin;
use Sonata\MediaBundle\Form\DataTransformer\ProviderDataTransformer;
use Symfony\Component\Form\DataTransformerInterface;
use Symfony\Component\Form\FormBuilderInterface;
/**
* Class MediaAdmin
*
* @package ExtendedBundles\MediaBundle\Admin
*/
class MediaAdmin extends BaseMediaAdmin
{
/**
* @inheritDoc
*/
protected function configureFormFields(FormMapper $formMapper)
{
parent::configureFormFields($formMapper);
$builder = $formMapper->getFormBuilder();
$transformers = $builder->getModelTransformers();
// Remove ProviderDataTransformer from array.
$transformers = array_filter(
$transformers,
function (DataTransformerInterface $transformer){
return ! $transformer instanceof ProviderDataTransformer;
}
);
// Remove all ModelTransformers and add filtered array of transformers.
$builder->resetModelTransformers();
array_walk(
$transformers,
function (DataTransformerInterface $transformer, $key, FormBuilderInterface $builder){
$builder->addModelTransformer($transformer, true);
},
$builder
);
// Add a new ProviderDataTransformer with customized options.
$builder->addModelTransformer(
new ProviderDataTransformer(
$this->pool,
$this->getClass(),
[ 'new_on_update' => false ]
),
true
);
}
} What do you think? Greetings |
you can reset your transformers in the media related admin method like so: public function configureFormFields(FormMapper $formMapper)
{
parent::configureFormFields($formMapper);
$formMapper->getFormBuilder()->resetModelTransformers();
$formMapper->getFormBuilder()->addModelTransformer(
new ProviderDataTransformer(
$this->pool,
$this->getClass(),
array(
'new_on_update' => false
)
),
true
);
} EDIT: Sure, you need to add your own transformers again. |
But that would reset/remove all other ModelTransformers, wouldn't it? |
Have same problem. After adding new ProviderDataTransformer with new_on_update false option, there is not deleting old files on update, like it was mentioned in #1273 (comment). Can we add some check before clearing previousProviderReference #1434? public function reverseTransform($media)
{
\\...
if ($this->options['new_on_update']) {
$newMedia->setProviderName($media->getProviderName());
$newMedia->setContext($media->getContext());
$newMedia->setBinaryContent($binaryContent);
}
\\...
} Then previousProviderReference will not be null, and will be removed. However, thumbnail is not removing. |
I experienced the same problem today after updating a OneToOne and replacing it in the Sonata Media form. |
same here, any new suggestions? in a ManyToOne relation... |
I have the following three entities
I was trying to edit the media from the property admin itself and facing the same issue. This also creates new thumbnails when the media image is replaced by a new on in the embedded admin form and all db entries are updated correctly. I also have the options symfony 4.3 |
Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
I am reopening this one. I can confirm a really strange behavior when you edit a media in the media admin (not in a relation through SonataMediaType). A new media is created, the old one is also updated... The fix adding the new_on_update seems to be working, but I have not checked all the cases. |
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
PHP version
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
As you can see an
INSERT INTO media__media
statement is executed, also anUPDATE 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 isnull
then set'new_on_update' => true
if not then set'new_on_update' => false
, and pass a new array argument this classProviderDataTransformer
This solution won't affect to
sonata_media_type
form type.Issues related
#974 but using form type: sonata_media_type
The text was updated successfully, but these errors were encountered: