SQLTeam.com | Weblogs | Forums

Email is getting sent but without content?


#1

Greetings experts,

The following code is intended to grab records from ClosedOrders table and email to the user with a link to take a survey.

When the user receives the email, the use is supposed to get the following content:

This is a computer generated email message.
Please DO NOT use the REPLY button above to respond to this email.

	    Dear '+ @authorizedname +':
	    
	    Thank you for using the order processing system.
	    
            Please click the link below to complete a survey
	    
	    http://survey.php?wo=@ID
	    
	    Regards, 
       Order administrator.' 

So far, when the email comes through, the body is blank.

Only thing a user sees is the subject that says, "Feedback Survey"

Any ideas what I am doing wrong?

This has been an awesome forum.

Thank you very much.

    ALTER proc [dbo].[GetUpdatedRecords] 
@UserEmail nvarchar(200)
 as
BEGIN


 Declare @sender nvarchar(200)
 declare @dept nvarchar(200)
 declare @loc nvarchar(200)
 declare @dteCreated nvarchar(20)
 declare @ID nvarchar(12)
 declare @email nvarchar(200)
 declare @authorizedname nvarchar(200) 
 declare @message nvarchar(1000)


  select @email = '', @message = 'SET QUOTED_IDENTIFIER OFF;
                    This is a computer generated email message.
		    Please DO NOT use the REPLY button above to respond to this email.
		    
		    Dear '+ @authorizedname +':
		    
		    Thank you for using the order processing system.
		    
	            Please click the link below to complete a survey
		    
		    http://survey.php?wo=@ID
		    
		    Regards, 
           Order administrator.' 
	 SELECT top 1 @email = @email+';'+Email, @message = @message
	 	FROM ClosedOrder
    WHERE status = 'Closed' and email=@UserEmail

EXEC msdb.dbo.sp_send_dbmail
@profile_name = 'Feedback', -- our defined email profile or static info
@recipients = @email,
@subject = 'Feedback Survey',
@body = @message;



Update ClosedOrder SET lsOpen = 1 WHERE lsOpen = 0 and email=@UserEmail

END

exec [GetUpdatedRecords] @userEmail='john.doe@yahoo.com'

#2

Is that the entire code, or is there more to it. If it is the entire code, the @email will be null because the variables are not initialized, which means they will be null.

Initialize those, for example like this:

declare @email nvarchar(200) = ''
 declare @authorizedname nvarchar(200) = ''
 declare @message nvarchar(1000) = ''

#3

Amazing!

That's it?

It is working. I can't believe that's all I needed!

Thanks a lot for your help

Just one more question, do you know why the ID value is not getting passed to the link?

Neither is the @authorizedname

By the way, that's the entire code.


#4

If you look through the code, @ID is declared, but it is not given a value. You have to give it a value (perhaps via a query?), or manually. For example, if @ID is always 123, you could do this:

DECLARE @dteCreated NVARCHAR(20)
DECLARE @ID NVARCHAR(12) = '123'; -- <---- GIVING ID A VALUE
DECLARE @email NVARCHAR(200) = ''
DECLARE @authorizedname NVARCHAR(200)  = ''
DECLARE @message NVARCHAR(1000) = ''


SELECT  @email = '' ,
        @message = 'SET QUOTED_IDENTIFIER OFF;
            This is a computer generated email message.
    Please DO NOT use the REPLY button above to respond to this email.
    
    Dear ' + @authorizedname + ':
    
    Thank you for using the order processing system.
    
        Please click the link below to complete a survey
    
    http://survey.php?wo= '  + @ID + '
    
    Regards, 
   Order administrator.' 

Note that you have to concatenate the variable to the string for it to get the value.

The reason for @authorizedname being blank is the same. You have to find a way to give it a value.


#5

You shouldn't do this:

see: SET @local_variable (Transact-SQL)

where it reads in part:

Do not use a variable in a SELECT statement to concatenate values (that is, to compute aggregate values). Unexpected query results may occur. This is because all expressions in the SELECT list (including assignments) are not guaranteed to be executed exactly once for each output row. For more information, see this KB article.


#6

gbritton,
Thanks for that piece of advice.

The issue currently though is that using James's solution, the code below when it comes through my email:
http://survey.php?wo= ' + @ID + '

displays as
http://survey.php?wo= + @ID +
I expect either blank or some value.

Any ideas?


#7

James,

I got the 123 passed to the link correctly but I need it as part of the query.

Adding it to the query below is not working:

	 SELECT top 1 @email = @email+';'+Email, @message = @message, @RequestID = RequestID,@authorizedname=InitiatedBy   
 	FROM ClosedOrders
WHERE status = 'Closed' and email=@UserEmail

#8

