"File path" is not kept across an export/import

Please provide brief step-by-step instructions on what you are doing so that I can attempt to reproduce and confirm the issue.

Closed due to lack of response.

To reproduce:

1. Set the "File path" configuration option in the "FileField Path settings" section of a field of type "file".
2. Export the CCK content type to which it belongs (including all of its fields).
3. Import it into another Drupal site.
4. Surf to the Configure form the imported file field.
5. The "File path" field is now blank.

I would expect that the "File path" would be carried over, and not be reset.

Note: Similar settings retention issue at #849778: Filefield Paths settings not carried over when adding existing field to new content type.

The path is stored in a table and no information about the path is added to the export, thus nothing gets done when you try to import it somewhere else. This also affects the ability to use filefield paths in features, since there is no way to export the value into a feature.

Currently there is no way to do this other than manually writing custom code to update the filefield_paths table with the new information.

There are a few separate issues about CCK exports being broken when using this module:

- #814328: How can i export the filefield paths configuration to other site?
- #849778: Filefield Paths settings not carried over when adding existing field to new content type

I'm going to mark those as duplicates of this issue.

I propose implementing ctools export.inc to add exportability coupled with Features integration. Possible downsides of this are a dependency on ctools, and CCK exports done outside of Features still being broken. We may be able to create the patch in a way that it doesn't require export.inc for the module to still function as is.

Thoughts?

Tagging issue as well.

Ok, here's my first take at a patch. This definitely needs lots of testing, b/c there are some architectural changes going on here.

Whoopsie.. Take 2 that doesn't remove one of the translation files. ;)

Hi q0rban,

Thanks for getting involved, I do really appreciate it.

