Skip to content

RE1-T46 bug fixes#296

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ucswift merged 6 commits into from
Mar 11, 2026
Merged

RE1-T46 bug fixes #296

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a0a5825
RE1-T46 bug fixes
ucswift Mar 11, 2026
acaa56e
RE1-T46 Bug fixes
ucswift Mar 11, 2026
ae8f185
RE1-T46 Fixes
ucswift Mar 11, 2026
7fc8f3f
RE1-T46 PR#296 fixes
ucswift Mar 11, 2026
84105a4
RE1-T46 PR#296 fixes
ucswift Mar 11, 2026
a3024b4
RE1-T46 PR#296 fixes
ucswift Mar 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion Core/Resgrid.Model/Repositories/IWorkflowRepository.cs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
using System.Collections.Generic;
using System.Collections.Generic;
using System.Threading.Tasks;

namespace Resgrid.Model.Repositories
Expand All@@ -8,6 +8,13 @@ public interface IWorkflowRepository : IRepository<Workflow>
Task<IEnumerable<Workflow>> GetAllActiveByDepartmentAndEventTypeAsync(int departmentId, int triggerEventType);
Task<IEnumerable<Workflow>> GetAllByDepartmentIdAsync(int departmentId);
Task<Workflow> GetByDepartmentAndEventTypeAsync(int departmentId, int triggerEventType);

/// <summary>
/// Atomically deletes a workflow and all its dependent child records (WorkflowRunLogs,
/// WorkflowRuns, WorkflowSteps) within a single database transaction, preventing FK
/// constraint violations caused by concurrent run inserts racing with deletion.
/// </summary>
Task DeleteWorkflowWithAllDependenciesAsync(string workflowId);
}
}

10 changes: 10 additions & 0 deletions Core/Resgrid.Model/Services/IAuthorizationService.cs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -332,5 +332,15 @@ public interface IAuthorizationService
Task<bool> CanUserManageWorkflowCredentialAsync(string userId, int departmentId);

Task<bool> CanUserViewWorkflowRunsAsync(string userId, int departmentId);

/// <summary>
/// Determines whether the specified user is a department admin or managing member of the given department.
/// Both <paramref name="userId"/> and <paramref name="departmentId"/> must be explicitly supplied by the
/// caller so the check is performed against the actual resource being modified, not just claims values.
/// </summary>
/// <param name="userId">The user identifier to verify.</param>
/// <param name="departmentId">The department identifier of the resource being modified.</param>
/// <returns><c>true</c> if the user is the managing member or a department admin; otherwise <c>false</c>.</returns>
Task<bool> CanUserModifyDepartmentAsync(string userId, int departmentId);
}
}
10 changes: 10 additions & 0 deletions Core/Resgrid.Services/AuthorizationService.cs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1516,5 +1516,15 @@ public async Task<bool> CanUserViewWorkflowRunsAsync(string userId, int departme
return _permissionsService.IsUserAllowed(permission, department != null && department.IsUserAnAdmin(userId), isGroupAdmin, roles);
}

public async Task<bool> CanUserModifyDepartmentAsync(string userId, int departmentId)
{
var department = await _departmentsService.GetDepartmentByIdAsync(departmentId);

if (department == null)
return false;

return department.IsUserAnAdmin(userId);
}

}
}
22 changes: 13 additions & 9 deletions Core/Resgrid.Services/WorkflowService.cs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -88,15 +88,11 @@ public async Task<bool> DeleteWorkflowAsync(string workflowId, CancellationToken
var workflow = await _workflowRepository.GetByIdAsync(workflowId);
if (workflow == null) return false;

// Delete child records in dependency order to avoid FK constraint violations:
// 1. WorkflowRunLogs (references WorkflowRuns)
await _runLogRepository.DeleteAllByWorkflowIdAsync(workflowId);
// 2. WorkflowRuns (references Workflows)
await _runRepository.DeleteAllByWorkflowIdAsync(workflowId);
// 3. WorkflowSteps (references Workflows)
await _stepRepository.DeleteAllByWorkflowIdAsync(workflowId);
// 4. Workflow itself
await _workflowRepository.DeleteAsync(workflow, cancellationToken);
// Atomically delete all child records (WorkflowRunLogs → WorkflowRuns → WorkflowSteps)
// and the workflow itself within a single database transaction. This prevents the
// FK_WorkflowRuns_Workflows constraint violation that occurs when a background worker
// inserts a new WorkflowRun between the sequential per-table deletes.
await _workflowRepository.DeleteWorkflowWithAllDependenciesAsync(workflowId);

return true;
}
Expand DownExpand Up@@ -172,6 +168,14 @@ public async Task<WorkflowStep> SaveWorkflowStepAsync(WorkflowStep step, Cancell
step.CreatedOn = DateTime.UtcNow;
return await _stepRepository.InsertAsync(step, cancellationToken);
}

