Features export does not take flag_definition_alter into account [#1971980]

When recreating a feature containing a flag for an entity where the flag definition is provided by the flag_flag_definitions_alter hook in flag.inc, I get the following error when opening the "Recreate" page of Features:

"Notice: Undefined index: taxonomy_term in flag_features_export() (line 22 of ...modules/contrib/flag/includes/flag.features.inc)"

The error is thrown because flag_features_providing_module() in flag.features.inc does not take the flag_definitions_alter hook into account when mapping the flag types to modules.

Comment	File	Size	Author
#8	flag-features_export-1971980-8.patch	700 bytes	Volx
#4	flag-features_export-1971980-4.patch	657 bytes	Volx
#3	flag-features_export-1971980-3.patch	577 bytes	Volx
#2	flag-features_export-1971980-2.patch	581 bytes	Volx

Comments

Comment #1

joachim commented 16 April 2013 at 20:18

Version:

7.x-2.0

» 7.x-3.x-dev

Thanks for reporting this.

Will probably need fixing on 3.x first.

Comment #2

Volx commented 20 August 2013 at 17:20

Status	File	Size
new	flag-features_export-1971980-2.patch	581 bytes

I finally got around to fixing this problem. Actually Flag is using its own hook wrongly. Per flag.api.php hook_flag_definitions_alter should be used to alter flag definitions provided by other modules, but flag_flag_definitions_alter actually adds _new_ definitions. If the alter hook would be allowed to add definitions, determining what module actually provides a definition is overly complicated. So flag should add flag definitions for entities not already provided for, not in the alter hook but in flag_fetch_definitions.

So assuming that flag is the only module adding definitions outside of hook_flag_definitions, we can simply check for any entities that have not been provided for and safely assume that Flag is the providing module. That is what the patch below does.

The patch applies to 7.x-2.0. A patch for 7.x-3.x-dev will follow.

Comment #3

Volx commented 20 August 2013 at 17:33

Status	File	Size
new	flag-features_export-1971980-3.patch	577 bytes

There is a small error in the patch above, here is a better one.

Comment #4

Volx commented 20 August 2013 at 17:49

Status:

Active

» Needs review

Status	File	Size
new	flag-features_export-1971980-4.patch	657 bytes

And here is the proposed patch for 3.x-dev. For the problem mentioned above, I created a new minor issue here #2070187: Fix misuse of hook_flag_type_info_alter.

Comment #5

joachim commented 21 August 2013 at 06:58

Urgh, that bit of flag always makes my head hurt. I just had to go remind myself why flag_features_providing_module() can't just call flag_fetch_definition()!

Comment #6

joachim commented 21 August 2013 at 07:01

Does it matter that vocabs aren't flaggable?

      // We deliberately exclude taxonomy vocabularies from the list of
      // supported entity types because they aren't fieldable or directly
      // viewable, which makes them impossible to flag.
      if ($entity_type === 'taxonomy_vocabulary') {
        continue;
      }

(If you need to reroll for this, can you also wrap the comment to 80 lines please?)

Comment #7

joachim commented 21 August 2013 at 07:04

Issue tags:

+3.1 blocker

Tagging as 3.1 blocker.

Comment #8

Volx commented 21 August 2013 at 12:16

Status	File	Size
new	flag-features_export-1971980-8.patch	700 bytes

I already exclude 'taxonomy_vocabulary'

if (!isset($modules[$entity_type]) && empty($entity['configuration']) && $entity_type !== 'taxonomy_vocabulary') {
  $modules[$entity_type] = 'flag';
}

But I fixed the comment length.

Comment #9

joachim commented 13 September 2013 at 09:06

Status:

Needs review

» Fixed

Committed, with a tweak to the comment wrapping.

Thanks for all your work on this.

git commit -m "Issue #1971980 by Volx: Fixed Features export not taking into account provision of flag types by flag_definition_alter()." --author="Volx "