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.
So we only have the action: Fetch content flagged by user. This only grabs a list as nodes. I need to be able to get flagged content that is something other than nodes... In this case actually entityForms...
One of my rules needs to check to see if you have flagged any of the entities related to a specific entity form type.
I've noticed that there is already code allowing you to flag any flag type. Should be able to implement the same for fetching flagged entities as well.
Comments
Comment #1
generalredneckSince there is already a loop that prints out all of the entity types and corresponding actions for each one, I just piggy backed off of it.
This allows you to get more than just nodes by user which was the case before. It also maintains backward compatibility so that people who currently use the option for just nodes are not disrupted by this being updated.
Comment #2
joachim CreditAttribution: joachim commentedGood call!
Just a few things:
I have a hunch that the entity label must always be set -- check the docs for the info hook.
Nice touch! (Shame this didn't get spotted till after I released 3.0, as we could have just changed it and not bothered with backwards compatibility...)
But can we keep the 'node' item in, even if it's a duplicate, to encourage the better behaviour?
Comment #3
generalredneckI think to be able to do that we would have to write up great script to change all of the existing rules from entity to node. I dont see having a duplicate dropdown item as desirable. It would most likely get called out as a bug later.
Your hunch is right... but if you see what I replaced below you will understand why I did that.
'label' => isset($entity_info[$type]['label']) ? $entity_info[$type]['label'] : $type,
I figured someone had a reason for doing it before. Also turns out that having that fore the label in the old code actuslly covrred up the fact they were checking the wrong key. The array is only 1 level deep and not 2. Which meant we were showing the machine name for all types.
I'll get it fixed up.
Comment #4
joachim CreditAttribution: joachim commented> I dont see having a duplicate dropdown item as desirable. It would most likely get called out as a bug later.
Call the backward compatibility one something like 'Node (legacy)' so they look different in the dropdown. I do see your point about the duplicate being taken for a bug in future, but the node one being called 'entity' will be taken as a WTF in future :)
> I figured someone had a reason for doing it before.
Maybe. Or maybe whoever it was didn't read the docs ;) Let's drop it.
Comment #5
generalredneckI was going through and cleaning up the lump of open issues that I've started and not finished and I came across this one...
I did try to make an upgrade so that we could go ahead and get rid of the "Legacy"... but it was really difficult. I've spent all morning on it. I had a hard time exporting and then importing the action which was the only way I could figure out how to do that...
So instead I did almost exactly what you asked for. Enjoy!
Comment #6
shabana.navas CreditAttribution: shabana.navas commentedThanks generalredneck. Tested this patch and it works as designed. As much as I hate seeing duplicate behaviors (Fetch node flagged by user & Fetch node flagged by user (Legacy), I guess it is something that we will have to stick in there for compatibility purposes. Hoping to commit this to the dev version asap. @Joachim, hope you confer.
Comment #7
joachim CreditAttribution: joachim commented> As much as I hate seeing duplicate behaviors (Fetch node flagged by user & Fetch node flagged by user (Legacy), I guess it is something that we will have to stick in there for compatibility purposes
Indeed! But once this is in, let's file a follow-up to remove the legacy one for D8.
Patch looks good to me. Go ahead and commit :)
Comment #8
generalredneckJust a quick thought... something I didn't think of doing. You may ask in the rules IRC channel about if they would have a good idea on how to do an upgrade on rules that people import and currently have in affect to update from the legacy rule to the new one. This may make this a bit less painful to commit :P.
Oh and if you don't mind, could you please put the commit in as attributed to me? I would be much appreciative.
Comment #9
joachim CreditAttribution: joachim commented> do an upgrade on rules
Digging around in rules.install may reveal some existing upgrade procedures for this sort of thing.
Yup, I always try to attribute commits, but good idea to remind our new maintainer :)
@Shabana: I've probably already told you, but I suggest you use Dreditor, which makes making commit messages very easy, including attributing them to the primary author of the patch.
Comment #10
shabana.navas CreditAttribution: shabana.navas commented@generalredneck, You will definitely get your name attributed. You did the work, you should get the kudos.
> Digging around in rules.install may reveal some existing upgrade procedures for this sort of thing.
Yeah, I will do some digging around and hopefully, we can find an easier solution for the D8 upgrade for the 'Legacy' issue.
@joachim, Yup, I have been using Dreditor since you gave me the heads-up on it. It really makes life a lot easier when it comes to checking patches and creating commit messages, which were always a pain for me. Thanks for that tip.
Comment #11
shabana.navas CreditAttribution: shabana.navas commentedCommitted to 7.x-3.x branch.
Comment #12
joachim CreditAttribution: joachim commentedCongrats!
It looks like your forgot to give the git attribution though -- using dreditor, you need to click on one of the names it pops up beneath the proposed commit message, and then use the entire git command on the command line to make that user the author of the commit (as well as being credited in the commit message).
Comment #13
shabana.navas CreditAttribution: shabana.navas commentedDoh! Is it too late or can I still change it?
Comment #14
joachim CreditAttribution: joachim commentedWe can't re-do commits once they're pushed to d.org git unfortunately!
Comment #15
shabana.navas CreditAttribution: shabana.navas commentedYikes! @generalredneck, really sorry. Definitely won't happen again. Will make sure you get your proper dues in the next commit for the issue: https://drupal.org/node/2054927.
Comment #16
generalredneckDon't worry too much about it... **It happens.
Comment #18
joachim CreditAttribution: joachim commentedI don't know if it was this patch or another of the changes to our Rules support, but something has broken -- any chance someone could take a look at #2077017: Most Recent Update (7.x-3.1) Broke Rules Configurations?