Closed (fixed)
Project:
Signup
Version:
5.x-2.4
Component:
Email
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
19 Sep 2008 at 12:02 UTC
Updated:
22 Oct 2008 at 06:32 UTC
Jump to comment: Most recent file
Comments
Comment #1
dwwGood idea. Please provide a patch or a donation to the chipin. ;)
Comment #2
stborchertHi.
Simple patch.
Main question: how to deal with tokens like
%usernameas there is no signup information for this user? Should we load it from the$userobject?Comment #3
dwwCool, thanks.
Re: "how to deal with tokens like %username as there is no signup information for this user?"
%useremail is fine how you handle it. %username can just be care of the global $user object, yes. We should just pass $user to _signup_get_email_tokens() instead of an empty array (that function expects an object, not an array, anyway). Then, we can clobber $signup_tokens['%useremail'] like you're doing, to make sure we get it right for site admins who customize their from address. Of course, %info will be busted in this case, but I don't think I care. It's not even listed as a supported token. ;)
My only other (minor) concern is that I'm wondering if we should do anything to distinguish this message -- add "Copy:" to the subject or prepend something at the start of the body or something. I'm also wondering if, in that case, it'd make more sense to actually just leave the user-tokens unreplaced entirely. Might make it more clear, especially if someone sends a copy to themselves for an event they're already signed up for.
Thoughts?
-Derek
Comment #4
stborchertI thought about it but didn't do it because I thought it would be better to send the copy to the address entered in
From. But you're right, it should be send to the address the users entered in his profile.Hm.
What would a site admin (or someone with the proper permissions) expect?
"Copy: ": yes, I think so.
"unreplaces tokens": perhaps (I would, but I know the code ;-))
Ok, next try.
Comment #5
dwwActually, I said to pass in $user to get %username handled right, and then: "... we can clobber $signup_tokens['%useremail'] like you're doing, to make sure we get it right for site admins who customize their from address."
However, thinking about it more, it makes more sense to just force it to the address in the $user object. What if an admin entered a noreply@example.com address for the From? If you select "Send yourself a copy" it should go to "you", regardless of what you put in the From field.
I'm still torn on the token handling issue. I hate to suggest this, but I wonder if we should add a knob for this somehow? I'm probably way over-thinking this problem. ;)
Let's see if anyone else has a compelling story, but if I had to pick one and wanted to guess which will result in the fewest bug reports to me (the ultimate measure of any decision in this issue queue...) ;) then I think I'd go with replacing the tokens with the values from global $user, instead of leaving them untouched. I bet some people will freak if they see un-replaced tokens in their copy, and assume it means that's what everyone else saw, too.
So, I think the best approach would be a hybrid of #2 and #4...
Comment #6
dwwLike so. Lightly tested. Seems to work fine.
Comment #7
stborchertPatch looks good and works.
What do you think about a message in front of the original mail body?
Something like
t('This message was sent to all signed up users of %event', array('%event' => $event->title)).Comment #8
dwwThanks for the review and testing.
Re: the message prepended to the email body -- I thought about that, but I don't believe it's really necessary. The user in question *just* filled out a form about this, wrote in the text themselves, and selected "send myself a copy". If they're so busy/scatter-brained that they can't figure out where this message came from when it hits their inbox, I'm not sure I want to bloat this module to help them. ;) But, I'll hold off committing and marking this fixed to give deviantintegral and/or any other interested parties a chance to weigh in here on the latest patch and this topic. BTW, if we did a message like that, I'd probably want to wrap it in a theme function.
Comment #9
dwwAfter discussion in IRC with deviantintegral and akalsey, here's another approach:
Adds a theme function for the version copy sent to the sender, passes in both the raw and token-replaced versions, and lets the the theme function control what the sender sees. One of the justifications for this is that, since the broadcasts are never stored in the DB, the sender has no easy way to save what they typed with the raw tokens. Now that they see both, not only do they see how the tokens work in practice, they also have a copy they can easily reuse later....
Comment #10
dwwNow with phpdoc comments for that theme function (and a better/more funny name for the 2nd arg).
Comment #11
dwwEarlier tonight I committed #10 to HEAD, just forgot to update the status here...
Comment #12
Anonymous (not verified) commentedAutomatically closed -- issue fixed for two weeks with no activity.