I've implemented a simple term add action. You can't add advacned settings, as the vocabulary might be added on the fly - like my own use case :).

I've also created a remove term to accompany it.
Fago, in delete_term action, what should I return? currently it's
+ return array('taxonomy_term' => $taxonomy_term);, but that term is actually deleted.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

amitaibu’s picture

FileSize
4.19 KB

I thing something went wrong with the diff, so re-attaching.

fearlsgroove’s picture

FileSize
4.12 KB

Good timing! I need this! This patch doesn't eval properly for add term as it references 'name' and 'description', but those are nested in a 'term' array for the setting. This changes that to be flat, but also has the side affect of removing the fieldlist. What's the proper way to do this?

fearlsgroove’s picture

Status: Needs review » Needs work
amitaibu’s picture

Title: Taxonomy axtions: Add/ Delete term » Taxonomy actions: Add/ Delete term
Status: Needs work » Needs review

@fearlsgroove,
Have you tried the the patch in #1? because it should be working, regardless of the nested array. You can see that the vocabulary actions are also nested. Setting back to needs review on patch #1

fearlsgroove’s picture

I did test and the patch works as intended - it adds terms successfully. The problem is that it doesn't properly process input evaluators. This is because the eval input option is defined as follows:

'eval input' => array('name', 'description')

but the _form function adds it to settings as follows:

