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

Unwanted effect of "Remove obsolete media" in Media Library #2553

Open
jeroendesloovere opened this Issue May 30, 2018 · 7 comments

Comments

Projects
None yet
4 participants
@jeroendesloovere
Member

jeroendesloovere commented May 30, 2018

Type

  • Non critical bug

Problem description

Adding images in CKEditor adds an unwanted behavior.

Since a specific fork cms version, you can now add media library images into ckeditor.
But we have a button "remove obsolete media" in the media library module,
which deletes all MediaItem entities which do not have a "connection".
And since mediaItems (images, movies, ...) do not have any connection between ckeditor, ...
They are removed...

@ohvitorino

This comment has been minimized.

Contributor

ohvitorino commented May 30, 2018

@jeroendesloovere do you have any idea/suggestion on how to fix this?

@jeroendesloovere

This comment has been minimized.

Member

jeroendesloovere commented May 30, 2018

Temporary solution:

  • Disable the "MediaItemCleanup" action right
  • Disable the media_library:delete:items command
  • Hide the "# connections" table column, so you don't have to think about the "0 connections".
  • (Disable the "MediaItemDelete" action right)

Cleanest solution (but the most advanced one):

  • Shift to a modern way of working: Replace CKEditor by a "block system", where you can choose between text, textarea (= CKEditor, but without images/video/iframe/...), one image, media gallery, iframe, userTemplate, ...

... Can't seem to come up with something in between that is fairly easy ...
I know a solution that would be hard to do... On every editor save, notify the media library that an image/video is being used... The hard thing to fix are then the "archived pages" & "hidden page block"

[Update] Another solution
I do found a better way... Read a little bit further.

@StijnVrolijk

This comment has been minimized.

Contributor

StijnVrolijk commented May 30, 2018

It a bit of a long shot and quite inefficient but if we stick to CKEditor we could use a scraper to scrape the website for images and thus determine the obsoleteness of the images in the library.

@jeroendesloovere

This comment has been minimized.

Member

jeroendesloovere commented May 30, 2018

@StijnVrolijk Good suggestion, but unfortunately that won't do it. Since you can have custom modules that have a $publishOn \DateTime field...

@StijnVrolijk

This comment has been minimized.

Contributor

StijnVrolijk commented May 30, 2018

Ahh yes, good point

@jeroendesloovere

This comment has been minimized.

Member

jeroendesloovere commented May 30, 2018

I have a "fairly good solution" for this "nasty problem".

Steps to create the fix:

  • We create internal search queries for all modules (pages, content blocks, blog, ...), which actually should only return the column where the editor content is in.
  • I would call them EditorMediaItemScrapers. Which will be executed by using a new service called EditorMediaItemScraperManager .
    • EditorMediaItemScraperManager->addProvider(ModuleEditorMediaItemScraperInterface $provider); (can be done automatically by using symfony tags)
      • PagesEditorMediaItemScraper
      • ContentBlocksEditorMediaItemScraper
      • BlogEditorMediaItemScraper
      • ...
    • These providers will create one "MediaGroup" per provider and connects all the "used" MediaItem entities to this MediaGroup.
  • When clicking "Remove obsolete media", the service is being executed, the "number of connections" is being updated and then the media items with 0 connections can be removed safely.

Bingo. Problem solved.

@carakas

This comment has been minimized.

Member

carakas commented May 30, 2018

Bingo? this is an even bigger mess. Just add a warning of what the consequences might be and remove it in Fork 6
the moment somebody doesn't know about this service to clean up the mess it won't work anymore.

@carakas carakas added this to the 6.0.0 milestone Aug 7, 2018

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