C2025-027-AnnotationsTypeSelect #574

Merged
jjenko merged 10 commits from C2025-027-AnnotationsTypeSelect into Development 2025-07-30 08:23:07 -04:00
13 changed files with 2286 additions and 1277 deletions
Showing only changes of commit d3888e3c32 - Show all commits

View File

@@ -93,8 +93,8 @@ namespace VEPROMS
// Save selected list to DB.
private void btnUpdate_Click(object sender, EventArgs e)
{
DataTable dt2 = coverToTable(UserID);
VEPROMS.CSLA.Library.AnnotationstypeSelections.Update(dt2);
DataTable dt2 = coverToTable();
VEPROMS.CSLA.Library.AnnotationstypeSelections.Update(dt2, UserID);
}
public class AnnotataionItem
{
mschill marked this conversation as resolved Outdated

I am not sure what this flag is?
I noticed it will be 1 on 1st iteration then 0 after that.

Can there be comments explaining why?

Flag removed

I am not sure what this flag is? I noticed it will be 1 on 1st iteration then 0 after that. Can there be comments explaining why? Flag removed

Must be missing something .... Not sure I understand why we are deleting items and reentering them ...... wouldn't we lose history on when they were originally put in if we do that?

When a user changes or adds to his selections, we need to remove his selections in the DB and insert the updates.

Must be missing something .... Not sure I understand why we are deleting items and reentering them ...... wouldn't we lose history on when they were originally put in if we do that? When a user changes or adds to his selections, we need to remove his selections in the DB and insert the updates.

Delete and reinsert works very well. Updates in this scenario are nightmares.

Delete and reinsert works very well. Updates in this scenario are nightmares.

not sure if this would help --- but just a thought - passing in a whole table of the annotationtypeselections instead of doing 1 update/insert at a time and then doing a merge with the current table?

In the end, as long as Devin and John are good with it, I am too --- just figured I would check in case we were losing any needed info by removing and re-adding...

not sure if this would help --- but just a thought - passing in a whole table of the annotationtypeselections instead of doing 1 update/insert at a time and then doing a merge with the current table? In the end, as long as Devin and John are good with it, I am too --- just figured I would check in case we were losing any needed info by removing and re-adding...

Would remove dltFlg and commented out code since no longer used.

Would remove dltFlg and commented out code since no longer used.

As FYI - still valid

As FYI - still valid

Flag removed

Flag removed
@@ -145,33 +145,27 @@ namespace VEPROMS
lstSelected.DisplayMember = "NameStr";
lstSelected.ValueMember = "TypeID";
DataTable lstSelectedTbl = VEPROMS.CSLA.Library.AnnotationstypeSelections.Retrieve(UserID);
if (lstSelectedTbl.Rows.Count > 0)
{
mschill marked this conversation as resolved
Review

I closed this previous comment since it moved down and reposted here - "Shouldn't need this as foreach loop below should loop 0 times if no rows."

This comment moved down thanks to changes since --- this is referring to the if statement now on line 148 - if (lstSelectedTbl.Rows.Count > 0)
since next statement is
foreach (DataRow lstSelectedRow in lstSelectedTbl.Rows)

I closed this previous comment since it moved down and reposted here - "Shouldn't need this as foreach loop below should loop 0 times if no rows." This comment moved down thanks to changes since --- this is referring to the if statement now on line 148 - if (lstSelectedTbl.Rows.Count > 0) since next statement is foreach (DataRow lstSelectedRow in lstSelectedTbl.Rows)
foreach (DataRow lstSelectedRow in lstSelectedTbl.Rows)
{
lstSelected.Items.Add(new AnnotataionItem(lstSelectedRow["Name"].ToString(), (int)lstSelectedRow["TypeID"]));
}
}
}
private void btnCancel_Click_1(object sender, EventArgs e)
{
this.Close();
}
private DataTable coverToTable(string userid)
private DataTable coverToTable()
{
int RowID = 0;
DataTable dt = new DataTable();
dt.Columns.Add("TypeID", typeof(Int32));
dt.Columns.Add("NameStr", typeof(string));
dt.Columns.Add("UserID", typeof(string));
dt.Columns.Add("RowID", typeof(string));
mschill marked this conversation as resolved Outdated

Shouldn't need this as foreach loop below should loop 0 times if no rows.

Shouldn't need this as foreach loop below should loop 0 times if no rows.

That look is used. It retrieves from AnnotationstypeSelections table.

That look is used. It retrieves from AnnotationstypeSelections table.
foreach (AnnotataionItem item in lstSelected.Items.OfType<AnnotataionItem>())
{
mschill marked this conversation as resolved Outdated

I thought Name was being removed from AnntationsSelections and just going to be pulled from AnnotationsType?

Also, not quite sure why RowID is needed / was added?
Should only need the TypeID and UserID unless I am missing something?

I thought Name was being removed from AnntationsSelections and just going to be pulled from AnnotationsType? Also, not quite sure why RowID is needed / was added? Should only need the TypeID and UserID unless I am missing something?
++RowID;
dt.Rows.Add(item.TypeID, item.NameStr, userid, RowID);
dt.Rows.Add(item.TypeID);
}
return dt;
}
mschill marked this conversation as resolved
Review

