Make taxonomy terms fieldable

Big +1 !

5. Figure out what's the output for those fields : where are they displayed by default ? at the top of taxo/term/tid pages like current 'description' ? Do we need to introduce a tpl file ? Are there other 'build modes' that make sense out of the box ?
See #409750: Overhaul and extend node build modes - basically, every fieldable entity has to wonder what are its display modes. Years of CCK has made this pretty clear for nodes, we have to invent this for other entities.

Bundles should definitely be vocabularies IMO. Technically, numeric bundle ids shouldn't be an issue for the field API, except if several entity types later come naturally with numeric bundles too. Maybe we can use 'vocab_[vid]' as a string id - sort of relates to 'prevent bundle names clashes', which hasn't been sorted out yet - and for which it seems we haven't even created an issue yet ?.

A nasty side effect however, whether we use a 'vocab_' prefix or not, is that those numeric 'vids as bundles names' bubble up to template names.
- field.tpl.php has a field-[field_name]-[bundle-name].tpl.php variant
- if terms are fieldable, they probably need a term.tpl.php (equivalent to node.tpl.php), and thus a term-[bundle-name].tpl.php variant
Having code or filenames depend on serial ids rather than string machine name is bad practice for code transportability, since ids can change from site to site (was one of the issues with flexinode). So this sort of calls for machine name for vocabularies :-(.

Could be done in a separate issue, though. I think #412518: Convert taxonomy_node_* related code to use field API + upgrade path will raise the same need : taxonomy fields using numeric vids in their definition will be painful to export / import.

Subscribing.

Subscribe...

Subscribe

No time to test just right now - but major yay !
Now is definitely a good time to push this forward. The sooner we get this in, the more time there is to digest the consequences (in followup patches) - for instance:
- making terms fieldable is a strong incentive to have the taxo listing pages themed through a template ? The current hook_taxonomy_load() probably begs for this too, BTW...
- how does a Field UI (even the current contrib one) fits in ?

About the patch itself: one of the possibly tricky bits in the form part is making sure the "add more" button works ok with unlimited-value fields. And actually, making the non-JS part work is a little more demanding on the form workflow. Existing node form workflow, albeit awful at places, was OK, but user form workflow is uglier still, and non-JS 'add more' currently doesn't work on user forms. See #367006: [meta] Field attach API integration for entity forms is ungrokable and field_add_more_submit()'s $form['#builder_function'] for the gory details.

Trying to test this, patch applied clean

Using devel module executing code below still have no effect

Looking in database - tables for field created but still no input form on term edit page

Suppose using taxonomy term as bundle is wrong this should be per vocabulary

$field = array(
  'field_name' => 'somefield',
  'type' => 'text',
  'cardinality' => 1, /* not necessary as it's the default.*/
);
field_create_field($field);
$instance = array(
  'field_name' => 'somefield',
  'bundle' => 'taxonomy term',
  'label' => t('Some field'),
  'description' => t('You can enter something here.'),
  'weight' => 0,
  'widget' => array(
    'type' => 'text_textfield',
    'label' => t('Some field'),
  ),
);
field_create_instance($instance);

- Minor : taxonomy_form_vocabulary() - "...for internal user only"
- machine name: field_attach_(create|rename|delete)_bundle() functions need to be called where appropriate
(field_attach_create_bundle() should also be called in the update function, probably)

Fix status after #15 crosspost over #13.

Other missing calls : field_attach_delete(), field_attach_form_validate(), field_attach_submit()

Subscribing. (First time in months.) Interesting to say the least. :)

Content types as of a few months ago have auto-machine-name-generating code, based on the human-readable name. Let's re-use that. If it causes problems with non-Latin characters, we're going to want to fix it in both places anyway.

- We probably need some form validation to ensure a machine name is not reused ?

- minor:

+  function createTerm($vocabulary) {
     $term = new stdClass();
     $term->name = $this->randomName();
-    $term->vid = $vid;
+    $term->vid = $vocabulary->vid;
+    $term->machine_name = $vocabulary->machine_name;
     taxonomy_term_save($term);
     return $term;
   }

$term->machine_name is kind of misleading, it's the vocab machine name, not the term machine name

- Setting $term->machine_name is not required for taxonomy_term_save($term); to work, right ? If it's just to have createTerm() return a fully built $term, it might be best to set the property after the term_save() ? or return the result of a term_load() ?

I am in favour of using name for machine names and title for human-readable names. This should actually be done consistently in Drupal.

We can start in this issue by using title and name for the human-readable name and machine name respectively.

re #33: I'd personally vote for the latter ($term->vocabulary_machine_name). Attached patch doesn't change this.

Updated patch:
- fixed taxonomy_load_vocabularies() -> taxonomy_get_vocabularies()
- fixed a couple 'taxonomy term' -> 'taxonomy_term' as $obj_type param in a few field_attach_*() calls
Also took care of the non-JS 'add more', as mentioned in #15:
- added multistep support to the term form, building on $form_state['term'] (the 'add more' button basically makes any form multistep),
- wrapped the "build $term from form values" part in a taxonomy_form_term_submit_builder() function, also called from the non-JS 'add more' handler.

And tested the feature to work just fine !

To test, create a vocab, give it the machine name 'test_vocab', and run the following code in a devel PHP block:

$field = array(
  'field_name' => 'term_field_multiple',
  'cardinality' => FIELD_CARDINALITY_UNLIMITED,
  'type' => 'text',
);
field_create_field($field);

$instance = array(
  'field_name' => 'term_field_multiple',
  'bundle' => 'test_vocab',
  'label' => 'Multiple',
);
field_create_instance($instance);

