Node settings change not triggering "overridden" flag

Y, this is part of a larger issue of trying to capture variable settings in a reasonable way...

The workflow settings should be straightforward but node type variables tacked on through form_alter() might need some more forensic science / backwards-engineering.

If it helps, you can simply manually fire off the form preparation... I did this for some code in Ubercart where I wanted to display a summary of a form that took alterations into consideration:

  $form_state = array('storage' => NULL, 'submitted' => FALSE);
  $form = drupal_retrieve_form($form_id, $form_state);
  drupal_prepare_form($form_id, $form, $form_state);

The $form array would contain any form altered elements after this. Then you can iterate through the form structure to find all the variables. Still not an easy task for sure. : D

Here's a patch that adds the Node Type Variables to the Node Export function, as well as adds $type->help, min_word_count which were missing.

I think I came up with a reliable solution to the Node Type Variable problem: Do it the way core does it.

As you can see from http://api.drupal.org/api/function/node_type_form_submit/6 , The Node Type edit form grabs every Core nodetype field, then "whatever's left is assumed to be a persistent variable.". After which it calls variable_set() on each additional form_value, the name being the key of the $form_state['values'] array item plus $node->type appended to it!

So, this simply adds an additional item to the hook_default_types array: variables.

Reverting is just looping through $node_type->variables and calling variable_set();

It works Swimmingly. Change the comments settings, and The "Overridden" flag is set.

so you end up with exported code like this:

$items = array(
    'blog' => array(
      'name' => t('Blog'),
      'module' => 'node',
      'description' => t(' '),
      'has_title' => '1',
      'title_label' => t('Title'),
      'has_body' => '0',
      'body_label' => t(''),
      'help' => t(''),
      'min_word_count' => '0',
      'variables' =>  array(
  'comment_blog' => '0',
  'comment_default_mode_blog' => '4',
  'comment_default_order_blog' => '1',
  'comment_default_per_page_blog' => '50',
     )
);

I'm not happy about abusing hook_node_info() with information that it's not supposed to contain. In addition, I've found that in practice node type variables need a bit more micromanagement than features' automated code exporting can provide -- there are some content type variable values that you want to export while others you may want to omit because you had them set specifically for your use case but may not be appropriate for a general-purpose feature.

If this functionality is important to your needs, you may want to split it out into its own hook (hook_node_default_settings or so?) and add features integration for it in its own module or include.

Yeah, that makes sense. I'll look into creating up a hook for that... I also had an idea of how to incorporate variables in a feature beyond hook_strongarm(), so that changes to system settings forms can be made, updated, or reverted.

I think I'll look into integrating the nodetype settings into that instead.

There were two node settings that are a part of core that still need to be added to features.node.inc, 'help' and 'min_word_count'.

I'll re-roll a patch for that if you'd like.

Thanks!

subscribing

There were two node settings that are a part of core that still need to be added to features.node.inc, 'help' and 'min_word_count'.

I'll re-roll a patch for that if you'd like.

I attached a small patch that will "only" add help and min_word_count. As it is a core setting this should be implemented in features.
It is diffed against the current BETA3.

I'd like to see this expanded to include the setting from the theme global settings page to display the "posted by" information for the node type as well... it's another core setting that would be excellent to preserve in Features.

Is there any reason why to not use/implement cck's content_copy import/export functionality for node type settings?

( http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/cck/modules... )

@careernerd - did you ever share a new module or include for comment settings, fivestar, workflow, etc?

I've committed derhasi's patch in #7 (http://drupal.org/cvs?commit=269174).

As for the larger issue of managing content type settings stored as variables I think an ideal implementation would leverage a more generalized variable override/management API (e.g. something like strongarm) even if it implements variable export only for the subset of variables exposed on the content type form.

It's also worth recognizing that certain variables store node-type related settings in a non-granular/module fashion (e.g. an array like array('blog' => 1, 'book' => 0, ...)) that would require additional management to split up across features. The theme_settings variable (which stores post display information per node type) is an example of this.

So do you think we should move the Variable defaults/override functionality to features.module itself? Let me know, I'm going to tackle this soon.

Strongarm is good in concept, but I think it should allow modification of variables with a warning and overridden flag, just like nodes, fields, and views work now.

As for the non-granular settings, I see the problem... If we're gonna have any kind of generic variable defaults system I'd have to say all we can do is document this problem and try to let feature-builders and module devs know that these settings types pose a problem. If certain modules that are useful enough have variables like this, we can try to patch them so that they store the vars the better way.

@careernerd: It would be great if you could look at the existing strongarm "override" mode implementation (you can put strongarm in this mode on admin/settings/strongarm) and see if it makes sense to expand off of that.

Another issue that was brought up is that variable exports truly pollute the info files of features. I wonder if it makes sense for there to be an intermediate structure, say a "variable set" that allows you to define bundles of variables and their default values and export those together with a single ID?

So, given that hook_node_info() in D6 is not sufficient for managing content types because it does not allow for integration with other modules, could content type creation just be handled through an install script -type process that executes the CCK APIs directly?

subscribing

There are a good deal of content type settings that are *not* just variables; given that CCK bundles Content_Copy and thus gives everyone a simple way of importing & exporting content types, I don't see any reason why this would not be used? I'll dig into it.

Related to this, should the hook_node_info() set the content types to be locked?

Digging through the D6 code.. the node types are compiled in node_types_rebuild() which:

• calls _node_types_build() to compile all of the responses from hook_node_info(); this initial pass doesn't actually save, update or even temporarily store anything, it seems like a superfluous use of the function?
• calls node_get_types($op=>'types', $node=>NULL, $reset=>TRUE) which does the heavy lifting - it loads everything from _node_types_build() again (the $reset=>TRUE flag) but this time loads all of the data and returns the array of content types.
• calls node_type_save() on all new content types and passes all of the data loaded by hook_node_info(). Note: at this point it passes through everything from hook_node_info(), not just the predefined values.
• node_type_save() then calls hook_node_type($op=>'insert', $info) to allow all other modules to do what they need.

As you can see above, hook_node_info() passes along everything that is loaded right through to the final hook_node_type().

This means that it is perfectly OK to load up hook_node_info() with whatever additional data we want to use. :-)

