Closed (fixed)
Project:
Role Change Notify
Version:
6.x-2.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
24 Aug 2009 at 00:26 UTC
Updated:
12 Apr 2014 at 14:51 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
rfayThis is a perfectly reasonable idea. I won't plan on doing it any time soon, but patches are welcome.
Comment #2
rfayUsing triggers and actions and rules you can do what you want with a php snippet. I think that may be your best path forward on this.
In general, I'd like the triggers-and-actions feature to be the future of this module. That way everybody can get what they want done without adding new features to the module.
Comment #3
dspring0021 commented@rfay, what kind of PHP snippet are you referring to. I need to do exactly that, but I can't think of how to do this. Furthermore, when using the Role Change Notify trigger in 'Triggers & Actions', I can't seem to find a way to get the correct tokens for the user whose account is having the role added. There's just stuff like 'current logged in user', 'author', 'owner', etc. See my comment on this post for more info.
Comment #4
ColinMctavish commentedsubscribing
Comment #5
teliseo commentedAttached is a patch to create separate add and remove triggers for each role (in addition to the global ones). By attaching these to different actions, different emails can be sent for each role addition (or removal).
Comment #7
rfayThanks, teliseo!
I see there's a thing or two to clean up still.
Way cool, and thanks so much!
I also hope you'll be willing to do this for the D7 version.
Comment #8
teliseo commented#5: role_change_notify-triggers_per_role.patch queued for re-testing.
Comment #10
rfayHmm. A retest of the committed code shows that your patch is not at fault. Something changed about the testbots and now we don't pass: http://qa.drupal.org/pifr/test/37173
I opened #1010320: DRUPAL-6--2 tests are broken (token problems with undefined indexes). Something has changed about the testbot....
Note that before this patch can go in it will need a test.
Thanks for your work on this!
-Randy
Comment #11
teliseo commentedHere’s a new patch that incorporates several things:
You had asked whether I intend to port my changes to D7. I don’t have time to do this now, as getting D6 to work the way I want is keeping me busy, but I might at some time in the future.
Comment #12
rfayWow, excellent.
Here's a reroll now that #1010320: DRUPAL-6--2 tests are broken (token problems with undefined indexes) is resolved and testing is working again. Unfortunately we can't test token-related stuff.
@teliseo, I think the reason for the user_load() is to make profile values appear for the profile support. Did you test it that way?
You seem to be invested in this stuff enough that you might be interested in being a comaintainer. Is that true?
Comment #13
teliseo commentedI guess your HEAD already had part of my fixes for the broken test that I was patching in role_change_notify_action_info_alter()—thanks for re-rolling the patch.
I have tested profile tokens without the user_load(), and they seem to work properly. I can’t find any difference between the user object arriving at nodeapi update and a new one obtained from user_load(), except that the original object has modifications that previous modules, or the calling module, have made. Unless I’m missing something, where else could a user object have come from at nodeapi update if not from a previous user_load()? I’ve been doing some work recently on Node clone and Node Repeat, so I think I understand this stuff pretty well.
Yes, I’d be happy to be a comaintainer of this module. Is there anything I have to do to initiate this?
Comment #14
rfayGreat, and Welcome!
I added your privileges for everything except CVS commit. You have to apply for CVS, but it's easy when you're invited to comaintain. Please apply at http://drupal.org/cvs-application/requirements. Once you've applied, give me the issue and I'll RTBC it and somebody will grant it for you. But you already have the privileges necessary to grant yourself CVS at http://drupal.org/node/333543/maintainers.
It will be fantastic to have somebody maintaining who is actually using the module, which I'm not.
Here's what I ask of comaintainers: Do your work publicly in the issue queue. Post patches in "needs review", rather than just committing. Make sure that things have tests if they can.
The reality is that most of the time nobody will review your work, so you'll be the reviewer (or with two of us maybe we'll be luckier than that!)
I'm going to test this patch one more round, but I'll leave it to you to finish it off once you have your CVS access.
---
Back on the user_load() issue, I think the case where you have to do a full load is when you're not using tokens. I'll check it.
Comment #15
rfayOh, if you're new to CVS... I recommend using it only to commit, as described here: http://groups.drupal.org/node/91424. We're not going to be there long, so it is not worth the effort of coming up to speed on it.
Comment #16
rfayOK, this looks good to me.
Are you also fixing #891214: warning: htmlspecialchars() expects parameter 1 to be string, array given in includes/bootstrap.inc on line 857, or am I mistaken?
I found no trouble with the profile fields with token on or off.
When you commit this, please add a note to the project page explaining the new feature and how it impacts existing users.
I'll be gone for the next couple of weeks so probably won't be looking at this.
Comment #17
rfayGuess I shouldn't in general RTBC my own patch (but it's really yours). But I'll let you RTBC and then "fix" it.
Comment #18
teliseo commentedThanks for making me a co-maintainer. I will do my best to act responsibly. Here is a link to my CVS account request. I’m already quite familiar with CVS, but will be much happier when everything moves to Git.
Yes, my patch also fixes #891214: warning: htmlspecialchars() expects parameter 1 to be string, array given in includes/bootstrap.inc on line 857—sorry I missed that. Note that the patch submitted there only prevents the PHP warning, but doesn’t actually decode the date profile field. My fix decodes the date field. I’ll update the issue after my patch is committed.
Comment #19
avpadernoFYI, teliseo has now CVS access to this project.
Comment #20
teliseo commentedNo further comments, so it’s now RTBC. Patch has been committed—waiting for snapshot update.
Comment #21
teliseo commentedCommitted to DRUPAL-6--2: http://drupal.org/cvs?commit=477344
Comment #23
simone960 commentedAny patch for D7 ?
Comment #24
simone960 commentedFound a way to do it with Rules module instead.
http://goo.gl/30hJvo
Comment #25
rfayAdded that to the project page.