First Attempt At Stored Procedure - Can Anyone Offer Advice

SQL SERVER 2000

Hi all

This is my first attempt at writing a stored procedure. I have managed to
get it working but its unlikely to be the best way of handling the problem.
While writing it I found some things that I don't understand so if any one
could shed any light it would be much appreciated. I have posted these at
the end.

Sorry about the length but I thought it might be worthwhile posting the code

The purpose of the procedures is as follows : we have a view of lots of bits
of information that need automatically mailing to different people. each
element of information has a name allocated against it. If we had 100 pieces
of data, 50 could go to manager 1 25 could go to manager 2 and 25 to manager
3 etc...

Both SP's look at the same view

The first SP generates a distinct list of managers and for each manager
calls the second SP

The second SP filters the view for the data belonging to the selected
manager, and builds an HTML mail. It then sends all the bits of information
belonging to that manager off in an EMAIL to him/her. ( It uses a brilliant
bit of code from sqldev.net to handle the mail)

the first mail then repeats for all the managers in the list

CODE ---- SP 1
ALTER PROCEDURE dbo.PR_ADMIN_CLIENT_WEEKLY_NOTIFICATION_2
AS
begin
SET NOCOUNT ON
declare @no_of_managers as int
declare @current_record as int
declare @manager_name as varchar(100)

-- count how many distinct managers we need to send the mail to
select @no_of_managers = COUNT(DISTINCT manager_name) FROM
dbo.vw_client_notification_email_1

-- open a cursor to the same distinct list
declare email_list cursor for select distinct manager_name from
dbo.vw_client_notification_email_1 dsc
open email_list

-- for each distinct manager get the managers name and pass it to the stored
procedure that generates the mail.
set @current_record = 0
while (@current_record) < @no_of_managers
begin
fetch next from email_list into @manager_name
EXECUTE dbo.pr_admin_client_weekly_notification @manager_name
set @current_record = @current_record+1
end
-- close the cursor
close email_list
deallocate email_list
end



CODE ---- SP2
ALTER PROCEDURE dbo.PR_ADMIN_CLIENT_WEEKLY_NOTIFICATION
(@current_manager_name as varchar(100))
-- a unique managers name is passed from the calling procedure
as begin
SET NOCOUNT ON
-- declarations for use in the stored procedure
DECLARE @to as varchar(100)
DECLARE @entry varchar(500)
DECLARE @region as varchar(100)
DECLARE @type as varchar(100)
DECLARE @site_ref as varchar(100)
DECLARE @aborted as varchar(100)
DECLARE @weblink as varchar(1000)
DECLARE @manager_name as varchar(100)
DECLARE @manager_email as varchar(100)
DECLARE @body VARCHAR(8000)
DECLARE @link varchar(150)
DECLARE @web_base VARCHAR(150)

-- set up a connection to the view that contains the details for the mail

DECLARE email_contents cursor for select region,type,
site_ref,aborted_visit,link,manager_name,manager_e mail from
vw_client_notification_email_1 where manager_name = @current_manager_name
open email_contents
--some initial text
set @body = '<font color="#FF8040"><b>Reports W/E ' +convert(char(50),
getdate()) + '</b></font><br><br> <a href = http://xxxx > Click here to log
on to xxxxx </a><br><br> '
--fetch the first matching record from the table and build the body of the
message
fetch next from email_contents into
@region,@type,@site_ref,@aborted,@link,@manager_na me,@manager_email
set @web_base = 'http://'
set @weblink = @web_base + @link
if @aborted = 0 set @aborted = '' else set @aborted = 'ABORTED'
set @body = @body + '<font size="2"><b> Region </b>' + @region
+ ' <b>Type</b> ' + @type
+ ' <b>Site Reference </b> <a href = "' + @weblink + '">' + @site_ref+
'</a>'
+ ' <b>Unique Report Reference </b>' + @link + '<br>'

-- continue reading the records for this particular message and adding on to
the body of the text
while(@@fetch_status = 0)
begin
fetch next from email_contents into
@region,@type,@site_ref,@aborted,@link,@manager_na me,@manager_email
if @aborted = 0 set @aborted = '' else set @aborted = 'ABORTED'
if (@@fetch_status = 0) set @body = @body + '<b> Region </b>' + @region
+ ' <b>Type</b> ' + @type
+ ' <b>Site Reference </b> <a href = "' + @weblink + '">' + @site_ref+
'</a>'
+ '<b>Unique Report Reference </b>' + @link + '<br>'
end

-- close the cursor
set @body = @body + '</font>'
close email_contents
deallocate email_contents
-- generate the mail
DECLARE @rc int EXEC @rc = master.dbo.xp_smtp_sendmail
@FROM = N'FROM ME',
@TO = @manager_email,
@server = N'server',
@subject = N'Weekly Import',
@message = @body,
@type = N'text/html'


end






Questions

is the way I've done it OK. I thought I would be able to do it in a single
SP but I really struggled nesting the cursor things.

@@fetchstatus seems to be global, so if your using nested cursors, how do
you know which one you are refering to. If you have multiple calls to the
same SP how does it know which instance of the SP it refers to.

When I first wrote it, I used a cursor in SP1 to call SP2, but I couldn't
get the while loop working - I have a feeling it was down to the @@
fetchstatus in the 'calling' procedure being overwritten by the
@@fetchstatus in the 'called' procedure.

The whole @@fetchatus thing seems a bit odd. In the second procedure, I have
to fetch, then check, manipulate then fetch again, meaning that the same
manipulation code is written twice. thats why in the first procedure I used
the select distint count to know how long the record set is so I only have
to run the manipulation code once. Is what I have done wrong?

its possible that the body of the mail could be > 8K, is there another
datatype I can use to hold more than 8K


many thanks for any help or advice

Andy