see comments in SQL --- shouldn't need RowID

see comments in SQL --- shouldn't need RowID

View File

@@ -24074,10 +24074,6 @@ ELSE
GO
-- C2025-027 Annotation Type Filtering
IF EXISTS (SELECT * FROM sys.objects WHERE object_id = OBJECT_ID(N'[dbo].[AnnotationTypeSelections]') AND type in (N'U'))
DROP TABLE [dbo].[AnnotationTypeSelections]
@@ -24095,28 +24091,36 @@ GO
-- Create date: 07/10/2025
-- Description: Store user Annotation selections for annotation filter.
-- =============================================
IF NOT EXISTS (SELECT * FROM sys.objects WHERE object_id = OBJECT_ID(N'[dbo].[AnnotationTypeSelections]') AND type in (N'U'))
BEGIN
CREATE TABLE [dbo].[AnnotationTypeSelections](
[ASTypeID] [int] IDENTITY(1,1) NOT NULL,
[TypeID] [int] NULL,
[UsrID] [varchar](50) NULL,
[Name] [nvarchar](100) NULL,
[Config] [nvarchar](max) NULL,
[DTS] [datetime] NULL,
[UserID] [nvarchar](100) NULL,
[LastChanged] [timestamp] NULL
) ON [PRIMARY] TEXTIMAGE_ON [PRIMARY]
[UserID] [varchar](50) NULL,
[LastChanged] [datetime] NULL,
CONSTRAINT [PK_AnnotationTypeSelections] PRIMARY KEY CLUSTERED
([ASTypeID] ASC)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
mschill marked this conversation as resolved Outdated

I could be wrong but I thought some of these were being removed and joins done to the AnnotationType table to get things like Name, IsEpAnnotationType, etc...?

I could be wrong but I thought some of these were being removed and joins done to the AnnotationType table to get things like Name, IsEpAnnotationType, etc...?

This is still valid.

I thought Name was being removed from AnntationsSelections and just going to be pulled from AnnotationsType?

Also, not sure what Config is for here?

This is still valid. I thought Name was being removed from AnntationsSelections and just going to be pulled from AnnotationsType? Also, not sure what Config is for here?
) ON [PRIMARY]
END
mschill marked this conversation as resolved
Review

Would suggest not having columns named both UserID and UserID in the same table as it is confusing which is the function of each,
Would suggest renaming UserID as something like CreatedBy.

