From http://drupal.org/node/693024#comment-3462560 #693024: Add alter hooks for the "faux" exportables.

Had two issues in features_get_default.

Whether to alter or not doesn't work without resetting as it doesn't track that the items have been altered or not yet (bug)

hook_node_info_alter is called but can't do anything as features doesn't handle this implementation, so patch adds an alter_state for features_api to make sure running alter is okay.. and adds drupal_alter when exporting a filter so node info can be altered (which is extremly useful; dynamic naming of content types!)! (feature request).

Patches iterconnected as one as one effects the other.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

hefox’s picture

Title: ADd hook_node_info_alter, handle alter/not alter differently in features_get_default » Add hook_node_info_alter, handle alter/not alter differently in features_get_default
Grayside’s picture

I've tried various ways of applying one or two patches to test this, and I'm not getting all the needed changes applied. Either there is a broken patch, or I need simpler instructions :)

I didn't like the documentation for features_get_alter_state() so I took a stab at rewriting it. Would have made a patch, but

/**
 * Gets the mode for alter hooks for a component.
 *
 * Alter hooks may be run per component instance or not at all, depending
 * on hook_features_api() implementation.
 *
 * @param $component
 *  (default: NULL) Specify a given component. If not specified, will return
 *  alter state for all components.
 * @param $reset
 *  (default: FALSE) Whether or not to rebuild the alter states from code.
 *
 * @return
 *  Associative array, maps the use of alter states against each component
 *  definition hook. Possible states include:
 *  - single: Run per single component.
 *  - inside_hook: An alter hook invocation is placed inside the default hook.
 *  - none: Do not alter (or alter is take care of after all components are loaded).
 */
hefox’s picture

Status: Needs review » Needs work

I was probably manually editing the patch or had some other patches, or some sort if sillyness that prevented it applying. I'll remake the patch, hopefully tonight.

goron’s picture

Status: Needs work » Needs review
FileSize
4.29 KB

Here's a rebuilt patch that applies cleanly to the 6.x-1.1 release and current dev (there were a few small changes to one of the functions).

hefox’s picture

Version: 6.x-1.x-dev » 7.x-1.x-dev
Status: Needs review » Needs work

Going to update this patch

  • features_get_alter_state -- screw that, why isn't there a centrlized place to get information from feautures api? that so could be cleaned up
  • Add key for what the alter hook is
  • Provide a global key that disables the alter from being run if it's being included
hefox’s picture

Status: Needs work » Needs review
FileSize
3.86 KB

Patch depends on #1390848: Centralize the call to hook_features_api().

What it does is add two new potential, non-required keys to hook_features_api

  • 'alter_type': What type of alter hook this hook uses. 'normal' is called after the main hook is called. 'inline' is embeded within the default hook and may not be implemented by some default hooks. 'none' is no alter hook exists. Defaults to 'normal'
  • 'alter_hook': What the name of the alter hook for this component is. Do not include the '_alter' part. Defaults to 'default_hook'.

