RG-T117 Fixes #454
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -173,6 +173,14 @@ public class KeywordIntentClassifier : INLUProvider | |
| "list_messages", null), | ||
| (R(@"^(show|list|get|what'?s)\s+(on\s+)?(the\s+)?(calendar|schedule|agenda)"), | ||
| "list_calendar", null), | ||
| // Upcoming-calendar phrasings: "when is the next event?", "what is upcoming in the | ||
| // calendar?", "upcoming events", "what's coming up", "next events". | ||
| (R(@"^when('?s|\s+is)\s+(the\s+)?next\s+(event|meeting|training|class)(s|es)?$"), | ||
| "list_calendar", null), | ||
|
There was a problem hiding this comment.
Shared string literal Kody rule violation: Centralize string constants Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| (R(@"^(what('?s|\s+is)\s+)?(upcoming|coming\s+up)(\s+(events?|meetings?|trainings?))?(\s+(on|in)\s+(the\s+)?(calendar|schedule|agenda))?$"), | ||
| "list_calendar", null), | ||
| (R(@"^(next|upcoming)\s+(events?|meetings?|trainings?)$"), | ||
| "list_calendar", null), | ||
| (R(@"^(show|list|get|my)\s+shifts?"), | ||
| "list_shifts", null), | ||
| (R(@"^(weather\s+)?(alerts?|warnings?)"), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -343,6 +343,19 @@ public async Task<List<ChatMessage>> GetThreadPageAsync(string threadRootMessage | |
| if (member != null && (member.IsBanned || (member.MutedUntil.HasValue && member.MutedUntil.Value > DateTime.UtcNow))) | ||
| return false; | ||
|
|
||
| // Double-taps are common: bail out before the insert so the ordinary duplicate never | ||
| // reaches the database (RepositoryBase logs every insert exception, so relying on the | ||
| // unique-violation catch below alone floods the error log). The catch still covers the | ||
| // genuine concurrent race two requests can win simultaneously. | ||
| var existingReactions = await _chatMessageReactionRepository.GetByMessageIdsAsync(new[] { chatMessageId }); | ||
|
There was a problem hiding this comment.
Unguarded await: Kody rule violation: Handle async operations with proper error handling Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. There was a problem hiding this comment.
Unguarded external DB call: Kody rule violation: Add try-catch blocks for external calls Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| var alreadyReacted = existingReactions != null && existingReactions.Any(r => | ||
| string.Equals(r.Emoji, emoji, StringComparison.Ordinal) | ||
| && (unitId.HasValue | ||
| ? r.UnitId == unitId | ||
| : r.UserId != null && string.Equals(r.UserId, userId, StringComparison.OrdinalIgnoreCase))); | ||
| if (alreadyReacted) | ||
| return true; | ||
|
|
||
| try | ||
| { | ||
| await _chatMessageReactionRepository.InsertAsync(new ChatMessageReaction | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| using FluentMigrator; | ||
|
|
||
| namespace Resgrid.Providers.Migrations.Migrations | ||
| { | ||
| /// <summary> | ||
| /// The initial schema created Notes and Documents with a misspelled "Catery" column while the | ||
| /// entities (and the Dapper-generated INSERT/UPDATE statements) use "Category" — every save | ||
| /// against a database built from M0001 failed with "invalid column name 'Category'". Renames | ||
| /// the column where the typo exists; guarded so databases that already have the correct | ||
| /// column (or were hand-fixed) are untouched. | ||
| /// </summary> | ||
| [Migration(113)] | ||
| public class M0113_FixNotesDocumentsCategoryColumn : Migration | ||
| { | ||
| public override void Up() | ||
| { | ||
| Execute.Sql(@" | ||
| IF COL_LENGTH('dbo.Notes', 'Catery') IS NOT NULL AND COL_LENGTH('dbo.Notes', 'Category') IS NULL | ||
| EXEC sp_rename 'dbo.Notes.Catery', 'Category', 'COLUMN';"); | ||
|
|
||
| Execute.Sql(@" | ||
| IF COL_LENGTH('dbo.Documents', 'Catery') IS NOT NULL AND COL_LENGTH('dbo.Documents', 'Category') IS NULL | ||
| EXEC sp_rename 'dbo.Documents.Catery', 'Category', 'COLUMN';"); | ||
| } | ||
|
|
||
| public override void Down() | ||
| { | ||
| // One-way typo fix; nothing to restore. | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| using FluentMigrator; | ||
|
|
||
| namespace Resgrid.Providers.MigrationsPg.Migrations | ||
| { | ||
| /// <summary> | ||
| /// The initial schema created notes and documents with a misspelled "catery" column while the | ||
| /// entities (and the Dapper-generated INSERT/UPDATE statements) use "category" — every save | ||
| /// against a database built from M0001 failed with 42703 "column category does not exist". | ||
| /// Renames the column where the typo exists; guarded so databases that already have the | ||
| /// correct column (or were hand-fixed) are untouched. | ||
| /// </summary> | ||
| [Migration(113)] | ||
| public class M0113_FixNotesDocumentsCategoryColumnPg : Migration | ||
| { | ||
| public override void Up() | ||
| { | ||
| Execute.Sql(@" | ||
| DO $$ | ||
| BEGIN | ||
| IF EXISTS (SELECT 1 FROM information_schema.columns WHERE table_schema = 'public' AND table_name = 'notes' AND column_name = 'catery') | ||
| AND NOT EXISTS (SELECT 1 FROM information_schema.columns WHERE table_schema = 'public' AND table_name = 'notes' AND column_name = 'category') THEN | ||
| ALTER TABLE public.notes RENAME COLUMN catery TO category; | ||
| END IF; | ||
|
|
||
| IF EXISTS (SELECT 1 FROM information_schema.columns WHERE table_schema = 'public' AND table_name = 'documents' AND column_name = 'catery') | ||
| AND NOT EXISTS (SELECT 1 FROM information_schema.columns WHERE table_schema = 'public' AND table_name = 'documents' AND column_name = 'category') THEN | ||
| ALTER TABLE public.documents RENAME COLUMN catery TO category; | ||
| END IF; | ||
| END $$;"); | ||
| } | ||
|
|
||
| public override void Down() | ||
| { | ||
| // One-way typo fix; nothing to restore. | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -312,7 +312,16 @@ export default function Composer({ | |
| </label> | ||
| )} | ||
|
|
||
| {allowImages && ( | ||
| <input | ||
| ref={fileRef} | ||
| type="file" | ||
| accept="image/*" | ||
| className="rgchat-fileinput" | ||
| style={{ display: 'none' }} | ||
| onChange={(event) => void handleFile(event.target.files?.[0])} | ||
|
There was a problem hiding this comment.
Inline arrow function in the Kody rule violation: Avoid using .bind() or arrow functions in JSX props Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| /> | ||
| )} | ||
| </div> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,23 @@ | ||
| using Newtonsoft.Json.Linq; | ||
| using Resgrid.Config; | ||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.ComponentModel.DataAnnotations; | ||
| using System.Net; | ||
| using System.Net.Http; | ||
| using System.Threading.Tasks; | ||
|
|
||
| namespace Resgrid.WebCore.Attributes | ||
| { | ||
| public class GoogleReCaptchaValidationAttribute : ValidationAttribute | ||
| { | ||
| // Shared client: a new HttpClient per validation leaks sockets under load ("Resource | ||
| // temporarily unavailable" on the register form). Validation attributes are synchronous, | ||
| // so the call is bounded by a short timeout instead of the 100-second default. | ||
| private static readonly HttpClient _httpClient = new HttpClient | ||
| { | ||
| Timeout = TimeSpan.FromSeconds(10) | ||
| }; | ||
|
|
||
| protected override ValidationResult IsValid(object value, ValidationContext validationContext) | ||
| { | ||
|
|
@@ -22,23 +31,37 @@ protected override ValidationResult IsValid(object value, ValidationContext vali | |
| String reCaptchResponse = value.ToString(); | ||
| String reCaptchaSecret = WebConfig.RecaptchaPrivateKey; | ||
|
|
||
| try | ||
| { | ||
| // POST keeps the secret out of URLs (request logs, proxies). | ||
| using var content = new FormUrlEncodedContent(new Dictionary<string, string> | ||
| { | ||
| ["secret"] = reCaptchaSecret, | ||
| ["response"] = reCaptchResponse | ||
| }); | ||
|
|
||
| var httpResponse = _httpClient.PostAsync("https://www.google.com/recaptcha/api/siteverify", content).GetAwaiter().GetResult(); | ||
| if (httpResponse.StatusCode != HttpStatusCode.OK) | ||
| { | ||
| return errorResult.Value; | ||
| } | ||
|
|
||
| String jsonResponse = httpResponse.Content.ReadAsStringAsync().GetAwaiter().GetResult(); | ||
|
Comment on lines
+43
to
+49
Contributor
There was a problem hiding this comment. 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Locate existing async request-validation components and all reCAPTCHA uses.
rg -n -C 3 --glob '*.cs' \
'GoogleReCaptchaValidationAttribute|IAsync(Action|Authorization|Resource)Filter|ModelState\.AddModelError' \
WebRepository: Resgrid/Core Length of output: 50369 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Inspect the reCAPTCHA attribute and focused usages without dumping the entire Web/Controllers match list.
printf -- '--- GoogleReCaptchaValidationAttribute.cs ---\n'
cat -n Web/Resgrid.Web/Attributes/GoogleReCaptchaValidationAttribute.cs
printf -- '\n--- GoogleReCaptchaValidationAttribute references ---\n'
rg -n --glob '*.cshtml' --glob '*.cs' --glob '*.razor' 'GoogleReCaptchaValidation|g-recaptcha|recaptcha' Web/Resgrid.Web
printf -- '\n--- AccountController POST snippets around register actions ---\n'
rg -n -A 80 -B 10 'Register|RegisterModel|GoogleReCaptcha|g-recaptcha' Web/Resgrid.Web/Controllers/AccountController.csRepository: Resgrid/Core Length of output: 10069 Move reCAPTCHA verification out of synchronous model validation.
🤖 Prompt for AI Agents |
||
| dynamic jsonData = JObject.Parse(jsonResponse); | ||
|
Comment on lines
+49
to
+50
Contributor
There was a problem hiding this comment. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Confirm the installed Newtonsoft.Json reference and inspect parsing exception handling.
rg -n -C 3 --glob '*.csproj' --glob 'packages.config' --glob '*.cs' \
'Newtonsoft\.Json|JObject\.Parse|JsonException|JsonReaderException' \
WebRepository: Resgrid/Core Length of output: 43035 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Inspect the validation attribute around the parsing and exception-handling logic.
sed -n '1,140p' Web/Resgrid.Web/Attributes/GoogleReCaptchaValidationAttribute.cs | cat -n
# Search for the exact method/attribute to ensure context is complete.
rg -n "class GoogleReCaptchaValidationAttribute|ValidationAttribute|LogException|TryVerify|OnAuthorization" Web/Resgrid.Web/Attributes/GoogleReCaptchaValidationAttribute.csRepository: Resgrid/Core Length of output: 3195 🌐 Web query:
💡 Result: In Newtonsoft.Json version 13.0.3, the JObject.Parse method is designed to parse a JSON string into a JObject [1][2]. If the provided string is not valid JSON or does not represent a JSON object (e.g., if it represents a JSON array or a primitive), the method throws a JsonReaderException [1][3][4]. JsonReaderException is a subclass of JsonException [5][6]. This exception typically includes detailed information about the error, such as the path in the JSON string where the error occurred, the line number, and the position [5]. Common scenarios that trigger this exception include: - Passing invalid JSON syntax [1][4]. - Passing a root JSON element that is not an object, such as a JSON array ("[]") [3]. In such cases, if you expect an array, JArray.Parse should be used instead; if the JSON structure is unknown, JToken.Parse is the appropriate method [3]. When handling parsing errors, it is standard practice to catch JsonReaderException specifically to handle malformed input, rather than catching the base JsonException or the generic System.Exception, which may obscure other issues [7][5][6]. Citations:
Fail closed when Google returns a non-object reCAPTCHA response.
🤖 Prompt for AI AgentsThere was a problem hiding this comment.
Untrusted JSON: Kody rule violation: Always validate JSON parsing Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| if (jsonData.success != true.ToString().ToLower()) | ||
| { | ||
| return errorResult.Value; | ||
| } | ||
|
|
||
| return ValidationResult.Success; | ||
| } | ||
| catch (Exception ex) when (ex is HttpRequestException || ex is TaskCanceledException || ex is OperationCanceledException) | ||
| { | ||
| // Transient network/DNS failure reaching Google: fail closed with a retryable | ||
| // validation message instead of letting the exception 500 the register page. | ||
| Framework.Logging.LogException(ex); | ||
|
There was a problem hiding this comment.
Missing structured context: the error log only passes the exception object with no operation name or relevant identifiers. Pass structured context, e.g., Kody rule violation: Include error context in structured logs Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| return new ValidationResult("We couldn't verify the reCAPTCHA right now. Please try again.", new String[] { validationContext.MemberName }); | ||
| } | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded string literal
"list_calendar"represents a finite intent set prone to typos and poor discoverability. Define an enum (e.g.,IntentTypes.ListCalendar) or a constants class and reference it in place of the literal.Kody rule violation: Use enums instead of magic strings
Prompt for LLM
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.