However, the first thing I am going to say may or may not blow away your desire to continue to provide your support (and I really hope it doesn't): CTools dependency is out of the question.

It's nothing personal to CTools itself, I have high great respect for merlinofchaos, he is a far greater developer than I could ever be and I'm sure the code in CTools is some of the best code on the Drupal scene. I do have an issue with adding another barrier to users by adding unnecessary dependencies.

I know this complicates matters, and is a royal pain for you or anyone else trying to supply this functionality.

My quick thoughts on FileField Paths exportables, after a quick read of this thread last night, on how to bring exportables to FileField Paths are:
- Create a modules/features.inc for all the code (this is just my standard process).
- Use Features hooks to simply dump at FileField Paths columns as an array as required.
- Use 'hook_features_populate_alter()' #693024: Add alter hooks for the "faux" exportables to detect when exported Content Types are using FileField Paths settings and add dependency on FileField Paths and exportable object. (And hope that this hook makes it in, because it is greatly needed).
- Use one of the non-exportable hooks from Features to re-add the data back into the database on installation of the Feature.

Take into account that this is just off the top of my head with only a few minutes thought, but the concept should work and it shouldn't take more than 3 or 4 hooks with minimal code to be achieved.

Happy to discuss alternatives, and I do still intend to look through your patches more thoroughly, but I really can't think of a valid reason to add CTools as a dependency as 99% of the users are unlikely to ever need to export the settings (huge assumption).

Cheers,
Deciphered.

@q0rban has marked as a duplicate of this issue #849778: Filefield Paths settings not carried over when adding existing field to new content type:

When you add an existing field to another content type on the CCK "Manage fields" screen, the CCK settings associated with that field are remembered and carried over as the default settings for the new instance of the field. But the Filefield Paths settings are not—those are lost. I assume that's not the desired behavior.

Let's make sure it gets addressed as well.

What if CTools were optional, rather than a dependency?

@#12, Whoopsi! I think I misread that when I marked it as duplicate. I think that is indeed a separate issue. :)

@colan

Optional requirements aren't really an option as it's just as confusing.
In my personal opinion, Features should supply the hooks to define exportables as it is the module that requires the exportables, or it should require CTools.

As this is not the case, I don't think CTools exportables are the way to go, but as I mentioned above it should be just as easy to allow exportable FFP settings without CTools.

I will turn attention back to FFP as soon as I have finished up the next stage of development on a couple of my other modules, but I am more than happy to work with anyone who wishes to help out, especially those who provide patches.

Cheers,
Deciphered.

Here's an updated patch that only depends on ctools for exportability. I *REALLY* think we should stick with ctools for this because it seriously saves us hundres of lines of code, not to mention that we then have the features module maintaining the ctools integration. Features is still somewhat in the 'wild wild west' phase, so this saves the maintainers of this module from having to keep up with any changes that may occur there.

Plus, all the cool kids are doing it! Seriously though, pretty much every DevSeed module that now defines it's features exportability by using cTools export.inc. Context 3 has moved to ctools, Strongarm 2 is ctools, Feeds, the list goes on and on. I think we can feel pretty good about requiring ctools for any exportability.

Whoops, found a typo in a defined variable.

Features does not require CTools but there are many things you can't do without it so for all practical purposes you need it. This is the standard method of creating Features exportables, it makes sense to do things the same way all other Features modules do rather than inventing a new way of doing things. It will also make the code easier for others to help maintain if we do it the 'standard' way in case the Features API changes in the future.

I think it makes sense to make it optional for people who don't want to use Features -- if CTools is available there is a Features exportable, otherwise there is not. So long as nothing is broken if CTools is disabled, it should have no ill effects on anyone. A note on the project page can mention that Features exports are provided if CTools is installed. Anyone who uses Features will have no trouble understanding that.

I will consider this argument, and try to make some time to review the patch, but I do have to take exception to at least one thing, you say that this approach saves hundreds of lines of code, whereas I still stick by the fact that this functionality can be done with a few small functions without altering the database with a lot less code than these patches are using.

The concept is really quite simple as these are simply settings, not true exportable objects. Simply dump out the data on Features export and re-import in on Feature activation. Not really much to it at all.

The only place it does become an issue is when we aren't talking features exporting, we're talking standard CCK export/import (as this issue was started on), and I don't believe that either of these approaches take that into account.

I know it may look like I'm trying to take the easiest way out, and maybe I am, but my plan has always been that when I did have time to turn back to FFP it would be a complete rewrite leading to version 2.0, in which I would be more agreeable to such drastic changes.

I will try to catch you (q0rbon) on IRC to chat further, but as discussed via email, there is definitely a timezone clash.

Cheers,
Deciphered.

subscribing

subscribing

Sub.

subscribe

Subscribe.

The patch in #17 is working for me :)

The ctools/features support patch is good, but is there any reason the settings for this module can't be saved in say...content_field_instance instead of a separate table? I think that is probably the ultimate solution (exportable by features and cck and no dependencies).

FYI, use the patch in #17 with caution, as I found a find/replace error in it after the fact but did not have the time to fix:

-  $ret[] = update_sql("DELETE FROM {variable} WHERE name LIKE 'filefield_paths_%'");
+  $ret[] = update_sql("DELIMITER FROM {variable} WHERE name LIKE 'filefield_paths_%'");

Whoops! :( Someone who has the time should walk through this with a fine-toothed comb and make sure there aren't other find/replace mistakes like this and re-roll the patch.

Just a quick note, this is now an issue for me (yes, that is how it works) so it will definitely be fixed.

But do please be patient, I don't have a lot of time to work on Drupal lately.

Cheers,
Deciphered.

FileField Paths 2.x (currently in development) is going to drop support for all non-CCK fields, which in turn allows it to take advantage of hook_widget_settings_alter() and have CCK handle all of the FileField Paths settings, which in turn means that exporting the CCK field (both using CCKs Export and Features Export) will export the data without any extra work involved.

Apologies to q0rban who put a lot of great work into this patch only to meet resistance, I do appreciate the effort, I just needed to decide exactly how I planned to approach this issue first, and unfortunately the end result was incompatible with the existing work you did.

Expect a dev/alpha/beta release by the end of the week (hopefully), as so far the re-write has gone extremely smoothly.

Cheers,
Deciphered.

That sounds *fantastic*!

Tagging

Sub. (Now I see what you meant by "dropping support for non-CCK modules". Cool.)

Subscribing.

Subscribing, glad to see this moving forward. :)

Subscribing

~

Here is support for Features import/export with no dependancy on ctools.
This against DRUPAL-6--1-4. ALL NEW CODE IN ITS OWN INC FILE - so no conflicts are going to happen. Should apply to -dev also if you want to try.

I did it while working through a deployment Features package where I wanted to add filefield_paths support to my CCKs.
It will attach itself to any features export when you bundle a content type or an individual field.
You can also do it on its own, but that's pretty pointless out of context.

Done without reference to the earlier ctools version :-}, I just scratched an itch as practice getting ready to deploy whole sites as features, so I jumped in before I came looking to see if it had been attempted.
However, I also feel a bit icky about ctools - I haven't quite got its idea or API, but Features is really happening.
On review of #17 I see a lot of similarity, though it's using different API.