$form['settings']['term']['name'] = array(
...

Vocab actually has the same problem - it doesn't properly process input evaluators when a new vocab is added. Hope that clarifies what i meant.

Perhaps this should be a separate bug since vocab is broken in this respect as well? Or the underlying problem is that the eval input option isn't robust enough to handle items in fieldsets?

amitaibu’s picture

Ok, now the issue is clear to me. Fago - how should we do it?

fearlsgroove’s picture

I looked through the code and can't find any other usage of fieldsets in forms that have input evaluators processed. The simplest solution would seem to be to just drop the fieldset since it's cosmetic (isn't it?) It would seem some additional effort needs to be put into making the input evaluator flag more robust, or the settings needs to be extracted from the form fields somehow.

We'd also need to keep in mind whether the implementation would affect existing rules on an upgrade. For example, when I moved:
settings[term][name]

from patch #1 to

settings[name],

the existing rules broke until I resaved it. That's doesn't seem like an easy setting to upgrade.

But that's just my .02 :D

fago’s picture

Status: Needs review » Needs work

hm, try if setting

$form['settings']['term']['#tree'] = FALSE

helps.

fearlsgroove’s picture

rerolled with #tree, seems to work!

fearlsgroove’s picture

Status: Needs work » Needs review
fearlsgroove’s picture

On further investigation the last patch doesn't work -- with #tree = FALSE it won't save the settings.

fearlsgroove’s picture

Status: Needs review » Needs work
fearlsgroove’s picture

Status: Needs work » Needs review
FileSize
3.93 KB

Apparently setting '#tree' => TRUE causes the values name & description to be stored on the root of the form_state object rather than as children of the 'settings' object, so the submit handling functions in rules admin refuse to process them, so they can't be saved. I can't find a way with #tree and #parents to place the properties as children of settings as they'd need to be to be properly handled by the submit function.

Attached is basically the patch from #2 without the unneeded fieldset. The name and description settings are moved to be children of settings so they can be handled by input evaluators.

amitaibu’s picture

Status: Needs review » Needs work

We can declare our own submit handler, that can populate the form_values in the correct place.

The problem is not only this specific fieldset - sometimes we use other modules forms, so we'll nneed to overcome this issue at one point.

fearlsgroove’s picture

Previous patch didn't save terms properly - the 'term' item didn't exist. This version explicitly creates a form state with the necessary values.

fearlsgroove’s picture

Ha! it was in there all along.

/**
 * Returns a setting of a given name. For settings sitting a nested array
 * the array keys may be separated by '|' in the name.
 */
function &_rules_get_setting(&$settings, $name) {

This patch changes the name & description to be term|name and term|description.

amitaibu’s picture

nice catch! I'll test it in a couple of days.

amitaibu’s picture

Assigned: amitaibu » Unassigned
Status: Needs work » Reviewed & tested by the community

Tested and working.

fago’s picture

Status: Reviewed & tested by the community » Needs work

Oh, nice catch - thanks! It looks like I thought already about this problem.. ;)

@patch: Two things:

* Why does the delete action return the variable? It isn't touching it.

* You pass a $form_state to taxonomy_save_term(), which isn't one. This is quite confusing. It looks like the function takes $form_values, which is quite odd behaviour for an API function - but ok that's it. So I think we should call it $form_values too.

amitaibu’s picture

Status: Needs work » Needs review
FileSize
4.08 KB

Patch address both comments from #19.

fearlsgroove’s picture

Continuing that thought, why not just pass $settings['term'] into taxonomy_save_term directly?

amitaibu’s picture

@fearlsgroove,
not possible as taxonomy_save_term(&$form_values), gets $form_values by reference.

AlexisWilke’s picture

Guys,

Argh!

I have my own patch on this node: #487284: Add a new term in vocabulary, that Amitaibu marked as a duplicate.

I created my patch as a duplicate of the Vocabulary functionality which I like. It makes use of the Taxonomy form for the term form. Why would I want that? Yes, the parent & related terms are probably mostly unusable, although it is not impossible that all the term automatically added by the "Add new term" functionality should all be children of a given parent term that you created by hand. If I have three types of nodes and I want to create terms on these nodes, maybe I have a different parent based on the node type.

You current solution is real weak for that!!!

Not only that, if you add modules, such as the Taxonomy VTN which adds a few fields in there, you loose them. In other words, you cannot support those extra fields. Even if the fields are not compatible with the tokens module, it would be useful to me as a user to be able to make the selection I need in there...

So, I strongly urge all of you to consider my patch instead of the one here. Also I'm still working on it because of these multi-level arrays in the forms, I should have a newer patch ready real soon.

The only thing I like about your patch is that you can dynamically select the vocabulary. This is a concern because I do not see a good solution to support both... Only solution in sight right now, we apply both patches and have people wonder why they get two "Add new term" in their list of action... but you do have that with emails, so why not with terms?!

I will post my new patch here later.

Talk to you soon guys. 8-)
Alexis Wilke

AlexisWilke’s picture

Title: Taxonomy actions: Add/ Delete term » Taxonomy actions: Add/Delete/Parent term
Status: Needs review » Patch (to be ported)
FileSize
9.58 KB

Alright! There is my patch.

1. It includes your patch (i.e. your Add & Delete functionality)

2. It includes a way to enter the name of the term in the Load. I do think the drop down should offer two special options instead of my current solution: Load by ID, Load by Name and the actual terms. That means the $term_text would then be known to be ignored (i.e. not Load by ID nor Load by Name) or be used as an ID or a Name. My current solution is to, for instance, write name:[user:mail] to load a term named after the email of a user.

3. It includes my patch to Add a term with a fixed taxonomy, but the complete form for the term.

4. Yet another term addition: Assign a parent term to a term, which I also need for my customer.

I tested all of this and it works great on my setup. I can add a term named [account:mail] another [user:mail] and then mark one as the parent of the other. Real cool! 8-)

Thank you.
Alexis

amitaibu’s picture

Title: Taxonomy actions: Add/Delete/Parent term » Taxonomy actions: Add/Delete term
Assigned: Unassigned » amitaibu
Status: Patch (to be ported) » Needs review

@AlexWike,
Thanks for going over the issue.
> Assign a parent term to a term, which I also need for my customer.

Having the action "Assign a term as the parent of another term." sounds good, but let's move it to another issue - so there will be no scope creep - this issue is about adding/ deleting terms - and as is it already takes time to review and commit.
Same goes for loading a term by name.

> It includes my patch to Add a term with a fixed taxonomy, but the complete form for the term.

Maybe a better approach would be to check in rules_action_taxonomy_add_term_form() that the vocab exists. and if so present the full form - anyway, I think it can be in a follow up patch.

btw, status should be needs review, as it will not be ported - as all patches are against DEV

AlexisWilke’s picture

Amitaibu,

Okay, there is a patch with just the Add/Delete of the term. It is just much more annoying to have to break things down that way as you can imagine.

