Please start any new threads on our new site at https://forums.sqlteam.com. We've got lots of great SQL Server experts to answer whatever question you can come up with.

 All Forums
 SQL Server 2000 Forums
 SQL Server Development (2000)
 TRIGGER IF BEGIN END

Author  Topic 

dprichard
Yak Posting Veteran

94 Posts
Posted - 2006-10-02 : 15:54:20
I have a trigger I am working on. I am trying to say if the request_status = 3 then send out an approved email, if status = 4 send out the denied email. The approved email is working, but the denied email is not.

Any help would be greatly appreciated.

Here is my code:

ALTER TRIGGER requestupdate
ON dbo.request
FOR UPDATE
AS
BEGIN
DECLARE @mailtoaddy NVARCHAR(128),
		@approvedbody NVARCHAR(MAX),
		@deniedbody NVARCHAR(MAX),
		@status INT;
SET @mailtoaddy = (SELECT employee.emp_email
FROM         INSERTED INNER JOIN
                      employee ON inserted.emp_id = employee.emp_id)
SET @approvedbody = 
N'<font size="2"><strong>TIME OFF REQUEST CONFIRMATION</strong></font><br><br>'+
(SELECT employee.emp_fname
FROM         INSERTED INNER JOIN
                      employee ON inserted.emp_id = employee.emp_id)+
N',<br><br> Your time off request has been approved.<br><br>Thank you and have a nice day!<br><br><strong>Management</strong>'
SET @deniedbody = 
N'<font size="2"><strong>TIME OFF REQUEST CONFIRMATION</strong></font><br><br>'+
(SELECT employee.emp_fname
FROM         INSERTED INNER JOIN
                      employee ON inserted.emp_id = employee.emp_id)+
N',<br><br> Your time off request has been denied.<br><br>Thank you and have a nice day!<br><br><strong>Management</strong>'
SET @status = (select request_status FROM INSERTED);
IF (@status = '3')
BEGIN
EXEC msdb.dbo.sp_send_dbmail
    @recipients = @mailtoaddy,
    @subject = 'TIME OFF REQUEST UPDATE',
    @body = @approvedbody,
	@body_format = 'HTML';
IF (@status = '4')
BEGIN
EXEC msdb.dbo.sp_send_dbmail
    @recipients = @mailtoaddy,
    @subject = 'TIME OFF REQUEST UPDATE',
    @body = @deniedbody,
	@body_format = 'HTML';
END
END
END

Michael Valentine Jones
Yak DBA Kernel (pronounced Colonel)

7020 Posts
Posted - 2006-10-02 : 17:55:06
Sending email is not the kind of thing you should be using a trigger for. You should send the email from somwhere else in your application.

Also, your trigger is not designed to handle more than one row in the result set.




CODO ERGO SUM
Go to Top of Page

dprichard
Yak Posting Veteran

94 Posts
Posted - 2006-10-02 : 19:40:07
I am trying to kick out an email on update or insert. There will never be a situation on this table where more than one record will be updated at a time.

I am pretty new to SQL. Only been working with it a few weeks. Where should I be putting this email info?
Go to Top of Page

Michael Valentine Jones
Yak DBA Kernel (pronounced Colonel)

7020 Posts
Posted - 2006-10-02 : 19:59:23
>>>I am trying to kick out an email on update or insert.

That's another problem with the trigger, since it is an UPDATE trigger and doesn't fire on INSERT.


>>>There will never be a situation on this table where more than one record will be updated at a time.

Famous last words. You should NEVER write a trigger with this assumption; it's alway wrong.


>>>Where should I be putting this email info?

I would suggest adding this to the stored procedures that do the inserts or updates. Or add it to your front-end application.






CODO ERGO SUM
Go to Top of Page

Kristen
Test

22859 Posts
Posted - 2006-10-03 : 01:07:17
Take note of what MVJ said:

"I am pretty new to SQL"

means you are going to have trouble with:

"There will never be a situation on this table where more than one record will be updated at a time"

At the very least put a check for COUNT(*) from INSERTED = 1 and use RAISERROR to bail out when it updates multiple rows.

Triggers need to happen very quickly, and without any external interaction. You can water down either of those ideals, of course, but it would be wise to try not to!