Note that even with the cool vocab machine names, we still face the more general Field API issue of bundle names clashes - in short, trouble happens when users create a node type named 'user', or a vocab named 'page'.
This is discussed in #470242: Namespacing for bundle names and is an issue we already have with nodes, so I don't think it should hold this patch.

Subscribe. Note to self: Read the issue.

Review based on just reading the patch:

Adding a "Machine readable name" field to the vocabulary form whose description is "for internal use only" does not tell users anything about what it is for. If they really don't need to know what it is for, it should just be generated automatically and invisibly, based on the original voc name or something. I saw discussion of auto-generating machine names but do not see code for it. If we can't auto-generate them, or if that is a task for a separate issue, then we should at least improve the description for now.

Also, the restrictions on machine-readable names, if the form element is present at all, should be explained on the form itself, not as a
"gotcha" only when the form is validated.

So, perhaps: "The unique machine name for this vocabulary, used to select term theme templates. Can only contain lowercase letters, numbers, and underscores."

Why add a textfield vocabulary_machine_name element to the term form whose #access is always false?

It's not critical, but couldn't the update function use db_update()->expression() to do all vocabs at once instead of needing the loop?

Some test cases are converted from form submissions into API calls. I'm all in favor of API calls and testing them, but it does mean we are losing the test of the form unless the form is explicitly tested somewhere else (in which case, YAY for converting tests to API when they can be).

So long as the machine-name auto-generation/fixing the description comments are addressed by a followup issue, none of these are blockers. I'm not RTBC'ing to see what the comments on my review are, but I'm also not CNW'ing because I'm not sure it is warranted.

Oh, and I forgot to add: Nice work!

"couldn't the update function use db_update()->expression() to do all vocabs at once instead of needing the loop?"

I think we need the loop on vocabs to run field_attach_create_bundle() on each vocab - which is currently missing.
The function happens to do nothing with the default sql storage (which we can be sure will be the one in use when the update runs), so it has no serious impact, but technically, not calling it is wrong.
Or we need a comment to explicitly state we skip it.

So CNW at least for this.

I do not see a diff for CHANGELOG.txt. Incidentally, CHANGELOG says of Field API:

* Provides a subset of the features of the Content Construction Kit (CCK) module.

I think "subset" is not necessarily accurate. :-)

sorry all... subscribe :(

taxonomy_fieldable_info() is missing a 'bundles' entry, listing existing bundles - see http://api.drupal.org/api/function/hook_fieldable_info/7.

Aside from this, I don't think we have remaining issues keeping us from RTBC ? (can't set it myself, I took part in the patch...)

Side issue, should not hold this back: #488542: Tie Field UI pages to any fieldable entity

Sorry, the PHPdoc is actually not clear on that :-/. The format for 'bundles' should be

array(
  'machine_name' => 'Human name',
  ...
);

Thanks ! This is ready for me.

Awesome. I'll review this tomorrow morning.

1. What are some good use cases for this? In the issue description, we suggest that synonyms and related terms could be converted to fields but I'm wondering if there are better, more compelling examples. The use case I can think of is to associate an image with a term -- I believe Jeremy Andrews once started a module for that. Are there any other contributed modules that we could nuke? Any other creative ideas? Having a couple of really compelling ideas validates the need for this and avoids that we are over-engineering this just because we can -- the taxonomy system is freaking enough people out as it is. ;-)

2. Instead of flattening the namespace to avoid clashes (eg. $term->vocabulary_machine_name and $term->machine_name) we could consider using nested objects: $term->vocabulary->machine_name and $term->machine_name). I'm not demanding that we change this; just bringing it up for discussion. Feels like a better way to deal with this problem.

3. I'm wondering if andypost's issues in #16 have been addressed. I haven't try to reproduce his experiment.

4. Does this have any performance impact on node listing pages that have a lot of taxonomy terms? There are sites that are taxonomy heavy.

@Dries my experiment in #16 was proposed by catch, there's a help page http://drupal.org/node/474420
Suppose taxonomy fields should work as described there and it's a most quick way to test it!

@Dries: I just started writing a patch to add a concept URI to taxonomy terms in line with the recent announcement of the new CommonTag format and part of the RDFa effort for Drupal 7. I'm not going into details here but in a nutshell, this would boost the linkage on the content on the web, and several web companies are involved in the consortium (including Yahoo! etc). Much like term_node, term_hierarchy, term_synonyms and term_relation, term_uri would be a good candidate for being a field.

@catch Thanx a lot for example! screenshot attached

Using vocabulary->machine_name as bundle is more clear but why not 'vid_' . $term->vid?

Edited:
catch: andypost: lets you export field/bundle definitions between sites.
catch: andypost: also nicer for templates field-bundlename.tpl.php

andypost's code in #16 didn't work because the $instance['bundle'] property ('taxonomy term') was wrong. It should be the machine name of the vocabulary.

re #60: see #3 - in short, numeric ids are evil when it comes to code portability between different sites, or between staged instances.
Similiar to the current node-[bundle (node type)].tpl.php template variants, we should have term-[bundle (vocab)].tpl.php (to be done in a followup patch), and having serial ids in template names is one road to hell. That's only one example.

@catch: convincing list of example + we can worry about #57(2) later.

I'll commit this once the test bot came back with results.

Committed! Thanks.

Two major field additions in a row. W00t !

Followup : #489834: Template for terms

Follow-up issue posted addressing goal 1: Convert taxonomy term subject and description to fields.

Keeping track of this thread...