> check in rules_action_taxonomy_add_term_form() that the vocab exists

I think that would be complicated. There are two issues: (1) it is not at all available in the interface and (2) it will break if the taxonomy rule weight is changed on us (although I think this is true for a lot of things).

I think both methods are a good thing just like you have several method to send an email in the System actions.

> Status: needs review

I'll keep that in mind. I find it weird that we cannot mark an issue as offering a patch anymore.

Thank you.
Alexis

AlexisWilke’s picture

And eventually the patch. 8-)

amitaibu’s picture

The need for such an action is valid, however the proposed solution IMO is not correct. It's a bad usuabillity to have two actions that do (almost) the same thing. I haven't got a better solution yet but we can discuss other solutions.

AlexisWilke’s picture

So why do we have 3 solutions to the email problem? What's the difference? In all cases you're just sending an email!

Unfortunately, I do not see a viable solution to allow a dynamically selected vocabulary in my patch, but that would most certainly be the best solution since it offers all the fields. (note that your patch would currently work for me since I really only need the name and description at this point, but still!)

Thank you.
Alexis

fearlsgroove’s picture

I agree that multiple actions doing the same thing is confusing. I think it makes more sense to provide separate options: select a vocabulary from a list, one field to evaluate against a vocab ID and one field that would evaluate against a vocab name, all on the same rule. It could be an order or precedence effect -- select item if any, else vocab id if valid, else vocab name if valid.

AlexisWilke’s picture

Except that you cannot select one of the terms from a specific vocabulary if you don't first choose a vocabulary. But since all the terms are all defined in one table, I guess we could show them all with:

Vocabulary Name: Term Name

Then we don't need to first select a vocabulary, that's done at the same time you select the term.

And yes, I'm absolutely not against a 3rd box to give the term name.

Thank you.
Alexis

fago’s picture

hm, right, the situation with the mail address is somehow similar, however the "send to user" action is needed for users without token or php input evaluation to be able to send to a dynamic user - which is a *reaaaally* common use case. I'd not say so with dynamic vocabs.

But yes, this is not ideal as is and will be improved for rules 2.x. However for the user it doesn't make sense to add 2 actions, also it'd make upgrading more complicated - thus I agree with others that there should be only one action for adding a term. I think we should go with the dynamic vocabulary option only for now - as I think that way the action can be used with rules 2.x to do both as is.

So for now, let's go with that and a simple form. It's good to move additions like "Assign a parent term" in separate actions nevertheless, as thus it can be used for creating new terms as well as for editing existing ones. However I wonder, if we couldn't make the taxonomy_term variable 'savable'?

@AlexisWilke: Thanks for your engagement, feel free to add another action variant to 3rd party module, if really needed. For rules itself I want to tackle this problem, where it is, in the ground. That dynamic vs. static issue is everywhere, e.g. see the terms itself or also the flag integration.

So can you both agree upon a patch? :)

amitaibu’s picture

In #29 AlexisWilke wrote:

Unfortunately, I do not see a viable solution to allow a dynamically selected vocabulary in my patch

Since both patches are trying to provide a general + simple solution I think the one in #20 is the right one, as it covers static and dynamic terms. Indeed, futher actions such as set parent term, can extend this functionlity.

fearlsgroove’s picture

Status: Needs review » Reviewed & tested by the community

+1 for number #20

might I be so bold?

AlexisWilke’s picture

fearlsgroove,

Without a way to define the term by name, it is totally useless to me...

jbeall’s picture

I got here from #343403: Condition: content has term.

I would like to be able to define a condition that checks to see if the content has been tagged with a term that is in a specific set of terms. For instance, to say "this condition is met if the content has been tagged with the terms blue, yellow, or green in the 'colors' vocabulary."

I understand that #343403: Condition: content has term depends on this bug, so I am subscribing to this as well.

-Josh

fearlsgroove’s picture

So your key feature is adding a term using the complete taxonomy form, the reason being principally to assign term hierarchy. I think a better way to approach assigning hierarchy would be to variablize terms within rules and use them as arguments for separate actions.