Does NOT address the discussed desire to get it into the base CCK export itself. Not sure where that should/could happen.

If you are still not at all interested, that's fine, it was just a fun little project for myself, but I can't see the discussed 2.x dev from September to compare with. It can even be grabbed and run stand-alone if anyone needs it.

Just for reference, you can find the FFP 2.x branch at http://drupal.org/node/937890

I may yet consider committing this patch in the short term, because I also equally agree that this is a necessity.

Thanks dman, patch in #36 works nicely.

Hallo,

I'm currently using filefield_paths on 27 cloned sites, which I manage by deploying custom scripts with drush. Since I'm not using 'features' the patch above was useless to me, but I found a (very rough) way to copy ffp settings when exporting/importing content types.

Basically, to copy a content type from a 'mother' site to its clones, it works like this:

<?php

// Paste content type from mommy, dollar signs have to be escaped.

$content_1 = <<<EOD
\$content['type']  = array (      
  'name' => 'my_content_type',
  'type' => 'my_content_type',
 
[...]

);
EOD;

// Import it by faking a form submit.

$form_state = array();

$form_state['values']['type_name'] = '<create>';
$form_state['values']['macro'] = $content_1;

drupal_execute('content_copy_import_form', $form_state);

unset($form_state);

// Following strings are from mommy's filefield_paths.filename and filepath tables, 
// pasted directly from mysqladmin web interface.

$my_field_name_filename = <<<EOD
a:4:{s:5:"value";s:60:"[filefield-onlyname-original].[filefield-extension-original]";s:7:"tolower";i:1;s:8:"pathauto";i:0;s:13:"transliterate";i:1;}
EOD;
$my_field_name_filepath = <<<EOD
a:4:{s:5:"value";s:20:"path/needed/to/have/a/nice/tidy/filesystem";s:7:"tolower";i:0;s:8:"pathauto";i:0;s:13:"transliterate";i:1;}
EOD;

// Now, entries in 'filefield_paths' database table DO NOT exist yet... so I INSERT'em ( and not UPDATE 'em)
// quite brutally, by the way.

db_query("INSERT INTO {filefield_paths} VALUES ('my_content_type', 'my_field_name', '%s', '%s')", $my_field_name_filename, $my_field_name_filepath );

?>

I tried a solution using filefield_paths.module functions, but I failed.
Now, this code seems to work on my test site, but I don't know whether there are other db fields (e.g, variable?) which should be changed as well, so any hint would be greatly appreciated, before I make a mess.

Regards

L.F.

Module settings being exportable is something that I wish every module would consider in its design. Features are impressive however not everything is supported and I want to use this on programatic ubercart products in an install profile. So I have no choice to write a new function and insert to the db like lucio.ferrari.

This might help someone not wanting to copy the serialized arrays. I guess the other thing that maybe helpfull would be calling;

filefield_paths_batch_update ( 'filefield' , $field_name , arg ( 3 ) );

function add_filefield_paths ( $type, $field_name )
{

    $filename_settings = array (
        'value' => "[tokens].[filefield-extension-original]" ,
        'tolower' => 0 ,
        'pathauto' => 0 ,
        'transliterate' => 0
    );
    $my_field_name_filename = serialize ( $filename_settings );

    $filepath_settings = array (
        'value' => "test/path/[tokens]" ,
        'tolower' => 0 ,
        'pathauto' => 0 ,
        'transliterate' => 0
    );
    $my_field_name_filepath = serialize ( $filepath_settings );

    db_query ( "INSERT INTO {filefield_paths} (type,field,filename,filepath) VALUES ( '%s', '%s', '%s', '%s')" ,
               $type , $field_name , $my_field_name_filename , $my_field_name_filepath );

}

Would also like to know if there is something lucio.ferrari and I have overlooked?

Cheers :)

That's much more elegant, my kudos. :-)

subscribing, I just installed the 2.x-dev and my initial tests look good :)

#36 works for me; would be welcomed in a new release -- I'm not too ninja enough to RTBC it, though.

#36 works for me; would be welcomed in a new release. I'm also no ninja, but I guess two no-ninjas can replace a ninja. :-)

Will test it out shortly, and if it works I will try to get it into a release as I my schedule won't allow for a proper re-write for at least another few months and I also need this functionality.

The patch looks real good, and works here too.

RE the patch - I believe (and intend) that the sub-functions labelled as CRUD could be merged back into the module proper, for better architecture. Not strictly necessary, but a small improvement IMO.