To continue the brainstorming some more...

What we could do is:

• In node_features_export_render() compile something similar to the $content['type'] variable from content_copy exports, and add a key that says e.g. 'features_module'=>TRUE.
• Add a new function, features_node_type(), that on $op=='import' parses the values.
• Because we should rely on other modules handling their own data, features_node_type() should only deal with settings for core Drupal modules.
• All other contrib modules could work off the 'features_module'=>TRUE value in their hook_node_type() call to store their respective data as needed. Given how popular Features is this shouldn't take too long to get lots of support.

This would be a cleaner way than just arbitrarily storing everything as a variable and also not only rely upon the existing hook_node_type() functionality but also extend it slightly by adding the 'features_module' value so modules can react accordingly.

Thoughts?

Here's a new hook to allow other modules to extend the data added to hook_node_info() to match some of my thoughts above, API documentation provided; FYI I also added a new function features_export_render_array() which is used to simplify the logic in the main node_features_export_render() function.

Here's the hook documentation for your edification:

hook_features_export_node_type($info)
-------------------------------------
This hook is provided to allow modules to extend the standard hook_node_info()
data structures, which can then be reimported when the feature is enabled
using hook_node_type($op='insert').  The return array should contain
everything needed to rebuild the module's settings as if it had been manually
created through other APIs or the standard admin interface.

Attributes:

  'info': an object containing all of the base settings generated by
    node_get_types('type', $type).  The Features module pre-assigns the
    $info->module attribute as 'features', which can be used to identify that
    this is being enabled using the Features module.

Modules are responsible for reimporting this data during the subsequent
hook_node_type($op='insert', $info) function call when the feature is enabled.

The returned array from each hook implementation will be added to the end of
the normal hook_node_info() array and keyed off the module's name, e.g. given
the following example module code:

/**
 * Implementation of hook_features_export_node_type().
 */
function mymodule_features_export_node_type($info) {
  return array(
    'key1' => array(
      'key2' => array(
        'key3' => 'bar',
      ),
    ),
    'key4' => 'val',
  );
}

the generated output will be:

function _mymodule_node_info() {
  $items = array(
    'contenttype' => array(
      // ... basic fields ...
      'mymodule' => array(
        'key1' => array(
          'key2' => array(
            'key3' => 'bar',
          ),
        ),
        'key4' => 'val',
      ),
      // ... other data returned from the hook ...
    ),
  );
  return $items;
}

Any feedback?

FYI the patch is untested :o)

Small fix to the patch, it wasn't closing the original node_info arrays.

If this is at all helpful, my workaround is to add a .install file to the features module directly that implements a hook_install() and changes variables using variable_set()

function features_blog_install() {
  variable_set('node_options_blog_entry', array('status', 'revision'));
  drupal_set_message('[Blog content type] <em>Workflow: Default options</em> set to <em>Published</em> and <em>Create new revision</em>', 'status');

  variable_set('comment_default_order_blog_entry', 2);
  drupal_set_message('[Blog content type] <em>Comment: Default display order</em> option set to <em>Date - oldest first</em>', 'status');

  variable_set('comment_subject_field_blog_entry', 0);
  drupal_set_message('[Blog content type] <em>Comment: Comment subject field</em> option set to <em>Disabled</em>', 'status');

  variable_set('comment_preview_blog_entry', 0);
  drupal_set_message('[Blog content type] <em>Comment: Preview comment</em> option set to <em>Optional</em>', 'status');

  variable_set('comment_form_location_blog_entry', 1);
  drupal_set_message('[Blog content type] <em>Comment: Location of comment submission form</em> option set to <em>Display below post or comments</em>', 'status');
}

function features_blog_uninstall() {
  $vars = db_query("select * from {variable} where name like '%blog_entry'");
  $var_names = array();
  while ($var = db_fetch_object($vars)) {
    variable_del($var->name);
    $var_names[] = $var->name;
  }

  drupal_set_message('Blog-related settings have been deleted: ' . implode(', ', $var_names), 'status');
}

FYI I'm moving my patch over to CCK itself: #726330: Rewrite content_copy module to use existing hooks

See http://drupal.org/project/content_type_exporter

Would you mind providing a reason why this was marked as won't fix? The project linked to in #25 is marked as obsolete:

Due to improvements in Features I am abandoning this module in favor of the solution that is a) much closer to what I was hoping for, b) has a tremendous amount of community support. As a result, please migrate all use of this module to use Features instead. The code will be left in CVS for educational value & comedic factor.

Hi,

I'm not actually sure if my issue is related at all, but I believe so.

I run features in an installation profile. It all works great except the fact that the nodes are not sporting the correct 'promoted to frontpage' settings and they don't get the correct menu entries.

Everything I export with my features works great, it just seems as if Drupal's default way of dealing with certain settings refuse to budge for what my features are instructing. I can not revert and two features always appear as overridden, from the first time my installation profile completes.

I have looked to the issues about not being able to revert, I just suspect my problem is somewhere else. I just want to have my features take care of all the settings I export through them, including Drupal's defaults.

Subbing

Closing this for lack of activity. Please re-open this issue if you can reproduce it in the latest version.