// Fetch the existing record to preserve immutable audit fields (CreatedOn, CreatedByUserId).
var existing = await _stepRepository.GetByIdAsync(step.WorkflowStepId);
if (existing == null)
throw new KeyNotFoundException($"WorkflowStep '{step.WorkflowStepId}' was not found. It may have been deleted or the ID is stale.");

step.CreatedOn = existing.CreatedOn;
step.CreatedByUserId = existing.CreatedByUserId;
step.UpdatedOn = DateTime.UtcNow;
await _stepRepository.UpdateAsync(step, cancellationToken);
return step;
Expand Down
16 changes: 9 additions & 7 deletions Providers/Resgrid.Providers.Cache/AzureRedisCacheProvider.cs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -36,10 +36,11 @@ public T Retrieve<T>(string cacheKey, Func<T> fallbackFunction, TimeSpan expirat
if (cacheValue.HasValue)
data = ObjectSerialization.Deserialize<T>(cacheValue);
}
catch
catch (Exception deserializeEx)
{
Remove(SetCacheKeyForEnv(cacheKey));
throw;
Logging.LogException(deserializeEx);
Remove(cacheKey);
data = null;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

if (data != null)
Expand DownExpand Up@@ -111,10 +112,11 @@ public async Task<T> RetrieveAsync<T>(string cacheKey, Func<Task<T>> fallbackFun
if (cacheValue.HasValue)
data = ObjectSerialization.Deserialize<T>(cacheValue);
}
catch
catch (Exception deserializeEx)
{
await RemoveAsync(SetCacheKeyForEnv(cacheKey));
throw;
Logging.LogException(deserializeEx);
await RemoveAsync(cacheKey);
data = null;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

if (data != null)
Expand DownExpand Up@@ -157,7 +159,7 @@ public async Task<bool> SetStringAsync(string cacheKey, string value, TimeSpan e
if (Config.SystemBehaviorConfig.CacheEnabled && _connection != null && _connection.IsConnected)
{
cache = _connection.GetDatabase();

if (value != null && cache != null)
{
try
Expand Down
2 changes: 1 addition & 1 deletion Repositories/Resgrid.Repositories.DataRepository/LogsRepository.cs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -201,7 +201,7 @@ public async Task<IEnumerable<Log>> GetAllLogsByDepartmentIdYearAsync(int depart
{
var dynamicParameters = new DynamicParametersExtension();
dynamicParameters.Add("DepartmentId", departmentId);
dynamicParameters.Add("Year", int.Parse(year));
dynamicParameters.Add("Year", int.TryParse(year, out var parsedYear) ? parsedYear : DateTime.UtcNow.Year);

@coderabbitaicoderabbitaiBotMar 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don't substitute malformed year values with the current year.

Line 204 turns invalid input into a valid-but-wrong filter. Because Web/Resgrid.Web/Areas/User/Controllers/LogsController.cs passes the query-string value through and Core/Resgrid.Services/WorkLogsService.cs does not validate it, a request like ?year=abcd will now return current-year logs instead of surfacing bad input. That is a correctness regression and will be very hard to spot from the UI.

Prefer failing validation upstream, or at minimum only apply a fallback for null/whitespace and reject malformed values explicitly.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Repositories/Resgrid.Repositories.DataRepository/LogsRepository.cs` at line
204, The code in LogsRepository.cs currently substitutes any malformed year
string with DateTime.UtcNow.Year; change this so only null/whitespace falls back
to the current year and any non-empty, non-numeric year is rejected: update the
dynamicParameters.Add("Year", ...) logic in the method that builds query
parameters (the line with dynamicParameters.Add("Year", int.TryParse(year, out
var parsedYear) ? parsedYear : DateTime.UtcNow.Year)) to check if
string.IsNullOrWhiteSpace(year) then use DateTime.UtcNow.Year, else attempt
int.TryParse and if parsing fails throw an ArgumentException (or propagate a
validation error) so callers (e.g., LogsController/WorkLogsService) can surface
invalid input instead of silently using the current year.


var query = _queryFactory.GetQuery<SelecAllLogsByDidYearQuery>();

Expand Down
13 changes: 7 additions & 6 deletions ...itories/Resgrid.Repositories.DataRepository/Servers/PostgreSql/PostgreSqlConfiguration.cs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -85,16 +85,17 @@ public PostgreSqlConfiguration()
INNER JOIN %SCHEMA%.%ASPNETUSERSTABLE% u ON u.Id = al.UserId
WHERE al.DestinationId = %CALLID% AND (al.ActionTypeId IS NULL OR al.ActionTypeId IN (%TYPES%))";
SelectPreviousActionLogsByUserQuery = @"
SELECT %SCHEMA%.%ACTIONLOGSTABLE%.*, %SCHEMA%.%ASPNETUSERSTABLE%.*
SELECT a1.*, u.*
FROM %SCHEMA%.%ACTIONLOGSTABLE% a1
INNER JOIN %SCHEMA%.%ASPNETUSERSTABLE% ON %SCHEMA%.%ASPNETUSERSTABLE%.Id = %SCHEMA%.%ACTIONLOGSTABLE%.UserId
WHERE a1.UserId = %USERID% AND ActionLogId < %ACTIONLOGID%";
INNER JOIN %SCHEMA%.%ASPNETUSERSTABLE% u ON u.Id = a1.UserId
WHERE a1.UserId = %USERID% AND a1.ActionLogId < %ACTIONLOGID%
ORDER BY a1.ActionLogId DESC LIMIT 1";
SelectLastActionLogByUserIdQuery = @"
SELECT %SCHEMA%.%ACTIONLOGSTABLE%.*, %SCHEMA%.%ASPNETUSERSTABLE%.*
SELECT a1.*, u.*
FROM %SCHEMA%.%ACTIONLOGSTABLE% a1
INNER JOIN %SCHEMA%.%ASPNETUSERSTABLE% ON %SCHEMA%.%ASPNETUSERSTABLE%.Id = %SCHEMA%.%ACTIONLOGSTABLE%.UserId
INNER JOIN %SCHEMA%.%ASPNETUSERSTABLE% u ON u.Id = a1.UserId
WHERE a1.UserId = %USERID%
ORDER BY ActionLogId DESC limit 1";
ORDER BY a1.ActionLogId DESC limit 1";
SelectActionLogsByCallIdQuery = @"
SELECT al.*, u.*
FROM %SCHEMA%.%ACTIONLOGSTABLE% al
Expand Down
13 changes: 7 additions & 6 deletions Repositories/Resgrid.Repositories.DataRepository/Servers/SqlServer/SqlServerConfiguration.cs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -83,16 +83,17 @@ public SqlServerConfiguration()
INNER JOIN %SCHEMA%.%ASPNETUSERSTABLE% u ON u.Id = al.UserId
WHERE al.[DestinationId] = %CALLID% AND (al.[ActionTypeId] IS NULL OR al.[ActionTypeId] IN (%TYPES%))";
SelectPreviousActionLogsByUserQuery = @"
SELECT %SCHEMA%.%ACTIONLOGSTABLE%.*, %SCHEMA%.%ASPNETUSERSTABLE%.*
SELECT TOP 1 a1.*, u.*
FROM %SCHEMA%.%ACTIONLOGSTABLE% a1
INNER JOIN %SCHEMA%.%ASPNETUSERSTABLE% ON %SCHEMA%.%ASPNETUSERSTABLE%.Id = %SCHEMA%.%ACTIONLOGSTABLE%.UserId
WHERE a1.UserId = %USERID% AND [ActionLogId] < %ACTIONLOGID%";
INNER JOIN %SCHEMA%.%ASPNETUSERSTABLE% u ON u.Id = a1.UserId
WHERE a1.UserId = %USERID% AND a1.[ActionLogId] < %ACTIONLOGID%
ORDER BY a1.[ActionLogId] DESC";
SelectLastActionLogByUserIdQuery = @"
SELECT TOP 1 %SCHEMA%.%ACTIONLOGSTABLE%.*, %SCHEMA%.%ASPNETUSERSTABLE%.*
SELECT TOP 1 a1.*, u.*
FROM %SCHEMA%.%ACTIONLOGSTABLE% a1
INNER JOIN %SCHEMA%.%ASPNETUSERSTABLE% ON %SCHEMA%.%ASPNETUSERSTABLE%.Id = %SCHEMA%.%ACTIONLOGSTABLE%.UserId
INNER JOIN %SCHEMA%.%ASPNETUSERSTABLE% u ON u.Id = a1.UserId
WHERE a1.UserId = %USERID%
ORDER BY ActionLogId DESC";
ORDER BY a1.ActionLogId DESC";
SelectActionLogsByCallIdQuery = @"
SELECT al.*, u.*
FROM %SCHEMA%.%ACTIONLOGSTABLE% al
Expand Down
96 changes: 96 additions & 0 deletions Repositories/Resgrid.Repositories.DataRepository/WorkflowRepository.cs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -9,6 +9,8 @@
using Resgrid.Model.Repositories.Connection;
using Resgrid.Model.Repositories.Queries;
using Resgrid.Repositories.DataRepository.Configs;
using System.Data;
using Resgrid.Config;
using Resgrid.Repositories.DataRepository.Queries.Workflows;

namespace Resgrid.Repositories.DataRepository
Expand DownExpand Up@@ -132,6 +134,100 @@ public async Task<Workflow> GetByDepartmentAndEventTypeAsync(int departmentId, i
throw;
}
}

/// <inheritdoc />
public async Task DeleteWorkflowWithAllDependenciesAsync(string workflowId)
{
try
{
var dp = new DynamicParametersExtension();
dp.Add("WorkflowId", workflowId);

// Lock SQL: acquired inside the transaction before child deletes so that no
// concurrent WorkflowRun INSERT can slip in between the child deletes and the
// parent delete, which would cause an FK violation.
string lockWorkflowSql;
string deleteWorkflowSql;
if (DataConfig.DatabaseType == DatabaseTypes.Postgres)
{
lockWorkflowSql = $"SELECT workflowid FROM {_sqlConfiguration.SchemaName}.workflows WHERE workflowid = {_sqlConfiguration.ParameterNotation}WorkflowId FOR UPDATE";
deleteWorkflowSql = $"DELETE FROM {_sqlConfiguration.SchemaName}.workflows WHERE workflowid = {_sqlConfiguration.ParameterNotation}WorkflowId";
}
else
{
lockWorkflowSql = $"SELECT [WorkflowId] FROM {_sqlConfiguration.SchemaName}.[Workflows] WITH (UPDLOCK, HOLDLOCK) WHERE [WorkflowId] = {_sqlConfiguration.ParameterNotation}WorkflowId";
deleteWorkflowSql = $"DELETE FROM {_sqlConfiguration.SchemaName}.[Workflows] WHERE [WorkflowId] = {_sqlConfiguration.ParameterNotation}WorkflowId";
}

if (_unitOfWork?.Connection != null)
{
// Enlist in the caller's ambient unit-of-work — reuse its connection and
// transaction so the deletes participate in the same transaction scope and
// do not commit independently.
var conn = _unitOfWork.CreateOrGetConnection();
var transaction = _unitOfWork.Transaction;

// Lock the parent row before touching any child tables so that concurrent
// run inserts targeting this workflow are blocked for the duration of the
// transaction, preventing FK constraint violations.
await conn.ExecuteAsync(sql: lockWorkflowSql, param: dp, transaction: transaction);

var deleteRunLogsQuery = _queryFactory.GetDeleteQuery<DeleteWorkflowRunLogsByWorkflowIdQuery>();
await conn.ExecuteAsync(sql: deleteRunLogsQuery, param: dp, transaction: transaction);

var deleteRunsQuery = _queryFactory.GetDeleteQuery<DeleteWorkflowRunsByWorkflowIdQuery>();
await conn.ExecuteAsync(sql: deleteRunsQuery, param: dp, transaction: transaction);

var deleteStepsQuery = _queryFactory.GetDeleteQuery<DeleteWorkflowStepsByWorkflowIdQuery>();
await conn.ExecuteAsync(sql: deleteStepsQuery, param: dp, transaction: transaction);

await conn.ExecuteAsync(sql: deleteWorkflowSql, param: dp, transaction: transaction);
}
else
{
// No ambient unit-of-work — open a dedicated connection and manage a local
// transaction to keep the multi-step delete atomic.
using (var conn = _connectionProvider.Create())
{
await conn.OpenAsync();

using (var transaction = conn.BeginTransaction(IsolationLevel.ReadCommitted))
{
try
{
// Lock the parent row before touching any child tables so that
// concurrent run inserts targeting this workflow are blocked for
// the duration of this transaction, preventing FK violations.
await conn.ExecuteAsync(sql: lockWorkflowSql, param: dp, transaction: transaction);

var deleteRunLogsQuery = _queryFactory.GetDeleteQuery<DeleteWorkflowRunLogsByWorkflowIdQuery>();
await conn.ExecuteAsync(sql: deleteRunLogsQuery, param: dp, transaction: transaction);

var deleteRunsQuery = _queryFactory.GetDeleteQuery<DeleteWorkflowRunsByWorkflowIdQuery>();
await conn.ExecuteAsync(sql: deleteRunsQuery, param: dp, transaction: transaction);

var deleteStepsQuery = _queryFactory.GetDeleteQuery<DeleteWorkflowStepsByWorkflowIdQuery>();
await conn.ExecuteAsync(sql: deleteStepsQuery, param: dp, transaction: transaction);

await conn.ExecuteAsync(sql: deleteWorkflowSql, param: dp, transaction: transaction);

transaction.Commit();
}
catch
{
transaction.Rollback();
throw;
}
}
}
}
}
catch (Exception ex)
{
Logging.LogException(ex);
throw;
}
}
}
}

Loading
Loading