Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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.
Comment | File | Size | Author |
---|---|---|---|
#27 | rules-6.x-add_del_term.patch | 6.83 KB | AlexisWilke |
#24 | rules-6.x-add_del_n_parent_term.patch | 9.58 KB | AlexisWilke |
#20 | rules-add-remove-term-20.patch | 4.08 KB | amitaibu |
#16 | rules-add-remove-term-5.patch | 4.13 KB | fearlsgroove |
#15 | rules-add-remove-term-4.patch | 4.01 KB | fearlsgroove |
Comments
Comment #1
amitaibuI thing something went wrong with the diff, so re-attaching.
Comment #2
fearlsgroove CreditAttribution: fearlsgroove commentedGood 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?
Comment #3
fearlsgroove CreditAttribution: fearlsgroove commentedComment #4
amitaibu@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
Comment #5
fearlsgroove CreditAttribution: fearlsgroove commentedI 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:
but the _form function adds it to settings as follows:
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?
Comment #6
amitaibuOk, now the issue is clear to me. Fago - how should we do it?
Comment #7
fearlsgroove CreditAttribution: fearlsgroove commentedI 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
Comment #8
fagohm, try if setting
$form['settings']['term']['#tree'] = FALSE
helps.
Comment #9
fearlsgroove CreditAttribution: fearlsgroove commentedrerolled with #tree, seems to work!
Comment #10
fearlsgroove CreditAttribution: fearlsgroove commentedComment #11
fearlsgroove CreditAttribution: fearlsgroove commentedOn further investigation the last patch doesn't work -- with #tree = FALSE it won't save the settings.
Comment #12
fearlsgroove CreditAttribution: fearlsgroove commentedComment #13
fearlsgroove CreditAttribution: fearlsgroove commentedApparently 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.
Comment #14
amitaibuWe 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.
Comment #15
fearlsgroove CreditAttribution: fearlsgroove commentedPrevious patch didn't save terms properly - the 'term' item didn't exist. This version explicitly creates a form state with the necessary values.
Comment #16
fearlsgroove CreditAttribution: fearlsgroove commentedHa! it was in there all along.
This patch changes the name & description to be term|name and term|description.
Comment #17
amitaibunice catch! I'll test it in a couple of days.
Comment #18
amitaibuTested and working.
Comment #19
fagoOh, 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.
Comment #20
amitaibuPatch address both comments from #19.
Comment #21
fearlsgroove CreditAttribution: fearlsgroove commentedContinuing that thought, why not just pass $settings['term'] into taxonomy_save_term directly?
Comment #22
amitaibu@fearlsgroove,
not possible as taxonomy_save_term(&$form_values), gets $form_values by reference.
Comment #23
AlexisWilke CreditAttribution: AlexisWilke commentedGuys,
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
Comment #24
AlexisWilke CreditAttribution: AlexisWilke commentedAlright! 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
Comment #25
amitaibu@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
Comment #26
AlexisWilke CreditAttribution: AlexisWilke commentedAmitaibu,
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
Comment #27
AlexisWilke CreditAttribution: AlexisWilke commentedAnd eventually the patch. 8-)
Comment #28
amitaibuThe 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.
Comment #29
AlexisWilke CreditAttribution: AlexisWilke commentedSo 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
Comment #30
fearlsgroove CreditAttribution: fearlsgroove commentedI 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.
Comment #31
AlexisWilke CreditAttribution: AlexisWilke commentedExcept 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
Comment #32
fagohm, 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? :)
Comment #33
amitaibuIn #29 AlexisWilke wrote:
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.
Comment #34
fearlsgroove CreditAttribution: fearlsgroove commented+1 for number #20
might I be so bold?
Comment #35
AlexisWilke CreditAttribution: AlexisWilke commentedfearlsgroove,
Without a way to define the term by name, it is totally useless to me...
Comment #36
jbeall CreditAttribution: jbeall commentedI 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
Comment #37
fearlsgroove CreditAttribution: fearlsgroove commentedSo 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.
Comment #38
AlexisWilke CreditAttribution: AlexisWilke commentedfearlsgroove,
This is the vocabulary function:
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
Comment #39
amitaibu> 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.
Comment #40
AlexisWilke CreditAttribution: AlexisWilke commentedAmitaibu,
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
Comment #41
amitaibu> What I really need is the fully dynamic names with Token support.
Can you explain?
Comment #42
fearlsgroove CreditAttribution: fearlsgroove commented#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
Comment #43
AlexisWilke CreditAttribution: AlexisWilke commentedAmitaibu,
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
Comment #44
amitaibu> 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'),
Comment #45
AlexisWilke CreditAttribution: AlexisWilke commentedAmitaibu,
Yes. The problem was not with the Add & Delete but the Load functionality. 8-)
Sorry for the trouble!
Alexis
Comment #46
fagook, 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?
Comment #48
giorgio79 CreditAttribution: giorgio79 commentedThanks 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!
Comment #49
antrios CreditAttribution: antrios commentedPatch 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?
Comment #50
antrios CreditAttribution: antrios commentedComment #51
AlexisWilke CreditAttribution: AlexisWilke commentedAntrios,
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
Comment #52
antrios CreditAttribution: antrios commentedAlexis,
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
Comment #53
AlexisWilke CreditAttribution: AlexisWilke commentedDo 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
Comment #54
fagoThe issue "Taxonomy actions: Add/Delete term" is fixed, for anything else please open separate issues.
Comment #56
adpo CreditAttribution: adpo commentedThere 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
Comment #57
mrlava CreditAttribution: mrlava commentedDid 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
Comment #58
mrlava CreditAttribution: mrlava commentednevermind, 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?
Comment #59
fagoPlease do not re-open old issues.
Comment #60
mitchell CreditAttribution: mitchell commentedUpdated component.