The hook_features_pipe_component_alter(&$pipe, $data, $export) {} doesn't need $module_name. It could make the same error like this.
I've rerolled the patch against latest remotes/origin/6.x-1.x, and delete the unusable CVS keyword from new file.

and attach

The first CVS patch cosed some whitespace error. Fixed (I hope..)

@szantog

Gave it a quick test, findings are as such so far:

- All functions should be in 'modules/features.inc', which is loaded when the features module is present (see filefield_paths_init()).
- filefield_paths_features_api() shouldn't need to declare the 'file' argument as the file will always be included if you do the above.
- 'Active updating' setting doesn't get exported.

Haven't finished testing yet, but in general it looks like it's on the right direction.

Ok, I just simple apply the patch previously reviewed. :)
Now, i made some changes:
1. As @dman in #47, the CRUD was moved in .module
2. Move .inc to modules/
3. Remove file argument from _features_api()
4. Add ffp_[type]_[field] variables to export 'Active updating'.
5. Add dependencies transliteration and pathauto, if the settings need it.

@szantog @dman,

I personally disagree with the placement of the CRUD functions in the .module file, firstly because it just adds extra clutter to the .module file, where my style is about grouping specific functionality together, not sticking it all in one place, and secondly because the functions are only required for the Features functionality.

Maybe in the future, during the development of FileField Paths 2.x it would make sense to make the functions available to the module as a whole, but for the simple purposes of adding Features support to the 1.x branch I would prefer to keep them in the features.inc file.

Other quick feedback, the 'hook_features_api' function should be in the features.inc file as well, everything Features should be in this file, the only file in this patch should be the features.inc file.

Haven't tested the actual functionality, but will do so as soon as I post this feedback. In the case that it is working I will re-arrange the code myself and commit this, otherwise I will let you know any further feedback.

Edit: Further feedback.

- in 'filefield_paths_item_features_export', what is the purpose of $variable_map = features_get_default_map('variable');? $variable_map doesn't get reference anywhere else.

- Still isn't exporting 'Active updating'

Committed.

Thanks for the patience guys.

groovy

Yees. First I missed the ffp_[type]_[field] variable, and I knew, the features_get_default_map('variable') makes the strongarm api declaration. This is real unnecesary. That's a new patch.

- Still isn't exporting 'Active updating'
?? I created a new content type, then in create feature page I only choose the new content type, and every settings was added. I moved it to another site, and all settings was imported include the Active updating. I did it with existing feature by another content type, changed status of Active updating was moved successful.

Hmm And what about this issue? I see you edit your comment, but I don't know, before or after commit? :)

Yes, I did discover the problem before I committed, or else I wouldn't have committed. It's simply that the 'Active Updating' option requires 'Strongarm' to be present. I'm not entirely thrilled with that, but I can live with it as it's just for the one option.

Thanks for your great work.

Looks like this still needs to be ported to Drupal 7.

I've ported features.inc to drupal 7 and it seems to work in my testing.

It probably happened in the earlier roll-in, But I notice a search & replace has mangled the original comment :

+ * The 'component' for this module is named 'filefield_paths' not just
+ * 'filefield_paths' to follow recommended practice documented in features.api

... so, that line should be removed altogether :-)

@mfb can you roll the patch against 7.x-1.x and can someone mark as RTBC when you've tested it?

I believe Deciphered isn't too keen on this patch, but I'm happy to commit it once it's RTBC because:
a) The functionality is needed
b) When Deciphered finishes 6.x-2.x we can port that directly to 7.x-2.x making any changes to this branch irrelevant.

@aidanlis,

Actually, I did commit the 6.x version, after it was re-written to suit my coding style, so as long as it's a port of that I have no major concerns it being committed to 7.x-1.x, assuming it's well tested and is RTBC.

Cheers,
Deciphered.

@Deciphered Ah, awesome :) I'll check it before it goes in.

There is a problem with the field identifier in D7. I'd suggest changing the schema for filefield_paths to use entity_type, bundle and field_name as primary key, and then using the combination of all three as field identifier which is what features module expects.

OK I think this is enough to resolve the actual problem: I added a hard-coded entity_type to the $key definition: "node-{$row->type}-{$row->field}". See also #1155184: FileField Paths should support comments (and any other entities) with file fields for a proposal to support other entity types.

Patch from #36 works for me. Thanks!

Fixed and committed to 7.x-1.x.

The fix for 6.x-1.x is now officially available in version 6.x-1.5.