If you cannot see a way to adding the emailing logic "upstream" then you could add the Email to a table of "emails to send" and have a scheduled task run every few minutes that grabs a row from the "ToBeSent" Email table, sends the email, then deletes the row. (And if people start getting 1,000 emails an hour you can then change the emailing task to amalgamate emails to the same person.

Indenting your code would probably help you with the problem with your Logic:

IF (@status = '3')
BEGIN
	EXEC msdb.dbo.sp_send_dbmail
	    @recipients = @mailtoaddy,
	    @subject = 'TIME OFF REQUEST UPDATE',
	    @body = @approvedbody,
		@body_format = 'HTML';
	IF (@status = '4')
	BEGIN
		EXEC msdb.dbo.sp_send_dbmail
		    @recipients = @mailtoaddy,
		    @subject = 'TIME OFF REQUEST UPDATE',
		    @body = @deniedbody,
			@body_format = 'HTML';
	END
END

and you will see that @status = '4' only happens within the @status = '3' section - which clearly will never be executed!

You can combine your 4 SET statements into one, which will be a lot more efficient. SQL is about doing things in "Sets", if you are used to programming in sequential-languages this is a big mind-shift.

SELECT	@mailtoaddy = employee.emp_email,
	@approvedbody = N'<font size="2"><strong>TIME OFF REQUEST CONFIRMATION</strong></font><br><br>'
			+ employee.emp_fname
			+ N',<br><br> Your time off request has been approved.<br><br>Thank you and have a nice day!<br><br><strong>Management</strong>',
	@deniedbody = N'<font size="2"><strong>TIME OFF REQUEST CONFIRMATION</strong></font><br><br>'
			+ employee.emp_fname
			+ N',<br><br> Your time off request has been denied.<br><br>Thank you and have a nice
day!<br><br><strong>Management</strong>',
	@status = inserted.request_status

and once you've got to that point you can then have:

SELECT	@mailtoaddy = employee.emp_email,
	@body = CASE 
		WHEN inserted.request_status = '3'
			THEN N'<font size="2"><strong>TIME OFF REQUEST CONFIRMATION</strong></font><br><br>'
			+ employee.emp_fname
			+ N',<br><br> Your time off request has been approved.<br><br>Thank you and have a nice day!<br><br><strong>Management</strong>'
		WHEN inserted.request_status = '4'
			THEN N'<font size="2"><strong>TIME OFF REQUEST CONFIRMATION</strong></font><br><br>'
			+ employee.emp_fname
			+ N',<br><br> Your time off request has been denied.<br><br>Thank you and have a nice
day!<br><br><strong>Management</strong>'
		END
FROM ...
WHERE	inserted.request_status IN ('3', '4')

EXEC msdb.dbo.sp_send_dbmail
    @recipients = ...

and you could easily change that to a style of:

INSERT INTO EMailWaitingToBeSent(recipients, subject, body, body_format)
SELECT	employee.emp_email,
	'TIME OFF REQUEST UPDATE',
	CASE 
		WHEN inserted.request_status = '3'
			THEN ...
		WHEN inserted.request_status = '4'
			THEN ...
		END,
	'HTML'
FROM	INSERTED
	INNER JOIN ...

and it will work for however many records happen to be updated in the trigger - in the future!

What is msdb.dbo.sp_send_dbmail? Unless that's some newfangled MS System Procedure in SQL2005 it seems a bit scary that a user-SProc has been added to msdb ...

Kristen
Go to Top of Page

dprichard
Yak Posting Veteran

94 Posts
Posted - 2006-10-03 : 11:53:31
Kristen,

I wasn't trying to dispute any info from the other replys, I am just new and trying to understand the why's behind what I am doing versus the steps so I have a better understanding. I appreciate your post, but it went way over me so please be patient if I am bugging you with all my newbie questions.

This is where I got the information on the sp_send_dbmail from:

http://msdn2.microsoft.com/en-us/library/ms190307.aspx

As far as this part of your post:

quote:
If you cannot see a way to adding the emailing logic "upstream" then you could add the Email to a table of "emails to send" and have a scheduled task run every few minutes that grabs a row from the "ToBeSent" Email table, sends the email, then deletes the row. (And if people start getting 1,000 emails an hour you can then change the emailing task to amalgamate emails to the same person.


When you say add email upstream, I am not sure what you mean (Cause I am new). What I am trying to get my app to do is the user submits a request for time off and it goes into the request table. I need to email the user saying it was done successfully and email the supervisor telling them there is a new request. I setup a similar trigger for the insert that does this. Then when they supervisor updates the request, I need to kick an email back saying whether it was approved or denied. I am doing this with the triggers right now cause I posted a few times trying to figure out how best to do this with no response and this was the only way I could figure out how.

So, it seems I steered my ship in the wrong direction and I want to correct it, but I just want to understand this a little better.

Thank in advance for any help!!!!!!!!!!
Go to Top of Page

Kristen
Test

22859 Posts
Posted - 2006-10-03 : 12:02:02
"When you say add email upstream"

Sorry, I meant "before you get to the trigger" - so that would be Stored Procedure, or Application, most likely.

"but it went way over me"

As you said you were new to SQL I tried to break it down into steps ... so fire away with any questions you may have.

Kristen
Go to Top of Page

dprichard
Yak Posting Veteran

94 Posts
Posted - 2006-10-03 : 13:01:00
So, I have a stored procedure for the insert statement right now. You are saying that I should put the Email part in the stored procedure with the insert?
Go to Top of Page

dprichard
Yak Posting Veteran

94 Posts
Posted - 2006-10-03 : 13:31:24
Okay, I am reading about jobs and stored procedures and am wondering if this is what you are talking about.

Setup a stored procedure to send out the email then setup a scheduled task to hit the stored procedure say once a day or once an hour to send them out?
Go to Top of Page

Kristen
Test

22859 Posts
Posted - 2006-10-03 : 14:03:48
"You are saying that I should put the Email part in the stored procedure with the insert?"

Your application will, most probably, talk to SQL Server either through a Stored Procedure or by using Dynamic SQL (SQL embedded in the application)

If you use a Stored Procedure AND all such inserts go through that SProc, its quite a nice place to trigger the Email from.

However, you will be triggering the Email ON THE SQL BOX and not on, for example, the Web Box / Client. For us this would be a no-no as we have very tight controls on our SQL boxes and sending bulk email is not something we allow.

So for us we would have to put the Email details into a table and have something else interrogate that table periodically and send the emails out - in our case a little VB application that just requests data from the EmailToBeSent table, and send the email out.

Now, if YOU CAN send Emails from your SQL box you can either do them in the SProc that inserts the row, or by a batch job.

So as to your second point you could schedule a stored procedure to run once an hour and send the Emails.

Or you could schedule a VB Task (or similar) on a different server to do the emails.

One of the potential benefits of sending them in batches is that all types of emails can be sent by a single procedure - its just got to read the Recipient/Subject/Message from the table, send the email, and then flag the record as "done".

So if you discovered you needed to change your Email process somehow - log that they were sent, change the SMTP server, BlindCarbon the office manager, whatever, then there is only one Sproc responsible for sending emails and changing it is centralised. You could even easily change it from using an SProc to send them on the SQL box to having a little VB Application on some other box.

Kristen
Go to Top of Page

dprichard
Yak Posting Veteran

94 Posts
Posted - 2006-10-03 : 15:06:25
This is the stored procedure I am using for the insert statement:

ALTER PROCEDURE dbo.insNewRequest @emp_id bigint, @request_start_date datetime, @request_end_date datetime, @request_duration int, @request_notes text, @time_off_id bigint
	/*
	(
	@parameter1 int = 5,
	@parameter2 datatype OUTPUT
	)
	*/
AS
	/* SET NOCOUNT ON */ 
	INSERT		request
		(
			emp_id,
			request_submit_date, 
			request_start_date,
			request_end_date,
			request_duration,
			request_notes,
			time_off_id
		)
Select		@emp_id,
		GETDATE(),
		@request_start_date,
		@request_end_date, 
		1 + (DATEDIFF(day, @request_start_date, @request_end_date)) - (select count(*) from WeekEndsAndHolidays where DayOfWeekDate between @request_start_date and @request_end_date),
		@request_notes,
		@time_off_id;


I tried to mixing the two without much luck. It didn't like the FROM INSERTED. It is telling me

Invalid object name 'INSERTED'.
Go to Top of Page

Kristen
Test

22859 Posts
Posted - 2006-10-03 : 15:20:08
Yup, the INSERTED pseudo table is only available in a trigger.

I reckon that, after your INSERT statement, you need to add something like this:

SELECT	@mailtoaddy = employee.emp_email,
	@body = CASE 
		WHEN request.request_status = '3'
			THEN N'<font size="2"><strong>TIME OFF REQUEST CONFIRMATION</strong></font><br><br>'
			+ employee.emp_fname
			+ N',<br><br> Your time off request has been approved.<br><br>Thank you and have a nice day!<br><br><strong>Management</strong>'
		WHEN request.request_status = '4'
			THEN N'<font size="2"><strong>TIME OFF REQUEST CONFIRMATION</strong></font><br><br>'
			+ employee.emp_fname
			+ N',<br><br> Your time off request has been denied.<br><br>Thank you and have a nice
day!<br><br><strong>Management</strong>'
		END
FROM	request
	INNER JOIN employee
		ON request.emp_id = employee.emp_id
WHERE	request.request_status IN ('3', '4')
	AND request.emp_id = @emp_id

EXEC msdb.dbo.sp_send_dbmail
    @recipients = ...

Kristen
Go to Top of Page

dprichard
Yak Posting Veteran

94 Posts
Posted - 2006-10-03 : 16:04:17
I can't find info on WHEN and the way you use it here. If you were trying to find more info on that what would you google. I want to read up on that please!

Okay, so this is what I am using now, but it looks like the @mailtoaddy = employee.emp_email isn't picking up the information.

I get the following error:

At least one of the following parameters must be specified. "@recipients, @copy_recipients, @blind_copy_recipients".

The way I was doing it before was assigning the @mailaddy = (SELECT STATEMENT)


ALTER PROCEDURE dbo.insNewRequest @emp_id bigint, @request_start_date datetime, @request_end_date datetime, @request_duration int, @request_notes text, @time_off_id bigint
	/*
	(
	@parameter1 int = 5,
	@parameter2 datatype OUTPUT
	)
	*/
AS
	/* SET NOCOUNT ON */ 
	INSERT		request
		(
			emp_id,
			request_submit_date, 
			request_start_date,
			request_end_date,
			request_duration,
			request_notes,
			time_off_id
		)
Select		@emp_id,
		GETDATE(),
		@request_start_date,
		@request_end_date, 
		1 + (DATEDIFF(day, @request_start_date, @request_end_date)) - (select count(*) from WeekEndsAndHolidays where DayOfWeekDate between @request_start_date and @request_end_date),
		@request_notes,
		@time_off_id;
		
DECLARE 
@mailtoaddy NVARCHAR(128),
@body NVARCHAR(MAX);
SELECT	@mailtoaddy = employee.emp_email,
	@body = CASE 
		WHEN request.request_status = '3'
			THEN N'<font size="2"><strong>TIME OFF REQUEST CONFIRMATION</strong></font><br><br>'
			+ employee.emp_fname
			+ N',<br><br> Your time off request has been approved.<br><br>Thank you and have a nice day!<br><br><strong>Management</strong>'
		WHEN request.request_status = '4'
			THEN N'<font size="2"><strong>TIME OFF REQUEST CONFIRMATION</strong></font><br><br>'
			+ employee.emp_fname
			+ N',<br><br> Your time off request has been denied.<br><br>Thank you and have a nice
day!<br><br><strong>Management</strong>'
		END
FROM	request
	INNER JOIN employee
		ON request.emp_id = employee.emp_id
WHERE	request.request_status IN ('3', '4')
	AND request.emp_id = @emp_id

EXEC msdb.dbo.sp_send_dbmail
    @recipients = @mailtoaddy,
    @subject = 'EMPLOYEE VACATION REQUEST NOTIFICATION',
    @body = @body,
    @body_format = 'HTML';
Go to Top of Page

dprichard
Yak Posting Veteran

94 Posts
Posted - 2006-10-03 : 16:05:59
Please disreguard the part about WHEN. I typed "SELECT CASE WHEN THEN" in google and found info on that.
Go to Top of Page

Kristen
Test

22859 Posts
Posted - 2006-10-04 : 01:43:00
Yup, its "CASE" you are after, rather than "WHEN". Glad you found it!

Note that I took your email logic from your UPDATE trigger, and this is your INSERT SProc - in particular it is testing request.request_status to decide which type of email to send, and that column is NOT part of the Insert statement - so will only ever be set to the default.

I guess that instead of that you need to be sending the "You time off request is being considered" to the employee and "Please consider this request" to the manager?

Kristen
Go to Top of Page

dprichard
Yak Posting Veteran

94 Posts
Posted - 2006-10-04 : 16:28:30
Duh, I am sorry. I didn't even notice I was combining the two.

This is my final result for the insert. Can you tell me if this looks okay and if I am going to run into any issues doing it this way? I ended up selecting the top one ordered by reqeuest desc because it was dying cause it was grabbing every row with the employee id and it was saying I couldn't do more than one result.

ALTER PROCEDURE dbo.insNewRequest @emp_id bigint, @request_start_date datetime, @request_end_date datetime, @request_duration int, @request_notes text, @time_off_id bigint

AS
	INSERT		request
		(
			emp_id,
			request_submit_date, 
			request_start_date,
			request_end_date,
			request_duration,
			request_notes,
			time_off_id
		)
Select		@emp_id,
		GETDATE(),
		@request_start_date,
		@request_end_date, 
		1 + (DATEDIFF(day, @request_start_date, @request_end_date)) - (select count(*) from WeekEndsAndHolidays where DayOfWeekDate between @request_start_date and @request_end_date),
		@request_notes,
		@time_off_id;
		
DECLARE @mailtoaddy NVARCHAR(128),
		@body NVARCHAR(MAX);
SET @mailtoaddy = (SELECT TOP (1)employee.emp_email
FROM         request INNER JOIN
                      employee ON request.emp_id = employee.emp_id
WHERE request.emp_id = @emp_id
ORDER BY request.request_id DESC)
SET @body = 
N'<font size="2"><strong>TIME OFF REQUEST CONFIRMATION</strong></font><br><br>'+
(SELECT TOP (1) employee.emp_fname
FROM         request INNER JOIN
                      employee ON request.emp_id = employee.emp_id
WHERE request.emp_id = @emp_id
ORDER BY request.request_id DESC)+
N' You have successfully submitted your time off request.  You will receive another confirmation once the request has been reviewed<br><br>Thank you and have a nice day!<br><strong>Management</strong>';

EXEC msdb.dbo.sp_send_dbmail
    @recipients = @mailtoaddy,
    @subject = 'EMPLOYEE TIME OFF REQUEST CONFIRMATION',
    @body = @body,
	@body_format = 'HTML';
Go to Top of Page

Kristen
Test

22859 Posts
Posted - 2006-10-05 : 01:49:14
Couple of things:

You have abandoned the style I had which only queries the EMPLOYEE once, you are doing the same query for each SET command.

I JOINed the EMPLOYEE and REQUEST table to get the request status - as I said I was doing that from your UPDATE logic. You don't need that at all for how you now have it - there is NO test on any request columns, so you just want:

FROM employee WHERE employee.emp_id = @emp_id

It is important that you understand why this was wrong, so please ask if its not clear - otherwise you will be blindly following rather than understanding what you are doing, and then I won't have been of any real help!!

Have a look at this and see if it makes sense:

SELECT	@mailtoaddy = employee.emp_email,
	@body = N'<font size="2"><strong>TIME OFF REQUEST CONFIRMATION</strong></font><br><br>'
		+ employee.emp_fname
FROM	employee
WHERE	employee.emp_id = @emp_id

Kristen
Go to Top of Page

dprichard
Yak Posting Veteran

94 Posts
Posted - 2006-10-05 : 07:38:38
Oh, I absolutely see what you are saying. I was overcomplicating it.

SELECT	@mailtoaddy = employee.emp_email,
	@body = N'<font size="2"><strong>TIME OFF REQUEST CONFIRMATION</strong></font><br><br>'
		+ employee.emp_fname + N',<br><br>This email is to confirm you have successfully submitted your time off request.  You will be sent an update email after your request has been approved.  <br><br>Thank you and have a nice day!<br><strong>Management</strong><br>'
FROM	employee
WHERE	employee.emp_id = @emp_id


Thank you so so so so so much for all your help with this!

Go to Top of Page

Kristen
Test

22859 Posts
Posted - 2006-10-05 : 13:28:03
Hehehe ... you got it! Move on to the next problem!!
Go to Top of Page
   

Subscribe to SQLTeam.com

SQLTeam.com Articles via RSS

SQLTeam.com Weblog via RSS

- Advertisement -

Resources

Articles

Forums

Blogs

Contact Us

About the Site