Bug4176 support notifications should include username #427
Conversation
| category => $sp->{_cat}{catname}, | ||
| subject => $sp->{subject}, | ||
| username => LJ::trim( "$show_name" ), |
There was a problem hiding this comment.
(style) we don't need the quotes around the "$show_name"!
|
Okay, I managed to reproduce the broken behaviour that Fu was getting, and I think this one ACTUALLY does the trick (in terms of getting plausible test results)! |
|
Excellent thank you! I'll take a look shortly! On Wed, Jun 19, 2013 at 6:43 PM, kaberett notifications@github.com wrote:
|
| @@ -1081,10 +1088,20 @@ sub work { | |||
| $dbr->selectrow_array("SELECT message, type, userid FROM supportlog WHERE spid = ? AND splid = ?", | |||
| undef, $sp->{spid}, $a->{splid}+0); | |||
|
|
|||
| # set up $show_name for this environment | |||
| my $show_name; | |||
| if ( $posterid eq '0' ) { | |||
There was a problem hiding this comment.
nitpick: $posterid is an int here not a string, so I'd strongly prefer "$posterid == 0". What you have works, but it's much clearer re: expectations this way!
There was a problem hiding this comment.
(nitpick/idiom)
Oh actually, changed my mind, here's a bit more idiomatic, and hopefully intuitive, way of doing that, with a bit more stringent error-checking (note that this shouldn't be necessary, but LJ::load_userid isn't guaranteed to give us a proper $u object, so it's always best to check!
my $show_name;
if ( $posterid ) {
my $u = LJ::load_userid ( $posterid );
$show_name = $u->display_name if $show_name;
}
$show_name ||= $sp->{reqname};
Which basically says: try to get a name from the posterid/user object. If, after all that, we had no show_name still, then use the reqname.
Notifications of new support requests now contain the username or (for logged-out requests) name provided by user. Notifications of follow-ups to support requests now contain the username of the person who left the response. I do not allow for anonymous follow- ups because as far as I can tell they can't happen.
Bug4176 support notifications should include username
|
Whoo! thank you! |
|
I broke this. There are bugs. It needs reopening, please, and then I will (attempt to) fix it. ;) (Reopen req arises from discussion w/ Dre.) I will try to get to this in the next week, but it is 3am & I have a houseguest arriving in 12 hours, so ;) KNOWN ISSUES:
|
|
@kaberett Oh, when you said bug I assumed you meant bug. I don't think pull requests can be reopened, so just create a new one. |
Kareila did most of the work on this; I just copied it over and then got dre to tell me what to do with the one non-cooperative piece!
-- facepalm, branched it off the wrong thing. gimme a sec.
--- fix'd.