Conversation
|
Thanks for opening this, but we'd appreciate a little more information. Could you update it with more details? |
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: Resgrid/Core/.coderabbit.yaml Review profile: CHILL Plan: Essentials Run ID: ⛔ Files ignored due to path filters (14)
📒 Files selected for processing (5)
Included review availability: This review used your included allowance. 2 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 3 reviews per hour. Your free on-demand review promotion remains active until October 9, 2026 at 6:00 PM UTC. 📝 WalkthroughWalkthroughAdmin Assist workspace availability now requires both Admin Assist feature flags. Non-setup pages check workspace availability, and navigation displays Admin Assist and setup links in a top-level dropdown. ChangesAdmin Assist workspace
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant AdminAssistController
participant AdminAssistFeatureAvailability
participant IFeatureToggleService
AdminAssistController->>AdminAssistFeatureAvailability: Check workspace availability
AdminAssistFeatureAvailability->>IFeatureToggleService: Evaluate Admin Assist flags
IFeatureToggleService-->>AdminAssistFeatureAvailability: Return flag states
AdminAssistFeatureAvailability-->>AdminAssistController: Return workspace availability
AdminAssistController-->>AdminAssistController: Return NotFound when disabled
Merge Risk: ⚪ Minimal · up to Admin Assist pages and links now follow the workspace flags, while setup remains separately available to eligible admins. No concrete failure in the changed behavior presents a PR-specific merge blocker. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 4 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
| localized.Should().Contain("AdminAssistController"); | ||
|
|
||
| var startup = File.ReadAllText(FindRepositoryFile("Web/Resgrid.Web.Services/Startup.cs")); | ||
| Regex.IsMatch(startup, @"^\s*services\.AddLocalization\(", RegexOptions.Multiline) |
There was a problem hiding this comment.
Regular expression processing without a timeout in Regex.IsMatch can allow untrusted input to trigger denial-of-service (DoS) attacks. Define a timeout for the regular expression.
Kody rule violation: Specify Timeout for Regular Expressions
Prompt for LLM
File Tests/Resgrid.Tests/Web/ApiLocalizationRegistrationTests.cs:
Line 35:
Regular expression processing without a timeout in `Regex.IsMatch` can allow untrusted input to trigger denial-of-service (DoS) attacks. Define a timeout for the regular expression.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var localized = typeof(Resgrid.Web.Services.Controllers.v4.AdminAssistController).Assembly.GetTypes() | ||
| .Where(t => typeof(ControllerBase).IsAssignableFrom(t) && !t.IsAbstract) | ||
| .Where(t => t.GetConstructors().Any(c => c.GetParameters().Any(p => p.ParameterType.IsGenericType && | ||
| p.ParameterType.GetGenericTypeDefinition() == typeof(IStringLocalizer<>)))) | ||
| .Select(t => t.Name) | ||
| .ToList(); |
There was a problem hiding this comment.
The long LINQ chain obscures the separate filtering and projection steps for controllers and localized. Store the controller filter in a named intermediate query before applying the localization filter and projection.
Kody rule violation: Limit Lengthy LINQ Chains
var controllers = typeof(Resgrid.Web.Services.Controllers.v4.AdminAssistController).Assembly.GetTypes()
.Where(t => typeof(ControllerBase).IsAssignableFrom(t) && !t.IsAbstract);
var localized = controllers
.Where(t => t.GetConstructors().Any(c => c.GetParameters().Any(p => p.ParameterType.IsGenericType &&
p.ParameterType.GetGenericTypeDefinition() == typeof(IStringLocalizer<>))))
.Select(t => t.Name)
.ToList();Prompt for LLM
File Tests/Resgrid.Tests/Web/ApiLocalizationRegistrationTests.cs:
Line 26 to 31:
The long LINQ chain obscures the separate filtering and projection steps for `controllers` and `localized`. Store the controller filter in a named intermediate query before applying the localization filter and projection.
Suggested Code:
var controllers = typeof(Resgrid.Web.Services.Controllers.v4.AdminAssistController).Assembly.GetTypes()
.Where(t => typeof(ControllerBase).IsAssignableFrom(t) && !t.IsAbstract);
var localized = controllers
.Where(t => t.GetConstructors().Any(c => c.GetParameters().Any(p => p.ParameterType.IsGenericType &&
p.ParameterType.GetGenericTypeDefinition() == typeof(IStringLocalizer<>))))
.Select(t => t.Name)
.ToList();
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| .ToList(); | ||
| localized.Should().Contain("AdminAssistController"); | ||
|
|
||
| var startup = File.ReadAllText(FindRepositoryFile("Web/Resgrid.Web.Services/Startup.cs")); |
There was a problem hiding this comment.
An IOException from File.ReadAllText(FindRepositoryFile("Web/Resgrid.Web.Services/Startup.cs")) currently lacks contextual reporting when Startup.cs cannot be read. Wrap the file-system read in try/catch and call Assert.Fail with the exception message.
Kody rule violation: Add try-catch blocks for external calls
string startup;
try
{
startup = File.ReadAllText(FindRepositoryFile("Web/Resgrid.Web.Services/Startup.cs"));
}
catch (IOException exception)
{
Assert.Fail($"Unable to read Startup.cs: {exception.Message}");
throw;
}Prompt for LLM
File Tests/Resgrid.Tests/Web/ApiLocalizationRegistrationTests.cs:
Line 34:
An `IOException` from `File.ReadAllText(FindRepositoryFile("Web/Resgrid.Web.Services/Startup.cs"))` currently lacks contextual reporting when `Startup.cs` cannot be read. Wrap the file-system read in `try`/`catch` and call `Assert.Fail` with the exception message.
Suggested Code:
string startup;
try
{
startup = File.ReadAllText(FindRepositoryFile("Web/Resgrid.Web.Services/Startup.cs"));
}
catch (IOException exception)
{
Assert.Fail($"Unable to read Startup.cs: {exception.Message}");
throw;
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| // The workspace launches after Setup Wizard and Setup Report: it also needs Ai.AdminAssist, not just Admin.Assist. | ||
| var adminAssistVisible = ClaimsAuthorizationHelper.IsUserDepartmentAdmin() && | ||
| await Resgrid.Services.AdminAssist.AdminAssistFeatureAvailability.IsEnabledAsync(featureToggleService, ClaimsAuthorizationHelper.GetDepartmentId(), false, Context.RequestAborted); | ||
| await Resgrid.Services.AdminAssist.AdminAssistFeatureAvailability.IsWorkspaceEnabledAsync(featureToggleService, ClaimsAuthorizationHelper.GetDepartmentId(), Context.RequestAborted); |
There was a problem hiding this comment.
Unhandled task rejection from AdminAssistFeatureAvailability.IsWorkspaceEnabledAsync can escape the awaited feature-availability operation without logging or a safe fallback. Guard the operation with try/catch, log the failure with operation and department context, and set adminAssistVisible to false; the same issue occurs in Core/Resgrid.Services/AdminAssist/AdminAssistFeatureAvailability.cs:20-20, Web/Resgrid.Web/Areas/User/Controllers/AdminAssistController.cs:80-80, Core/Resgrid.Services/AdminAssist/AdminAssistFeatureAvailability.cs:23-23, Tests/Resgrid.Tests/AdminAssist/FeatureToggleTests.cs:49-49, Tests/Resgrid.Tests/AdminAssist/SetupPromptVisibilityTests.cs:45-45, Tests/Resgrid.Tests/AdminAssist/SetupPromptVisibilityTests.cs:59-59, Tests/Resgrid.Tests/AdminAssist/SetupPromptVisibilityTests.cs:70-70, Tests/Resgrid.Tests/AdminAssist/FeatureToggleTests.cs:59-59, Tests/Resgrid.Tests/AdminAssist/FeatureToggleTests.cs:57-57, Tests/Resgrid.Tests/AdminAssist/SetupPromptVisibilityTests.cs:88-88, and Tests/Resgrid.Tests/AdminAssist/FeatureToggleTests.cs:51-51.
Kody rule violation: Handle async operations with proper error handling
try
{
adminAssistVisible = ClaimsAuthorizationHelper.IsUserDepartmentAdmin() &&
await Resgrid.Services.AdminAssist.AdminAssistFeatureAvailability.IsWorkspaceEnabledAsync(featureToggleService, ClaimsAuthorizationHelper.GetDepartmentId(), Context.RequestAborted);
}
catch (Exception ex)
{
logger.LogError(ex, "Failed to determine Admin Assist workspace availability for department {DepartmentId}", ClaimsAuthorizationHelper.GetDepartmentId());
adminAssistVisible = false;
}Prompt for LLM
File Web/Resgrid.Web/Areas/User/Views/Shared/_TopNavbar.cshtml:
Line 9:
Unhandled task rejection from `AdminAssistFeatureAvailability.IsWorkspaceEnabledAsync` can escape the awaited feature-availability operation without logging or a safe fallback. Guard the operation with `try`/`catch`, log the failure with operation and department context, and set `adminAssistVisible` to `false`; the same issue occurs in `Core/Resgrid.Services/AdminAssist/AdminAssistFeatureAvailability.cs:20-20`, `Web/Resgrid.Web/Areas/User/Controllers/AdminAssistController.cs:80-80`, `Core/Resgrid.Services/AdminAssist/AdminAssistFeatureAvailability.cs:23-23`, `Tests/Resgrid.Tests/AdminAssist/FeatureToggleTests.cs:49-49`, `Tests/Resgrid.Tests/AdminAssist/SetupPromptVisibilityTests.cs:45-45`, `Tests/Resgrid.Tests/AdminAssist/SetupPromptVisibilityTests.cs:59-59`, `Tests/Resgrid.Tests/AdminAssist/SetupPromptVisibilityTests.cs:70-70`, `Tests/Resgrid.Tests/AdminAssist/FeatureToggleTests.cs:59-59`, `Tests/Resgrid.Tests/AdminAssist/FeatureToggleTests.cs:57-57`, `Tests/Resgrid.Tests/AdminAssist/SetupPromptVisibilityTests.cs:88-88`, and `Tests/Resgrid.Tests/AdminAssist/FeatureToggleTests.cs:51-51`.
Suggested Code:
try
{
adminAssistVisible = ClaimsAuthorizationHelper.IsUserDepartmentAdmin() &&
await Resgrid.Services.AdminAssist.AdminAssistFeatureAvailability.IsWorkspaceEnabledAsync(featureToggleService, ClaimsAuthorizationHelper.GetDepartmentId(), Context.RequestAborted);
}
catch (Exception ex)
{
logger.LogError(ex, "Failed to determine Admin Assist workspace availability for department {DepartmentId}", ClaimsAuthorizationHelper.GetDepartmentId());
adminAssistVisible = false;
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
2 participants
Summary
Fixes Admin Assist setup and reporting issues by correcting API localization registration, tightening feature-flag requirements, and updating navigation and setup prompt visibility.
Changes
IStringLocalizer<T>—including Admin Assist—can be constructed successfully instead of returning HTTP 500 errors.Admin.AssistandAi.AdminAssistfeature flags.