Would suggest not having columns named both UserID and UserID in the same table as it is confusing which is the function of each, Would suggest renaming UserID as something like CreatedBy.
IF OBJECT_ID('DF_AnnotationTypeSelections_LastChanged', 'D') IS NULL
ALTER TABLE AnnotationTypeSelections ADD CONSTRAINT [DF_AnnotationTypeSelections_LastChanged] DEFAULT (getdate()) for [LastChanged];
GO
IF EXISTS (SELECT * FROM sys.indexes WHERE name='idx_AnnotationTypeSelections_UsrID'
IF EXISTS (SELECT * FROM sys.indexes WHERE name='idx_AnnotationTypeSelections_UserIDTypeID'
AND object_id = OBJECT_ID('[dbo].[AnnotationTypeSelections]'))
mschill marked this conversation as resolved Outdated

if commented out, should be removed or reason given why commented out?

if commented out, should be removed or reason given why commented out?

Comment removed.

Comment removed.
begin
DROP INDEX [idx_AnnotationTypeSelections_UsrID] ON [dbo].[AnnotationTypeSelections];
DROP INDEX [idx_AnnotationTypeSelections_UserIDTypeID] ON [dbo].[AnnotationTypeSelections];
end
CREATE NONCLUSTERED INDEX idx_AnnotationTypeSelections_UsrID
ON [dbo].[AnnotationTypeSelections] (UsrID)
INCLUDE (TypeID, Name)
CREATE UNIQUE INDEX [idx_AnnotationTypeSelections_UserIDTypeID] ON [dbo].[AnnotationTypeSelections]
(
[UserID] ASC,
mschill marked this conversation as resolved Outdated

remove:

, OPTIMIZE_FOR_SEQUENTIAL_KEY = OFF

This was what we caught was not compatible with SQL 2016

remove: , OPTIMIZE_FOR_SEQUENTIAL_KEY = OFF This was what we caught was not compatible with SQL 2016
[TypeID] ASC
)
INCLUDE (ASTypeID)
mschill marked this conversation as resolved Outdated

Wouldn't you want the index on UsrID,TypeID?

I also thought name was being removed from this table?

see previous notes / wouldn;t this be UNIQUE by UsrID, TypeID

Wouldn't you want the index on UsrID,TypeID? I also thought name was being removed from this table? see previous notes / wouldn;t this be UNIQUE by UsrID, TypeID

Added an index.

Added an index.

Comment still valid:

index should be UNIQUE by
(1) UsrID, TypeID
or
(2) TypeID, UsrID
and INCLUDE other columns pulled back like:
ASTypeID

for whether it should be (1) or (2), I suggest trying each individually and looking at the execution plan to see which is grabbed / better --- if you need assistance with this, please let me know and I can give assistance as I am quite familiar with looking at the execution plans in SQL,

Comment still valid: index should be UNIQUE by (1) UsrID, TypeID or (2) TypeID, UsrID and INCLUDE other columns pulled back like: ASTypeID for whether it should be (1) or (2), I suggest trying each individually and looking at the execution plan to see which is grabbed / better --- if you need assistance with this, please let me know and I can give assistance as I am quite familiar with looking at the execution plans in SQL,

There is another comment around line 24235that is related to this. It just moved since there have been changes since that comment was made. That one should have likely the exact index needed.

There is another comment around line 24235that is related to this. It just moved since there have been changes since that comment was made. That one should have likely the exact index needed.
WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, SORT_IN_TEMPDB = OFF, DROP_EXISTING = OFF, ONLINE = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON, OPTIMIZE_FOR_SEQUENTIAL_KEY = OFF) ON [PRIMARY]
GO
-- C2025-027 Annotation Type Filtering
@@ -24145,7 +24149,7 @@ AS
(SELECT COUNT(*) FROM [Annotations] WHERE [Annotations].[TypeID]= [TypeID]) [AnnotationCount],
[IsEPAnnotationType]
FROM [AnnotationTypes] --A
WHERE TypeID NOT IN (SELECT TypeID FROM AnnotationTypeSelections WHERE UsrID = @UserID)
WHERE TypeID NOT IN (SELECT TypeID FROM AnnotationTypeSelections ATS WHERE ATS.UserID = @UserID)
GO
-- C2025-027 Annotation Type Filtering
@@ -24171,18 +24175,19 @@ AS
BEGIN
SELECT [ASTypeID]
,ATS.[TypeID]
,[UsrID]
,ATS.[UserID]
,AT.[Name]
,AT.[Config]
,ATS.[DTS]
,ATS.[LastChanged]
,AT.[UserID]
,AT.[IsEPAnnotationType]
FROM [dbo].[AnnotationTypeSelections] ATS
INNER JOIN AnnotationTypes AT ON AT.TypeID = ATS.TypeID
WHERE UsrID = @UsrID
WHERE ATS.UserID = @UsrID
END
GO
-- C2025-027 Annotation Type Filtering
IF EXISTS (SELECT * FROM dbo.sysobjects WHERE id = OBJECT_ID(N'[getAnnotationstypeFiltered]') AND OBJECTPROPERTY(id,N'IsProcedure') = 1)
DROP PROCEDURE [getAnnotationstypeFiltered];
@@ -24200,19 +24205,19 @@ CREATE PROC [dbo].[getAnnotationstypeFiltered]
)
AS
BEGIN
IF((SELECT count(TypeID) FROM AnnotationTypeSelections WHERE UsrID = @UsrID) > 0)
IF((SELECT count(TypeID) FROM AnnotationTypeSelections WHERE UserID = @UsrID) > 0)
BEGIN
SELECT [ASTypeID]
,ATS.[TypeID]
,[UsrID]
,ATS.[UserID]
,AT.[Name]
,AT.[Config]
,ATS.[DTS]
,ATS.[LastChanged]
,AT.[UserID]
,AT.[IsEPAnnotationType]
FROM [dbo].[AnnotationTypeSelections] ATS
INNER JOIN AnnotationTypes AT ON AT.TypeID = ATS.TypeID
WHERE UsrID = @UsrID
WHERE ATS.UserID = @UsrID
END
ELSE
BEGIN
mschill marked this conversation as resolved Outdated