Third party modules may do all kinds of things, which increases the risk of this becoming unstable if we use the whole form. A better approach there IMHO would be for each module that wants to integrate to define it's own integration.

I guess i'm all about the incremental as far as this goes ..

@jbeall -- I'm sorry I dropped that bomb of a patch on that issue and polluted it. You're right that it doesn't actually depend on this patch, except that the patch that I created had this patch already applied, so it's ugly. I'm hoping to put some effort into splitting that into separate issues soon, but it'd also be nice to have this patch drop first.

AlexisWilke’s picture

fearlsgroove,

This is the vocabulary function:

/**
 * Action: Create a new vocabulary configuration form.
 *
 * @see rules_action_taxonomy_add_vocab_submit().
 */
function rules_action_taxonomy_add_vocab_form($settings, &$form, $form_state) {
  module_load_include('inc', 'taxonomy', 'taxonomy.admin');
  $form['settings']['vocab'] = array(
    '#type' => 'fieldset',
    '#title' => t('Vocabulary settings'),
  );

  $form['settings']['vocab']['vocab'] = taxonomy_form_vocabulary($form_state, !empty($settings) ? $settings : array());
  // Remove the 'Save' button.
  unset($form['settings']['vocab']['vocab']['submit']);
}

Why would the "Add term" not use the same philosophy? Did something change from the time fago wrote the "Add vocabulary" function?

Now... I checked both forms (vocabulary & term) and none of the 3rd party widgets are added. This is because we directly call the taxonomy form function and we don't give the callbacks a chance to add their widgets. (Is that wrong?)

In other words, what I wrote is a continuation of what the primary author wrote. If you think that's wrong, you may want to ask him why he did it all wrong to start with.

My point of view is that the other patch is all wrong because it does NOT follow the same process as for the other parts of the exact same file...

Thank you.
Alexis Wilke

amitaibu’s picture

> if you think that's wrong, you may want to ask him why he did it all wrong to start with.

Since I wrote the vocabulary patch I think I can explain :) - We use the original vocab form provided in taxonomy module, since there is no predefined variable needed. So you create vocabs on the fly with no problem.

The reason we don't use the term form as is - is because we need to be able to create terms on the fly.

Another action to 'Set term parent' can compliment the suggested action.

AlexisWilke’s picture

Amitaibu,

Okay. Again, I do not need to assign a parent, related or weight, especially non-dynamic values (although some people could find that handy...)

What I really need is the fully dynamic names with Token support.

Thank you.
Alexis

amitaibu’s picture

> What I really need is the fully dynamic names with Token support.

Can you explain?

fearlsgroove’s picture

#20 supports dynamic names with token replacement ... The one exception is it won't properly evaluate taxonomy tokens without a patch that's pending in the token queue - see #465434: Token rules integration taxonomy fix. Thats just waiting on Fago to say it's ok to get into a release as far as I can tell :D

AlexisWilke’s picture

Amitaibu,

I need to create a new token each time a new user registers, pays for a specific service and create a sub-user. Each time, I create a term using the user identifier ([user-uid] or [account-uid]). Not being able to use those is VERY limiting. I cannot just create the same term over and over again, it would be useless.

The term is then used with TAC Lite to protect the work of that user/sub-users.

I thought the Add was working fine in that regard, the problem was the load where I could only load by ID which is rather useless for me. If I know the ID, I can as well use the drop-down offered.

Thank you.
Alexis

amitaibu’s picture

> Not being able to use those is VERY limiting.

This is not correct. You can still use tokens and PHP eval with the #20 patch:

+ 'eval input' => array('term|name', 'term|description'),

AlexisWilke’s picture

Amitaibu,

Yes. The problem was not with the Add & Delete but the Load functionality. 8-)

Sorry for the trouble!
Alexis

fago’s picture

Status: Reviewed & tested by the community » Fixed

ok, so I also think #20 is the right one -> committed.

Let's put stuff like assign the parent term in a separate action, possible working also on existing terms.

@AlexisWilke: I understand your use-case, so let's make sure it's possible, if it isn't yet?

Status: Fixed » Closed (fixed)

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

giorgio79’s picture

Thanks guys for this patch.

I am trying to test it, but I am getting:

Add a new term to vocabulary
Unavailable argument: Taxonomy vocabulary

When adding a new action. How do I set this argument?

I would like to assign the node title as a new term in a vocabulary, but the original node does not have tagging or taxonomy associated, is that the problem?

PS: Ok, I found out I have to load vocabulary to have the argument ready. :)

Looking nice!

antrios’s picture

Patch works great! I use it to create new term based on node's title (token) and assign it to that node. However there is a problem when I update newly created node - after update assignment beetwen term and node is deleted!

I tried to create rule which is triggerd on update - it is possible, but I can't use tokens when loading a term - using tokens is avaiable only in 'add new term' action.

Currently I use time consuming and manual method: I enable vocabulary for my node type, edit it manually, choose proper term from drop list (it exists - only assignment to node is deleted), save the node and finally disable vocabulary. Now doing changes is nightmare ;)

Could anyone help me?

antrios’s picture

Status: Closed (fixed) » Active
AlexisWilke’s picture

Antrios,

The MO Auto add terms module would help you with having to assign/unassign the taxonomy to your node type. That is, you could create a role, make yourself member of that role, and give only that role the right to see the drop down. That way, it is hidden for everyone except yourself.

Now, the real solution would be to fix the Rules if there is a real problem there... and I don't recall having such a problem. Do you have CCK taxonomies or other taxonomies in that same node?

Thank you.
Alexis

antrios’s picture

Alexis,

Thanks for response, I will try your method you described.

And now going back to my point. I created new node type with CCK, it has three text fields and one taxonomy. Page I'm trying to do is climbing guide, and the node type is called CRAG - text fields are called DESCRIPTION, PARKING, ACCESS. User selects also rock type (taxonomy) with drop down (for example granit, limestone etc.). After saving new node, rule is triggered and automatically I add term to CRAGS vocabulary with crag name. By doing that I have vocabulary CRAGS with crag names, which I use later in ROCK node type. ROCK type is assigned to CRAGS also, so when you create ROCK you have to select crag to which it belongs. I hope I described my idea ;) And it works ;) Unfortunatelly as soon as I edit CRAG node it losts information about CRAGS taxonomy ;(

To summarize I will try to draw all relations:

Crag node having:
ROCK TYPE taxonomy
DESCRIPTION CCK text field
PARKING CCK text field
ACCESS CCK text field
and term based on crag name added to CRAGS vocabulary autmatically by rule

Rock node having:
CRAGS vocabulary (I use drop down to select crag to which rock belongs)

It all sound complicated (at least for me), but I hope I explained it enough ;)
And finally description of problem - when I update CRAG node it losts information about assignment to CRAGS taxonomy, and I can't show rocks belonging to that crag, somehow rock losts its parent :)

Antrios

AlexisWilke’s picture

Do you lose the term only when you edit?

Can you see the terms properly attached before editing?

Maybe that's because a vocabulary is not properly attached to a node type. (i.e. Rock/Crags and Crags/Rock)

Alexis

fago’s picture

Category: feature » task
Status: Active » Fixed

The issue "Taxonomy actions: Add/Delete term" is fixed, for anything else please open separate issues.

Status: Fixed » Closed (fixed)

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

adpo’s picture

There is a problem with path #20:

An unknow line type was found in line 70! :

function rules_action_taxonomy_term_assign_to_content($node, $taxonomy_term, $settings) {

Roles ver. 6.x-1.x-dev

mrlava’s picture

Version: 6.x-1.x-dev » 6.x-1.2
Category: task » bug
Status: Closed (fixed) » Active

Did this problem come back or I am doing something wrong? I can have it add a term no problem (the [node:title] is used as the term), but when trying to delete it, it does nothing at all, it deletes the node and leaves the term.

The following is my rule:

IF

Deleted content is

DO

Load a term
Delete a term

mrlava’s picture

nevermind, i see i have to do it by term id... which... doesn't seem possible because it has to get the term from the title which has no id... maybe a feature request? let us choose the term to delete by title instead of id?

fago’s picture

Status: Active » Closed (fixed)

Please do not re-open old issues.

mitchell’s picture

Component: Provided module integration » Provided Module Integrations

Updated component.