This way a module can indicate if there is an alter hook, and what it is (and if an export needs to define it -- truefully don't alter_info it being used outside of hook_node_info, though there may be other core like that).

Adding some mess with an global to say whether to run an alter, not really happy about that.

hefox’s picture

Title: Add hook_node_info_alter, handle alter/not alter differently in features_get_default » Add hook_node_info_alter, and alter_type, alter_hook to features_info

Patch does not deal with the features_get_default bug anymore, focusing on the feature request.

hefox’s picture

Status: Needs review » Needs work

Need to update it for image styles

mpotter’s picture

I like most of the ideas in #6, but I'm wondering why the Node exportable needs the new inline alter added to it. Doesn't the existing features_get_default call drupal_alter for the default_hook already? Why does Node need to be different/special?

hefox’s picture

features_get_default is just a helper function for features to help it generically call default hooks by module and only used 'for real' for hooks that features itself has provided (fields, filters). hook_node_info (and image), while given features support via features itself, are core hook called by core functions ( _node_types_build, image_styles()), so cannot add any hook that is not already there in core. :-/

mpotter’s picture

Confused. Still seems like features_get_default is already calling drupal_alter('node_info', ... and thus the "inline" drupal_alter call is not needed, so I'm not sure you answered my question.

hefox’s picture

hook_node_info is a hook used by core. Core does not use features_get_default to call it, core uses _node_types_build.

hefox’s picture

Assigned: Unassigned » hefox
hefox’s picture

Status: Needs work » Needs review
FileSize
4.88 KB

I still reallly don't like the global/etc. doing in the patch, but it's sorta a screwy situation.

Grayside’s picture

You could replace it with a wrapper function that hits a statically cached value. That would avoid bad global mojo and perhaps simplify debugging.

hefox’s picture

Fixing type in alter hook for image (image_style => image_styles).

Meh, wrapper function is unnecessary overhead

mpotter’s picture

Status: Needs review » Needs work

+++ b/includes/features.node.inc
@@ -86,6 +87,10 @@ function node_features_export_render($module, $data, $export = NULL) {
+ $output[] = ' global $features_alter;'; ¶

whitespace at end of line

Also failed to apply to latest 7.x-1.x-dev version.

Otherwise I think the approach is good.

hefox’s picture

Status: Needs work » Needs review
FileSize
4.89 KB

Here's an reroll against a fresh pull of 7.x-1.x so hopefully it applies (with the white space removed)

Sarenc’s picture

The last patches weren't applying against the 6.x-1.x-dev of Feb 22/2012 so I re-rolled one.

I didn't see an /includes/features.image.inc file so I ignored that bit.

mpotter’s picture

I'm still not happy with the way the alter hook for node is implemented. The use of the global features_alter variable just seems kludgy. For the sake of trying to get the really important piece of this patch into Features soon, here is a reduced patch for the 7.x-1.x-dev version that removes the global and the node alter and just contains the core code to add the new alter hook information to the api.

hefox’s picture

Status: Needs review » Reviewed & tested by the community

sounds good to me

mpotter’s picture

Committed f7447af

mpotter’s picture

Status: Reviewed & tested by the community » Patch (to be ported)

Also, the node stuff should go to a new issue if we can figure out a better way.

hefox’s picture

Version: 7.x-1.x-dev » 6.x-1.x-dev
scottrigby’s picture

Status: Patch (to be ported) » Needs review
FileSize
3.36 KB

Merged f7447af to 6.x-1.x & patched for review.

hefox’s picture

mxmilkiib’s picture

I'm looking to export a forum setup to a feature.

I've patched Features 6.x-1.x-dev with #23 on #1390848: Centralize the call to hook_features_api() and #25 here, but I'm not getting 'Forum topic' in the Content types.

Is there a further step I should be taking? Or am I viewing this the wrong way, or simply barking up the wrong tree? Thanks for any advice.

DamienMcKenna’s picture

Bump for testbot.

hefox’s picture

Title: Add hook_node_info_alter, and alter_type, alter_hook to features_info » Add hook_node_info_alter
Version: 6.x-1.x-dev » 7.x-2.x-dev
Issue summary: View changes
Status: Needs review » Active

This is not going backported I think, so kicking this back to 7.x to add the hook_node_info_alter that was part of the original patches

hefox’s picture

Status: Active » Needs review
FileSize
682 bytes
marcusx’s picture

Status: Needs review » Reviewed & tested by the community

Thanks!

I had a content type named "Show" and needed to swap the t()function for adding some context so I can avoid the translation of the content type name. Works like expected. Here is my hook as an example for a potential use case.

function MYMODULENAME_node_info_alter(&$info) {
  if (key($info) == 'show') {
    $info['show']['name'] = t('Show', array(), array('context' => 'Content Type Name'));
  }
}
hefox’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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