You don't seem to have the variable @RequestID defined. Was your intention to use @ID? If so, change the select statement to this:

SELECT TOP 1
        @email = Email ,
        @message = @message ,
        @ID = RequestID ,
        @authorizedname = InitiatedBy
FROM    ClosedOrders
WHERE   status = 'Closed'
        AND email = @UserEmail

#9

Sorry I got the IDs confused.

I already did what you suggested but it keeps coming blank.

Is it because of where the query is placed?

The ID is not passing the values. The authorizedname is not passing the values.

Please see entire query again.

Thanks so much for your patience.

declare @ID nvarchar(50) = ''
declare @email nvarchar(200) = ''
declare @authorizedname nvarchar(200) = ''
declare @message nvarchar(1000) = ''

SET QUOTED_IDENTIFIER OFF;
select @message = '
This is a computer generated email message.
Please DO NOT use the REPLY button above to respond to this email.

	    Dear '+ @authorizedname +':
	    
	    Thank you for using the order processing system.
	    
            Please click the link below to complete a survey
	    
	    http://survey.php?wo= ' + @ID + '
	    
	    Regards, 
       Order administrator.' 
 SELECT top 1 @email = @email+';'+Email, @message = @message, @ID = ID,@authorizedname=InitiatedBy   
 	FROM ClosedOrders

WHERE status = 'Closed' and email=@UserEmail


#10

Yes, it is because the query that fills in the values of the variables is after the statement where you compose the @message string. But, I did not understand how @ID could have gotten its value. In any case, can you try this?

ALTER PROC [dbo].[GetUpdatedRecords]
    @UserEmail NVARCHAR(200)
AS 
    BEGIN
        DECLARE @ID NVARCHAR(50) = ''
        DECLARE @email NVARCHAR(200) = ''
        DECLARE @authorizedname NVARCHAR(200) = ''
        DECLARE @message NVARCHAR(1000) = ''

        SET QUOTED_IDENTIFIER OFF;

        SELECT TOP 1
                @email = @email + ';' + Email ,
                @message = @message ,
                @ID = ID ,
                @authorizedname = InitiatedBy
        FROM    ClosedOrders
        WHERE   status = 'Closed'
                AND email = @UserEmail;
        
        SELECT  @message = '

This is a computer generated email message.
Please DO NOT use the REPLY button above to respond to this email.
	    Dear ' + @authorizedname + ':

	    Thank you for using the order processing system.

            Please click the link below to complete a survey

	    http://survey.php?wo= ' + @ID + '

	    Regards, 
       Order administrator.' 
       
        EXEC msdb.dbo.sp_send_dbmail @profile_name = 'Feedback', -- our defined email profile or static info
            @recipients = @email, @subject = 'Feedback Survey',
            @body = @message;



        UPDATE  ClosedOrder
        SET     lsOpen = 1
        WHERE   lsOpen = 0
                AND email = @UserEmail

    END
GO

#11

Ok I got that worked out.

As soon as I moved the email above the @message, it worked.

I just have ONE last question.

I need to pass 4 parameters to this link.

How do I add to the one already here?

http://survey.php?wo= ' + @ID + '

I tried:

http://survey.php?wo= ' + @ID + '&loc=' + @location + '

While I did not get an error, it didn't hyperlink all of the values.


#12

I don't know the default rules that the mail client might use for hyperlinking. One option might be to send the e-mail as html and explicitly specifying the hyperlink.


#13

Ok, thanks a lot for all your help.

You rock!


#14

Thats right - he rocks!
Show him by liking he's replies (heart symbol).


#15

I did and I really appreciate his help.


#16

I agree. I have a number of suggestions of things that could be improved.

Is there some reason why the @variables are being populated instead of just creating the @message?

        SELECT TOP 1
                @email = @email + ';' + Email ,
                @message = @message ,
                @ID = ID ,
                @authorizedname = InitiatedBy
        FROM    ClosedOrders
        WHERE   status = 'Closed'
                AND email = @UserEmail;

Change

@email = @email + ';' + Email ,

to

@email = Email ,

In fact because the WHERE clause is

AND email = @UserEmail

then you could just use @UserEmail instead of @email

... sp_send_dbmail ...
            @recipients = @UserEmail, ...

although setting it explicitly from the DB would ensure that it was capitalised as-stored in the DB (if that is important).

This

@message = @message ,

is doing nothing useful. Personally I would set the message here e.g.:

SELECT TOP 1
                @email = Email ,
                @message = '

This is a computer generated email message.
Please DO NOT use the REPLY button above to respond to this email.
	    Dear ' + InitiatedBy + ':

	    Thank you for using the order processing system.

            Please click the link below to complete a survey

	    http://survey.php?wo= ' + ID + '

	    Regards, 
       Order administrator.' 
        FROM    ClosedOrders
        WHERE   status = 'Closed'
                AND email = @UserEmail;