Can name be different than the AnnotationTypes.Name?

Basically, if have a separate field for it -- what happens if someone
selects:
Annotation: TypeID = 5, Name="TEST1"
to be filtered
then
changes the Name of the Annotation Type to be "TEST3"
..
I believe then it would be "TEST3" in PROMS
but the option in the filtering list would say "TEST1"

Can name be different than the AnnotationTypes.Name? Basically, if have a separate field for it -- what happens if someone selects: Annotation: TypeID = 5, Name="TEST1" to be filtered then changes the Name of the Annotation Type to be "TEST3" .. I believe then it would be "TEST3" in PROMS but the option in the filtering list would say "TEST1"

The all annotation type names in the form are pulled from AnnotationTypes table.

The all annotation type names in the form are pulled from AnnotationTypes table.
@@ -24241,14 +24246,8 @@ IF EXISTS( SELECT * FROM INFORMATION_SCHEMA.DOMAINS WHERE Domain_Name = 'TableVa
DROP TYPE [dbo].[TableValAnnotTypeSelections]
CREATE TYPE [dbo].[TableValAnnotTypeSelections] AS TABLE(
[TypeID] [int] NOT NULL,
[NameStr] [varchar](200) NULL,
[UserID] [varchar](50) NULL,
[RowID] [int] NOT NULL,
PRIMARY KEY CLUSTERED
(
[RowID] ASC
)WITH (IGNORE_DUP_KEY = OFF)
[TypeID] [int] NOT NULL
mschill marked this conversation as resolved Outdated

shouldn't need a primary key on a type?

shouldn't need a primary key on a type?
)
GO
@@ -24261,28 +24260,30 @@ GO
-- =============================================
CREATE PROC [dbo].[UpdateAnnotationstypeSelections]
(
mschill marked this conversation as resolved Outdated

If commented out, would remove or give reason why commented out?

If commented out, would remove or give reason why commented out?

Removed it

Removed it
@TempTable AS dbo.TableValAnnotTypeSelections READONLY
@TempTable AS dbo.TableValAnnotTypeSelections READONLY,
@UserID [varchar](50) NULL
)
AS
BEGIN
DECLARE @cnt integer = 0
DECLARE @cnt2 integer = 0
SET @cnt = (SELECT count(*) from @TempTable)
DECLARE @UserID VARCHAR(50) = (SELECT TOP 1 UserID FROM @TempTable)
DELETE FROM AnnotationTypeSelections WHERE usrID = @UserID;
declare @i int
select @i = min(RowID) from @TempTable
declare @max int
select @max = max(RowID) from @TempTable
WHILE @i <= @max
BEGIN
INSERT INTO AnnotationTypeSelections (TypeID, Name, Usrid)
select TypeID, NameStr, UserID from @TempTable where RowID = @i
set @i = @i + 1
END
DELETE FROM AnnotationTypeSelections where UserID = @UserID
AND
TypeID not in
mschill marked this conversation as resolved Outdated

only need to delete the ones not already in the table. If we delete and re-add then will lose history of when truely added --- see comment below (around line 24278) for more complete,

only need to delete the ones not already in the table. If we delete and re-add then will lose history of when truely added --- see comment below (around line 24278) for more complete,
(Select TypeID From @TempTable tmp)
--this would insert all the ones that are in the uploaded table and not already in AnnotationTypeSelections
Insert INTO AnnotationTypeSelections (TypeID, UserID)
Select tmp.TypeID, @UserID
mschill marked this conversation as resolved Outdated

So, shouldn't NameStr not be part of the AnnotationTypeSelections and instead pull from AnnotationTypes?

(As could have situation where the Names become out of sink if it is stored multiple places)

So, shouldn't NameStr not be part of the AnnotationTypeSelections and instead pull from AnnotationTypes? (As could have situation where the Names become out of sink if it is stored multiple places)

I left it in the table.

I left it in the table.

would not loop through this is will perform poorly and should not be necessary. It looks like my previous comment got removed with this change as it is now past the end of this file, so I will reply to this shortly with that same comment again.

would not loop through this is will perform poorly and should not be necessary. It looks like my previous comment got removed with this change as it is now past the end of this file, so I will reply to this shortly with that same comment again.

would think would want to do something like:

--this would delete only the ones for this User that are not in the uploaded table
DELETE FROM AnnotationTypeSelections where UsrID in
(Select UsrID From @TempTable tmp)
AND
TypeID not in
(Select TypeID From @TempTable tmp)

--this would insert all the ones that are in the uploaded table and not already in AnnotationTypeSelections
Insert INTO AnnotationTypeSelections (TypeID, UsrID)
Select TypeID, UserID
FROM
@TempTable tmp
LEFT OUTER JOIN
AnnotationTypeSelections ATS on ATS.TypeID = tmp.TypeID
AND ATS.UsrID = tmp.UsrID
where
ATS.ASTypeID IS NULL

would think would want to do something like: --this would delete only the ones for this User that are not in the uploaded table DELETE FROM AnnotationTypeSelections where UsrID in (Select UsrID From @TempTable tmp) AND TypeID not in (Select TypeID From @TempTable tmp) --this would insert all the ones that are in the uploaded table and not already in AnnotationTypeSelections Insert INTO AnnotationTypeSelections (TypeID, UsrID) Select TypeID, UserID FROM @TempTable tmp LEFT OUTER JOIN AnnotationTypeSelections ATS on ATS.TypeID = tmp.TypeID AND ATS.UsrID = tmp.UsrID where ATS.ASTypeID IS NULL

Also as a heads up - I sent an email with an attachment --- looks like there is a side consequence to having Userid be part of the table type and not separate --- that side consequence occurs if you have items selected and then want to de-select all of the items.

Also as a heads up - I sent an email with an attachment --- looks like there is a side consequence to having Userid be part of the table type and not separate --- that side consequence occurs if you have items selected and then want to de-select all of the items.
FROM
@TempTable tmp
LEFT OUTER JOIN
AnnotationTypeSelections ATS on ATS.TypeID = tmp.TypeID
AND ATS.UserID = @UserID
where
ATS.ASTypeID IS NULL
END
GO

View File

@@ -41,8 +41,6 @@ namespace VEPROMS.CSLA.Library
// if the user has not created a annotation sub-set list saved to AnnotationTypeSelections table.
if (dt.Rows.Count < 1)
{
//dt.Rows.Add(DataPortal.Fetch<AnnotationTypeInfoList>());
//DataPortal.Fetch<AnnotationTypeInfoList>();
DataRow row;
mschill marked this conversation as resolved
Review

commented out code should be removed or reason given for why it is commented out / should be in the code long term.

commented out code should be removed or reason given for why it is commented out / should be in the code long term.
Review

Comment removed.

Comment removed.
Review

still appears to be:

//dt.Rows.Add(DataPortal.Fetch());
//DataPortal.Fetch();

still appears to be: //dt.Rows.Add(DataPortal.Fetch<AnnotationTypeInfoList>()); //DataPortal.Fetch<AnnotationTypeInfoList>();
int rowflg = 0;
foreach (AnnotationTypeInfo annosel in DataPortal.Fetch<AnnotationTypeInfoList>())
@@ -145,39 +143,12 @@ namespace VEPROMS.CSLA.Library
}
}
}
//public static void Update(string UserID, int TypeID, int dltFlg, string Name = "")
//{
// using (SqlConnection cn = Database.VEPROMS_SqlConnection)
// {
// using (SqlCommand cm = cn.CreateCommand())
// {
// try
// {
// cm.CommandType = CommandType.StoredProcedure;
// cm.CommandText = "UpdateAnnotationstypeSelections";
// cm.CommandTimeout = Database.DefaultTimeout;
// cm.Parameters.AddWithValue("@UserID", UserID);
// cm.Parameters.AddWithValue("@TypeID", TypeID);
// cm.Parameters.AddWithValue("@dltFlg", dltFlg);
// cm.Parameters.AddWithValue("@Name", Name);
// cm.ExecuteNonQuery();
// }
// catch (Exception ex)
// {
// }
// }
// }
//}
public static void Update(DataTable dt)
public static void Update(DataTable dt, string UserID)
{
mschill marked this conversation as resolved Outdated

if commented out, should this be removed?

if commented out, should this be removed?
using (SqlConnection cn = Database.VEPROMS_SqlConnection)
{
using (SqlCommand cm = cn.CreateCommand())
{
try
{
cm.CommandType = CommandType.StoredProcedure;
cm.CommandText = "UpdateAnnotationstypeSelections";
mschill marked this conversation as resolved Outdated

if have a try catch, should there be something in the catch?

if have a try catch, should there be something in the catch?
@@ -186,13 +157,9 @@ namespace VEPROMS.CSLA.Library
//Pass table Valued parameter to Store Procedure
SqlParameter sqlParam = cm.Parameters.AddWithValue("@TempTable", dt);
sqlParam.SqlDbType = SqlDbType.Structured;
cm.Parameters.AddWithValue("@UserID", UserID);
cm.ExecuteNonQuery();
}
catch (Exception ex)
{
}
}
}
}