Add 'flush' back into image module

Another need for image flush :

When you disable or re-enable a module that provides effects used in styles, images should be recomputed.

Such modules should be able to call a function in hook_enable() and hook_disable().

A kind of image_all_styles_flush, doing exactly the same job as image_style_flush but for all styles would work.

Here is a patch that adds a image_all_styles_flush to image.module.

This patch does not bring any visible change, it only makes this function available for other modules.

I am in need for that function in order to have IM_Raw module fully working on Drupal 7 (see #989520: ImageMagick raw action for Drupal 7).

I'm really missing this feature too. Sometimes styles seem to get stuck, re-saving a style doesn't cause a flush, switching to a different image style and back doesn't cause a flush. Just end up with broken styles.

subscribing ( it may also be useful when changing the quality of the generated images )

In case folks are desperate, there is a core drush 4 command that flushes.

sub

+1. New features go into latest core first though. And obviously, this patch needs work.

+1: this is a much missed feature!

My interim fix is to add the following to my themes template.php

$style = array();
$style["name"] = 'myStyleName';

image_style_flush($style);

Hope this helps ;)

Here's a patch (older p0 format, not the git p1 format) for 7.x that adds the flush operation back into the list of operations for image styles.

I'm sure this probably won't be rolled into the core module, but if anyone needs/wants this this should do the job for you.

Apply patch, then clear cache since this adds a menu path.

Updated path for files to be patched, hopefully this one will pass testing. The previous patch in #10 applied just fine if applied from within the modules/image directory.

This one should apply from the drupal docroot, which I think is what the testbot expects.

Changing back to 7.x-dev also for the testbot.

Ack! Apparently the testbot doesn't support p0 patches anymore. here's a p1 version of the same patch.

Changing issue back to D8.

I have another use case for this. I'm using hook_image_styles_alter() to change the default sizes of these presets for a project I'm working on. The problem is that if anyone views any of the resized images before this module is enabled (or if we decide to change the sizes later on) all the cached images are already created at the previous size, and since we didn't go through the UI to change the size, they don't get rebuilt.

The work-around is to "override" the default provided by the module and then "revert" back. But if we had a "refresh images" button that wouldn't be necessary.

Comments on the patch... (This looks like a patch for D7?)

I can't find the link or button that actually says 'refresh images' or 'update images' or 'resize all images'. I also see a lot of use of the word "Flush" so maybe it wasn't renamed, but I still can't see how to trigger it.

Patch against new core structure.

Also, it adds a 'flush' operation link on the image styles overview screen.

