Download & Extend
File (Field) Paths » Issues

Add support for bulk rebuilds of derivative files in Image module

Posted by sp3boy on August 31, 2009 at 7:50pm
Project:File (Field) Paths
Version:6.x-1.x-dev
Component:Code
Category:feature request
Priority:normal
Assigned:Unassigned
Status:closed (fixed)

Issue Summary

I have been testing Filefield_paths with Image (as an alternative to the enhancement to Image proposed in #103793: Add token support for image file directory and image names). The attached patch addresses three areas that were preventing me from making full use of Filefield_paths with Image:

  • The admin form field was not long enough for my list of tokens which I wanted to build the Image file path from.
  • If I used a taxonomy token but my node had no taxonomy set, a path was created with '//'.
  • When I used the Update option "Rebuild derivative images" in the admin/content/node form, the selected nodes were not processed to apply the new token values.

Note: I think the second point is not the same as a recent issue regarding double slashes appearing when individual terms in a hierarchy were not set.

I have tested the patch against a CVS export of DRUPAL-6--1. I hope that's the most appropriate code version. I had some minor problems applying the patch against the 6.x-1.3 tar files, I think because one set of files is created with Windows line terminators and the other without?

Note: there is a complimentary patch against Image for a couple of small changes there.

Login or register to post comments

Comments

#1

Posted by sp3boy on August 31, 2009 at 8:07pm

Hmm... didn't realise I would not be able to edit the original post to add the attachment. So here it is. The related issue in Image is #564690: Improve support for Filefield_paths to tokenise image filename and path.

AttachmentSize
filefield_paths-564680-1.patch 2.33 KB

#2

Posted by sp3boy on September 3, 2009 at 3:49pm

Additional testing has highlighted a problem with the "double slash" conversion - it should have been converting any number of consecutive slashes into a single one but wasn't, causing image files to be lost during rebuild of derivatives. Revised patch attached.

AttachmentSize
filefield_paths-564680-2.patch 2.33 KB

#3

Posted by Deciphered on September 3, 2009 at 11:56pm

Hi sp3boy,

Sorry I've been so absent of lately but I've had a lot going on.

As I don't like committing code without being 100% sure that it's the best way to fix an issue, can you provide me with a brief step-by-step on how to reproduce each of the three issues so that I can confirm the fix?

Cheers,
Deciphered.

#4

Posted by sp3boy on September 4, 2009 at 3:40pm

I quite understand and would be more worried if you were happy to commit the patch unquestioned!

The first issue is not so easy to replicate in practice unless you have a site with complex multi-vocabulary taxonomy and fairly elaborate path requirements (which I have, hence the default 128 field size was not enough!). The simplest way I've found to illustrate the physical limit on that field is to copy "1234567890" into the clipboard and try pasting it into the field 13 times. It should refuse to accept more than 8 characters of the 13th paste.

For the other two, here's my best shot at a minimal test. By the nature of what I'm trying to achieve on my site, there are various components:

Set-up

  1. Install fresh D6.13 with token-6.x-1.12 and CVS exports of Filefield_paths DRUPAL-6--1 and Image HEAD.
  2. Install the "Token for Vocabularies" module in #185446: Provide tokens for each vocabulary. I know it is not yet even in CVS and is in a postponed state but it is a simple solution and matches my requirements 100% :)
  3. To test the necessity for parts of this patch, the Image module should be patched with the patch from #564690: Improve support for Filefield_paths to tokenise image filename and path - sorry about that but the two patches are mutually-dependent.
  4. Enable modules Filefield_paths, Image, Token, Token for Vocabularies.
  5. Create two Taxonomy vocabularies, e.g. "Tax-1" and "Tax-2", assigning both to the Image content type but leaving them optional.
  6. In Image Settings, Image Path Settings, you should be able to set the filepath field to "images/[vocab-tax-1-term-raw]/[vocab-tax-2-term-raw]", which uses tokens from "Token for Vocabularies".

Testing pre-patch

  1. Create an Image node with Tax-2 set but Tax-1 unspecified. If you look at the properties of the image displayed on the resulting node view, it should have a double slash where the Tax-1 term would otherwise appear.
  2. Edit the Image node and set a Tax-1 term. Select the "Rebuild derivative images" checkbox. Save the node. The derivative images will be physically rebuilt (you can try deleting one from the filesystem immediately before clicking Save) but their path will not be changed to reflect the new Tax-1 term value.

Testing post-patch

  1. Apply the above patch and create another Image with only Tax-2 set. The image path should not have a double slash.
  2. Edit the second image and set a value for Tax-1. Select the "Rebuild derivative images" checkbox. Save the node. The derivative images will be rebuilt and their path will be changed to reflect the new Tax-1 term value. (This is the functionality that is co-dependant on the patch to Image).
  3. A two-step version of this test is to edit an image, change its taxonomy, leave the "Rebuild derivative images" checkbox unticked and then when the node is saved, use the admin/content/node form to select the node, and perform the Rebuild derivative images update on it. This will not apply taxonomy changes to the image file path unless the above patch for filefield_paths is applied.

I hope that provides adequate information.

Thanks and regards.

#5

Posted by sp3boy on September 10, 2009 at 7:40pm

Further to my first patch, I have found another couple of issues which are additionally addressed in this revised patch:

  • When I ran the Retro-active update on (for example) Page nodes with attached files, the image_filefield_paths_update() hook was firing the filefield_paths_nodeapi() to process each file a second time, even though the node was not an Image. This was generating errors on moving the files because they had already been moved by the action of the upload_filefield_paths_update(). I have put in a test for the $node->images being set.
  • I noticed that the "origname" column from {files} is not automatically loaded to an Image node (upload_load() picks it up as it does a "SELECT *" from {files}). This prevents retro-active changes from being able to work from the original name and give an entirely predictable result. I have added this data capture to image_filefield_paths_get_fields().

BTW the "double-spaced" appearance of the patch is due I think to the CVS copies of the module files having Windows line terminators (CR + LF) which is confusing my CVSNT software somewhat (it wants to convert them to CR + CR + LF).

AttachmentSize
filefield_paths-564680-5.patch 3.7 KB

#6

Posted by Deciphered on September 10, 2009 at 10:18pm

Patch looks good to me,

Will apply it shortly and give it a run through.

Cheers,
Deciphered.

#7

Posted by sp3boy on September 16, 2009 at 6:22pm

After a lot more testing I have no changes to this patch but quite a few to the corresponding patch against Image. Please see #564690: Improve support for Filefield_paths to tokenise image filename and path for the latest.

#8

Posted by Deciphered on September 17, 2009 at 4:57am

I did make a change to the patch, but only very minor.
I will do my best to get this committed ASAP.

#9

Posted by Deciphered on October 28, 2009 at 3:04am
Status:needs review» fixed

Committed to DRUPAL-6--1.

#10

Posted by System Message on November 11, 2009 at 3:10am
Status:fixed» closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.