What if [InitiatedBy] or [ID] are NULL? The whole message will be NULL ... presumably the message makes no sense (in this form) if either of those are NULL so just add

                AND InitiatedBy IS NOT NULL
                AND ID IS NOT NULL

to the WHERE clause. (Maybe those columns are defined as NOT NULL anyway? If this is an email going to end users, outside the company, I would leave them in anyway to prevent any future-accident as it would be embarrassing, to the company, to send a blank message! or a message with a LINK that does not work.

Also if ID is defined as a numeric datatype?? then it should be explicilty converted (in some circumsatnces SQL will implictly convert it, but I think it is better to always explicilty convert it then anyone else looking at the code, or you in a few years time!, will be able to see that the conversion was deliberate rather than accidental!)

If ID is numeric change

' + ID + '

to

' + CONVERT(varchar(20), ID) + '

There is no ORDER BY on the SELECT TOP 1. So it is entirely random which record is chosen (for a given email = @UserEmail) and thus which Survey ID is used. At the very least add an ORDER BY the Primary Key(s) for [ClosedOrders] so that the sequence is at least predictable / repeatable.

The Email is being sent based on

WHERE status = 'Closed'

but the update is being performed based on

        WHERE   lsOpen = 0

I would prefer to see them being preformed on the same criteria, but maybe this "difference" is deliberate / by design?

If nothing else the (lack of!) Normalisation would worry me that an email could be sent on an order that has status "Closed" and IsOpen = 1 or some variation of that.

In fact that will happen. The email is sent based on

WHERE status = 'Closed'

but that Status is then NOT changed, so the next time you run this the same user will get the email again, won't they?

I would prefer to see a Status that was a progression of numbers / stages:

10=Open
20=Closed but email not sent
30=Closed - email has been sent

so that the steps are clear and there aren't any "unanticipated" complications of multiple status codes that don't make sense!

If multiple such Status codes imply "Open" or "Closed" I would have a separate Status table:

StatusID - 10, 20, 30 as above
Name - i.e. "Open", "Closed but email not sent", "Closed after email sent"
IsOpen - either 0 or 1. Use this for reporting on all "Open" or "Not open" records

Thus there will be only ONE Status column in [ClosedOrders] but you can join to an [OrderStatus] table to see the Name and/or IsOpen state columns

I expect it is either a "don't care" or "too unlikely to happen", but there is an opportunity for a new ClosedOrder to be added between:

SELECT TOP 1
                ...
        FROM    ClosedOrders
        WHERE   status = 'Closed'
                AND email = @UserEmail;

and

        UPDATE  ClosedOrder
        SET     lsOpen = 1
        WHERE   lsOpen = 0
                AND email = @UserEmail

such that the user would not get an email for the later Closed Order. If that IS important I would do the update and generate the Message all-in-one statement like this:

BEGIN TRANSACTION

UPDATE  ClosedOrder
SET     lsOpen = 1,
                @email = Email ,
                @message = '

This is a computer generated email message.
Please DO NOT use the REPLY button above to respond to this email.
	    Dear ' + InitiatedBy + ':

	    Thank you for using the order processing system.

            Please click the link below to complete a survey

	    http://survey.php?wo= ' + ID + '

	    Regards, 
       Order administrator.' 
WHERE   lsOpen = 0
        AND email = @UserEmail
-- add this here?    AND Status = 'Closed'
-- and this?         AND InitiatedBy IS NOT NULL
-- and this?         AND ID IS NOT NULL
-- Check for and abort if error detected here 

EXEC msdb.dbo.sp_send_dbmail @profile_name = 'Feedback', -- our defined email profile or static info
            @recipients = @email, @subject = 'Feedback Survey',
            @body = @message;
-- Check for and abort if error detected here 
COMMIT

#17

Kristen, great point on a few things.

First, you are absolutely right about the WHERE clause, where status='Closed'.

I should not have used it at all.

The initial reason to use @email = @email + ';' + Email was because we wanted code to have the ability to send email to several users at one.

However, since that idea has been scrapped, it means sense to just use @email=email.

About this:@message = @message

I actually tried your suggestion of using @message='This is a computer generated message...' but for some reason it didn't work. I will try it again shortly.

You asked:

What if [InitiatedBy] or [ID] are NULL?

This is how the table is setup.

An update is made in another table in another server.

We grab those updates where status='Closed' and dump them into ClosedOrders table.

Pretty much all fields in this table are required.

So, there is very, very little chance that InitiatedBy or ID is null but I will make your recommended change.

You said:

' + ID + '

to


' + CONVERT(varchar(20), ID) + '

This is a varchar datatype.

Thanks for all of those useful tips


#18

I thought that was probably the case. Obvious won't work if the EMail contains some data specific to the person and I guess that is where you are now :slight_smile:

I would suggest that if you did want to send an email to multiple people you should use the BCC field, not the TO field, otherwise all recipients will see the email addresses of all others; that also leads to the risk that if one recipient has a virus etc. that could "harvest" the address of all the others from the TO (or CC) field in the Email.

That said I think there is benefit in sending the emails "singly". although its a pain to have to LOOP round generating each one, I think you will find it easier to debug any BOUNCE messages. Also if you want to change the link to

'http://survey.php?wo=' + ID + '&MySecretTrackingCode=' + MyCode

you can do that, which then enables you to see how many people clicked-through [that specific link], so even if the content of the email is not user-specific you might want some user-specific tracking codes in it. Well ... "we do" :slight_smile: ... although I always think its a bit sneaky ...


#19

Thanks again Kristen.

Here are a couple of more teasers for you.

1, I did try your suggestion of including the message as part of the query.

I came back as blank when email is fired.
so, I reverted to the way I had it before.
2, Looking the way you framed the variables on the url:

'http://survey.php?wo=' + ID + '&MySecretTrackingCode=' + MyCode

What happens, say the myCode contains values with spaces, how do you handle that?

In my case, I handled it the following way, using REPLACE

'http://survey.php?wo=' + ID + '&MySecretTrackingCode=' + MyCode+'&loc='+Replace(@loc,' ', '%20')+'
It seems to work but not sure if there is a better way.


#20

I would change spaces to "%20" as you have done. You might find that "+" also works if you prefer that (the URL will look "prettier")

Or if the space is there because the value is actually two separate pieces of information I would do

'&key1=' + Value1 + '&key2=' + Value 2

for us the MySecretTrackingCode is a (big!) integer, allocated in a database table ("LinksToEMailXReference") to be unique to the email, and also the location of the link within that email. So we are interested, for example, whether users only click links at the top of the email, or whether they also click links towards the bottom of the email / where they have to scroll.

If we want to know how many people clicks on the 3rd link we have to interrogate the LinksToEMailXReference table to find all the unique IDs used on emails for the 3rd link in that email run, and then how many of those were actually clicked on

I've lifted the link from a marketing email in my inbox (at random), here it is:

http://email.example.com/r/XLVsq3T98d5gyNDsYrbTORc.htm

The link shows in my email here as "Click here to see COOL STUFF", the actual URL behind it has that tracking number "looking like an HTML file". I am sure that the "XLVsq3T98d5gyNDsYrbTORc" code is unique to me, AND unique to THAT url in THAT email ...

If you adopt that sort of practice then you never have a problem with spaces in your LINKS (but your EmailClickThroughTracking table might have quite a lot of rows in it!!!)

One of the values you are concatenating into the String must be NULL.

You can "fix" that using COALESCE() or ISNULL() like this:

SELECT TOP 1
                @email = Email ,
                @message = '

This is a computer generated email message.
Please DO NOT use the REPLY button above to respond to this email.
	    Dear ' 
+ COALESCE(InitiatedBy, '*** InitiatedBy IS NULL***')
+ ':

	    Thank you for using the order processing system.

            Please click the link below to complete a survey

	    http://survey.php?wo= ' 
+ COALESCE(ID, '*** ID  IS NULL***')
+ '

	    Regards, 
       Order administrator.' 
        FROM    ClosedOrders
        WHERE   status = 'Closed'
                AND email = @UserEmail;

if you want to check for both BLANK and NULL then use:

+ COALESCE(NullIf(RTrim(
    COALESCE(InitiatedBy, '*** InitiatedBy IS NULL***')
    ), ''), '*** InitiatedBy IS BLANK ***')

although you don't really want the email going out with those messages in it, so for debugging only.

Better to prevent any emails going out like that by adding conditions to your WHERE clause to prevent anything going out with NULL / Blank values:

                AND InitiatedBy IS NOT NULL
                AND ID IS NOT NULL
                AND RTrim(InitiatedBy) <> ''
                AND RTrim(ID) <> ''

but the problem then is that IF that happens the Email does not go, but you do not get any sort of message to tell you that something needs fixing.

You can have a report:

WHERE
       InitiatedBy IS NOT NULL
    OR ID IS NOT NULL
    OR RTrim(InitiatedBy) <> ''
    OR RTrim(ID) <> ''

or you can first get all the data, for the email run, into a #TEMP table and then any "goofy rows" you could report on (send an email to yourself with the details), delete those from the #TEMP table and then EMail only the rows left in the #TEMP table (i.e. which are "clean")