I was writing module that similar to that issue — imagestyleflush (http://drupal.org/sandbox/StepanKuzmin/1454096, applyed for full project at http://drupal.org/node/1454106).

Here is patch, that brings this module functionality to Drupal 7.x

Patches go to 8.x first.

This can be achieved by a contrib module like https://drupal.org/project/imagestyleflush. Now, in D8, all you need is to extend the ImageStyleListController and provide there the "flush" link. You need also to build a page callback and a route for image style flush.

I can't see a reason for why someone would/should have to install an additional contrib module to perform this operation.

OK, Sir! Then let's try with this patch :)

interdiff was left out as is not relevant because codebase has changed dramatically.

Oh! Forgot that translation manager is already injected and we have $this->t() (great!)

And also a test for new UI parts.

Ouch! Forgot an unneeded line inside.

And the image_menu() entry is senseless. We need only the route.

Tests are interfering.

As this turned green, reviews are welcomed.

@sun, can you provide a review on this? Thank you.

+++ b/core/modules/image/lib/Drupal/image/Form/ImageStyleFlushForm.php
@@ -0,0 +1,80 @@
+
+/**
+ * Base form controller for image style flush.
+ */
+class ImageStyleFlushForm extends ConfirmFormBase {
+

I think you can extend EntityConfirmFormBase, and avoid having to manage the ImageStyle entity directly.

+++ b/core/modules/image/lib/Drupal/image/Form/ImageStyleFlushForm.php
@@ -0,0 +1,80 @@
+  /**
+   * {@inheritdoc}
+   */
+  public function getDescription() {
+    return $this->t('This action cannot be undone. Image derivatives will be magically recreated as they are requested.');
+  }
+

When a beginner, I did not understand this. Can we explain a bit more, like "Image derivatives are the result of processing original images through the image styling. These derived images are stored in the file system to speed up further visits. Flushing them now will lead Drupal to recreate the derivatives as they are requested next time."

Also, I'd suggest to keep the "'This action cannot be undone." piece separate for translation. This one can be taken from parent::getDescription i.e.

+    return parent::getDescription() . ' ' . $this->t('Image derivatives will be magically recreated as they are requested.');

you can extend EntityConfirmFormBase [...]

Great catch. Thank you! Makes perfectly sense.

Tested manually.

This patch add a 'Flush' operation to the dropdown in each item on the list of image styles. Clicking on it you're taken to a confirmation form. Confirming the flush, the subdirectory of the 'styles' directory in public://, named after the image style being flushed, is removed from the file system. Clicking again on 'Edit' in the style list, the directory is recreated to produce the sample image on the style preview.

RTBC.

Needs a reroll for the yml file now that #2095121: Ensure every path in routing.yml files has a leading slash is in.

Oh, this was very fast! :)

The cancel route doesn't need parameters, we are returning to full list.

Oh

Back to RTBC

#43: flush-700696-43.patch queued for re-testing.

untag

+++ b/core/modules/image/lib/Drupal/image/Tests/ImageAdminStylesTest.php
@@ -347,6 +347,37 @@ function testEditEffect() {
+    $this->drupalGet($admin_path);
+    $flush_path = $admin_path . '/manage/' . $style_name . '/flush';
+    $this->assertLinkByHref($flush_path);
+
+    // Flush the image style derivatives using the user interface.
+    $this->drupalPostForm($flush_path, array(), t('Flush'));

This test would be better if you used clickLink to click on the flush operation on the admin form instead of asserting the href and then just posting the form.

Yes, I thought first this approach but clickLink() is taking the link label/text as argument. In that UI all flush labels are the same: t('Flush'). The installed profile brings some default image styles, so my test style is not the only one. There's a chance to flush other style. Handling that would get me far from the initial purpose of this test.

Reviewed this patch. I agree this should be part of core, and I think the code looks good. I've some feedback on UX and DX though.

1. +++ b/core/modules/image/lib/Drupal/image/Form/ImageStyleFlushForm.php
   @@ -0,0 +1,56 @@
   +  public function getQuestion() {
   +    return $this->t('Are you sure you want to flush image derivatives for %name image style?', array('%name' => $this->entity->label()));
   +  }
   

   Some of the UI language is a bit scary. For example, "flushing image derivatives for an image style" is very geeky. Can't we write something like "Are you sure you want to apply the updated %name image effect to all existing images?"?

2. +++ b/core/modules/image/lib/Drupal/image/Form/ImageStyleFlushForm.php
   @@ -0,0 +1,56 @@
   +  public function getDescription() {
   +    return parent::getDescription() . ' ' . $this->t('Image derivatives are the result of processing original images through the image styling. These derived images are stored in the file system to speed up further visits. Flushing them now will lead Drupal to recreate the derivatives as they are requested next time.');
   +  }
   

   This is super technical. For end-users it doesn't matter that Drupal stores cached copies of processed images. For example, I use Adobe Lightroom to process my photos. It's a tool for professional photographers that handles tens of thousands of photos. I'm sure it uses a ton of caching. However, at no point is that exposed in the UI. I'm not sure that explaining the inner workings of Drupal is useful for the majority of our users.

3. +++ b/core/modules/image/lib/Drupal/image/Form/ImageStyleFlushForm.php
   @@ -0,0 +1,56 @@
   +  /**
   +   * {@inheritdoc}
   +   */
   

   Outside the scope of this discussion but is there any reason these "empty" {@inheritdoc} comments can't be added by api.module? For me it really negatively affects the readability and DX experience of the code.

Agreed with the remarks about the descriptions.

1. Are you sure you want to apply the updated %name image effect to all images?
(I removed "existing", because we can't apply the effect to non-existing images :-)

2. I think the suggestion is to leave out this description entirely. Seems like the right idea.

@Dries, @yoroy, I agree partially with you. But "flushing" image cache is not an operation provided to regular users. If the site admin has access to this operation, at least he must get some information about what flush means for his site.

OK. Then "This action cannot be undone." is really scary while with this operation (flush) nothing gets lost. That's why I changed the description to not create any unnecessary worry for the user.

Comments from #46 to #50 have been addressed. Seems safe to set back to RTBC.

I strongly disagree with @Dries and @yoroy about removing the wording in #50.2. As @claudiu.cristea says, this functionality is aimed at site builders, not end users, so I am not worried about it being a tad techie. If I see "This action cannot be undone." I want to know what is involved! I think the explanation is #50.2 is both appropriate and useful. I too process photographs (although in GIMP not Adobe Lightroom) and, if I am about to commit an edit that will change my image, I want to preview it and know what I am doing before I press the [OK] button!

I like the changes in the latest patch, but I'm not sure about the new description. The description was changed from #49.2 to the following:

+++ b/core/modules/image/lib/Drupal/image/Form/ImageStyleFlushForm.php
@@ -0,0 +1,56 @@
+  public function getDescription() {
+    return $this->t('This operation is only refreshing the cache of %name image effect.', array('%name' => $this->entity->label()));
+  }

What are we expecting a site builder to do or take away from this information? What about the following:

"Drupal applies image style to a copy of the original image. This operation does not change the original image but will cause the copies to be recreated."

Assuming that information is correct, the explanation/description seems accessible, yet may provide a bit more explanation per Stuart's suggestion in #54. Thoughts?

@Dries,

The only thing that site admin needs to worry about when removing the image cache is that the site may expect some extra-loading when recreating image derivatives. This is the only real reason why we show him the confirmation form instead of performing the operation directly. Maybe we should add some words about that. I'm not a native English speaker, maybe someone else can add few words to explain this?

+1 to #55

In fact it's the explanation to a normal human being :) of #49.2 (which BTW was my input :))

OK

RTBC again.

That image styles are applied to copies of the original is already explained in the page help as you can see from the screenshot in #18, not sure it needs to be repeated here.

Two more issues with the latest wording:

- It uses 'Drupal', which we shouldn't do (it complicates distributions, see 'Wording' section in the ui guidelines here:https://drupal.org/node/604342)
- "Drupal applies image style" doesn't sound right, either "Applies image styles" or "Applies an image style".

I think you only need this:

"Flushing does not change the original image but the copies created for this style will be recreated."

As a general note, I disagree with the assumption that site builders are more technical and thus we can use more technical language. Everybody who installs Drupal (think of 1-click installs through hosting providers or Drupal Gardens) is a user/1 with access all areas. We should always strive to use the simplest possible words to describe only that which people need to make their next decision, regardless of assumed role or skill level.

Fewer, simpler words makes Drupal faster (don't make me think!) and more accessible.

Agree with the "Drupal" word removal.

I disagree with your final note. If you give somebody the right/permission to hit the "red button" then he has the right to know. If that user is able to "flush", at least he must be warned about side effects. E.g. (1) he will not lose images (2) copies will be recreated (3) he may experience site loading/slowness, etc.

Changes s/"the original image"/"the original images"

@yoroy, Can you re-RTBC please if this version is OK? As functionality it was RTBC several times.

Ok. From 260 to 100 characters in that description is a good thing :-)

Committed 3bcad72 and pushed to 8.x. Thanks!