Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork73
Develop#255
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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
coderabbitaibot commentedOct 3, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
WalkthroughAdds a v4 AvatarsController (Get/Upload/Crop), updates frontend avatar URLs to v4, enhances message recipient parsing (P:/G:/R: prefixes with membership checks), orders inbox/sent messages by SentOn desc, adds permission.Data guards, adds TMobile to DirectSendCarriers, and removes a subscription notice. Changes
Sequence Diagram(s)sequenceDiagram autonumber actor User participant Client as Web Client participant API as AvatarsController (v4) participant Img as ImageSharp participant Store as IImageService rect rgb(240,248,255) Note over User,API: Upload avatar (new) User->>Client: Select image Client->>API: POST /api/v4/Avatars/Upload?id&type (file) API->>Img: Load/validate image Img-->>API: PNG bytes, width/height API->>Store: Save image Store-->>API: URL/key API-->>Client: {status, url, width, height} end rect rgb(245,255,250) Note over User,API: Get avatar (fallback capable) Client->>API: GET /api/v4/Avatars/Get?id&type API->>Store: Fetch image alt Found Store-->>API: Image bytes API-->>Client: PNG image else Not found API-->>Client: Default PNG image end end rect rgb(255,250,240) Note over User,API: Crop avatar Client->>API: PUT /api/v4/Avatars/Crop {crop params} API->>Store: Load existing image API->>Img: Apply crop Img-->>API: Cropped PNG API->>Store: Save cropped image API-->>Client: {status, url} endsequenceDiagram autonumber actor Sender participant API as MessagesController (v4) participant Dept as Department Services participant Users as User Repo Note over Sender,API: Send message with mixed recipient IDs (P:/G:/R:) Sender->>API: POST /Messages/Send { recipients[] } loop For each recipient token API->>API: Strip prefix (P:/G:/R:), sanitize API->>Dept: Validate dept membership / resolve group/role alt Person (P) Dept-->>API: Person exists? API->>Users: Exclude current user API->>API: Add to recipients set else Group (G) Dept-->>API: Group members API->>API: Filter by dept, != current user API->>API: Add members to recipients set else Role (R) Dept-->>API: Role members API->>API: Filter by dept, != current user API->>API: Add members to recipients set end end API-->>Sender: Message queued with resolved recipientsEstimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Thanks for opening this, but we'd appreciate a little more information. Could you update it with more details? |
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.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Web/Resgrid.Web.Services/Controllers/v4/MessagesController.cs (1)
364-388:Fix role member lookup to use the sanitized ID
int.Parse(role.Id)will throw whenrole.Idis prefixed (e.g.R:42). We already strip the prefix intoroleIdToSendToand successfully parseroleId, so reuse that value when calling_personnelRolesService. Otherwise any prefixed role selection breaks message sending.- var roleMembers =- await _personnelRolesService.GetAllMembersOfRoleAsync(int.Parse(role.Id));+ var roleMembers =+ await _personnelRolesService.GetAllMembersOfRoleAsync(roleId);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
Core/Resgrid.Model/MobileCarriers.cs(1 hunks)Core/Resgrid.Services/MessageService.cs(2 hunks)Providers/Resgrid.Providers.Claims/ClaimsLogic.cs(3 hunks)Web/Resgrid.Web.Services/Controllers/v4/AvatarsController.cs(1 hunks)Web/Resgrid.Web.Services/Controllers/v4/MessagesController.cs(3 hunks)Web/Resgrid.Web.Services/Resgrid.Web.Services.xml(1 hunks)Web/Resgrid.Web/Areas/User/Controllers/ConnectController.cs(3 hunks)Web/Resgrid.Web/Areas/User/Views/Dispatch/Chat.cshtml(1 hunks)Web/Resgrid.Web/Areas/User/Views/Home/EditUserProfile.cshtml(1 hunks)Web/Resgrid.Web/Areas/User/Views/Messages/_UnreadTopMessagesPartial.cshtml(1 hunks)Web/Resgrid.Web/Areas/User/Views/Personnel/ViewPerson.cshtml(1 hunks)Web/Resgrid.Web/Areas/User/Views/Personnel/ViewRole.cshtml(1 hunks)Web/Resgrid.Web/Areas/User/Views/Shared/_Navigation.cshtml(1 hunks)Web/Resgrid.Web/Areas/User/Views/Subscription/Index.cshtml(1 hunks)Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.viewcall.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.cs: Prefer functional patterns and immutable data where appropriate in C#
Use modern C# features appropriately
Use meaningful, descriptive names for types, methods, and parameters; avoid unclear abbreviations
Separate state from behavior (e.g., use records for state and place operations in separate static classes)
Prefer pure methods over methods with side effects
Use extension methods appropriately for domain-specific operations
Design for testability; avoid hidden dependencies inside methods and prefer explicit, pure functions
Minimize constructor injection; keep the number of injected dependencies small
Prefer composition with interfaces to extend behavior
Files:
Providers/Resgrid.Providers.Claims/ClaimsLogic.csCore/Resgrid.Services/MessageService.csWeb/Resgrid.Web.Services/Controllers/v4/MessagesController.csWeb/Resgrid.Web/Areas/User/Controllers/ConnectController.csWeb/Resgrid.Web.Services/Controllers/v4/AvatarsController.csCore/Resgrid.Model/MobileCarriers.cs
🧬 Code graph analysis (3)
Providers/Resgrid.Providers.Claims/ClaimsLogic.cs (1)
Providers/Resgrid.Providers.Claims/ResgridClaimTypes.cs (4)
Data(21-27)ResgridClaimTypes(3-72)Resources(29-60)Actions(5-12)
Web/Resgrid.Web/Areas/User/Controllers/ConnectController.cs (1)
Core/Resgrid.Config/SystemBehaviorConfig.cs (1)
SystemBehaviorConfig(11-209)
Web/Resgrid.Web.Services/Controllers/v4/AvatarsController.cs (2)
Web/Resgrid.Web/Areas/User/Controllers/ConnectController.cs (6)
HttpGet(39-60)HttpGet(62-93)HttpGet(192-225)HttpGet(227-236)HttpGet(238-250)HttpGet(272-301)Core/Resgrid.Config/SystemBehaviorConfig.cs (1)
SystemBehaviorConfig(11-209)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (csharp)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (csharp)
🔇 Additional comments (24)
Web/Resgrid.Web/Areas/User/Views/Subscription/Index.cshtml (1)
424-463:LGTM! Clean removal of the warning alert.The removal of the Push-To-Talk Addon unavailability warning streamlines the UI while preserving the essential PTT information and management button. The structural adjustment at line 455 maintains proper HTML nesting.
Web/Resgrid.Web.Services/Resgrid.Web.Services.xml (1)
35-43:Confirm auth-free access claim.Docs say the v4 avatar GET endpoint needs no auth header. Double-check the controller/security attributes—if authorization is enforced, the comment will mislead SDK consumers. Update the summary accordingly or loosen the endpoint.
Providers/Resgrid.Providers.Claims/ClaimsLogic.cs (13)
113-136:LGTM! Defensive guard prevents parsing errors.The null/whitespace check on
permission.Dataprevents exceptions when parsing role IDs from an empty or null string. The fallback to View-only claims when no valid data exists is a safe, restrictive default.
200-223:LGTM! Consistent defensive pattern applied.The same null/whitespace guard pattern is correctly applied for log permissions, maintaining consistency with the call permission logic reviewed earlier.
281-304:LGTM! Defensive pattern applied for shift permissions.The null/whitespace guard is correctly applied for shift permissions, consistent with other permission types.
450-475:LGTM! Defensive pattern applied for message permissions.The null/whitespace guard is correctly applied, though note that the fallback includes Delete claims in addition to View (lines 472-473), which differs slightly from other permission types that grant only View.
561-584:LGTM! Defensive pattern applied for document permissions.The null/whitespace guard is correctly applied for document permissions, maintaining consistency.
640-663:LGTM! Defensive pattern applied for note permissions.The null/whitespace guard is correctly applied for note permissions, maintaining consistency.
719-742:LGTM! Defensive pattern applied for schedule permissions.The null/whitespace guard is correctly applied for schedule permissions, maintaining consistency.
798-821:LGTM! Defensive pattern applied for training permissions.The null/whitespace guard is correctly applied for training permissions, maintaining consistency.
893-910:LGTM! Defensive pattern applied for PII permissions.The null/whitespace guard is correctly applied for personal information permissions. Note the outer condition on line 893 already checks for non-admin and non-null data, and the inner check at line 895 provides additional safety.
966-989:LGTM! Defensive pattern applied for inventory permissions.The null/whitespace guard is correctly applied for inventory permissions, maintaining consistency with the pattern across all permission types.
1104-1116:LGTM! Defensive pattern applied for contact view permissions.The null/whitespace guard is correctly applied for contact viewing permissions, maintaining consistency.
1148-1161:LGTM! Defensive pattern applied for contact edit permissions.The null/whitespace guard is correctly applied for contact editing permissions, maintaining consistency.
1193-1205:LGTM! Defensive pattern applied for contact delete permissions.The null/whitespace guard is correctly applied for contact deletion permissions, completing the consistent application of this defensive pattern across all permission types.
Web/Resgrid.Web.Services/Controllers/v4/AvatarsController.cs (2)
45-57:LGTM! Clean avatar retrieval with sensible default.The Get endpoint correctly handles both the default avatar type and custom types, returning a default profile image when no avatar exists. The use of the static lazy-loaded default is efficient.
172-178:LGTM! Clean lazy initialization pattern.The GetDefaultProfileImage method correctly uses lazy initialization for the static default profile image resource.
Web/Resgrid.Web/Areas/User/Views/Home/EditUserProfile.cshtml (1)
295-295:LGTM! Avatar endpoint updated to v4.The avatar image URL has been correctly updated from v3 to v4, consistent with the introduction of the new AvatarsController v4 and similar updates across other views.
Web/Resgrid.Web/Areas/User/Views/Shared/_Navigation.cshtml (1)
7-7:LGTM! Avatar endpoint updated to v4 in navigation.The navigation bar avatar image URL has been correctly updated from v3 to v4, maintaining consistency with the new AvatarsController v4.
Web/Resgrid.Web/Areas/User/Views/Personnel/ViewPerson.cshtml (1)
33-33:LGTM! Avatar endpoint updated to v4.The profile avatar image URL has been correctly updated from v3 to v4, consistent with the avatar API migration.
Core/Resgrid.Services/MessageService.cs (2)
50-54:LGTM! Inbox messages now ordered by date.The addition of
OrderByDescending(x => x.SentOn)ensures inbox messages are returned with newest first, improving user experience and consistency with the unread messages query at line 60.
63-71:LGTM! Sent messages now ordered by date.The addition of
OrderByDescending(x => x.SentOn)ensures sent messages are returned with newest first, providing consistent behavior with the inbox ordering.Web/Resgrid.Web/Areas/User/Views/Personnel/ViewRole.cshtml (1)
45-45:LGTM! Avatar endpoint updated to v4 in role view.The avatar image URL for role users has been correctly updated from v3 to v4, consistent with the avatar API migration across the application.
Web/Resgrid.Web/Areas/User/Views/Dispatch/Chat.cshtml (1)
169-169:LGTM! Avatar endpoint updated to v4 in chat interface.The avatar image URL in the chat user list has been correctly updated from v3 to v4, completing the consistent migration of avatar endpoints across all UI components.
| MobileCarriers.CellC, | ||
| MobileCarriers.TMobile |
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.
Inconsistency: TMobile added to DirectSendCarriers but CarriersMap still uses email gateway.
AddingMobileCarriers.TMobile toDirectSendCarriers implies it should use direct SMS API, but line 195 inCarriersMap still maps TMobile to the email gateway"{0}@tmomail.net". This is inconsistent with other carriers that use direct send (e.g., TracFone at line 197, BellMobility at line 203, TelusMobility at line 204 all map to"Direct").
This mismatch could cause TMobile messages to be routed incorrectly depending on which mapping the SMS sending logic consults first.
Apply this diff to align the CarriersMap with the DirectSendCarriers configuration:
-{MobileCarriers.TMobile, "{0}@tmomail.net"},+{MobileCarriers.TMobile, "Direct"},
Alternatively, if TMobile should continue using the email gateway, remove it from DirectSendCarriers instead.
🤖 Prompt for AI Agents
In Core/Resgrid.Model/MobileCarriers.cs around lines 305-306 (TMobile added toDirectSendCarriers) and CarriersMap at line ~195 (TMobile mapped to"{0}@tmomail.net"), there's an inconsistency: either update the CarriersMapentry for TMobile to use "Direct" (to match DirectSendCarriers) or removeTMobile from the DirectSendCarriers list if it should continue using the emailgateway; pick one approach and make the corresponding change so both lists agree(update the mapping to "Direct" if you want direct API sending, otherwise removeTMobile from DirectSendCarriers).| publicasyncTask<ActionResult>Upload([FromQuery]stringid,int?type) | ||
| { | ||
| varimg=HttpContext.Request.Form.Files.Count>0? | ||
| HttpContext.Request.Form.Files[0]:null; | ||
| // check for a valid mediatype | ||
| if(!img.ContentType.StartsWith("image/")) | ||
| returnBadRequest(); | ||
| // load the image from the upload and generate a new filename | ||
| //var image = Image.FromStream(img.OpenReadStream()); | ||
| varextension=Path.GetExtension(img.FileName); | ||
| byte[]imgArray; | ||
| intwidth=0; | ||
| intheight=0; | ||
| using(Imageimage=Image.Load(img.OpenReadStream())) | ||
| { | ||
| //image.Mutate(x => x | ||
| // .Resize(image.Width / 2, image.Height / 2) | ||
| // .Grayscale()); | ||
| width=image.Width; | ||
| height=image.Height; | ||
| MemoryStreamms=newMemoryStream(); | ||
| awaitimage.SaveAsPngAsync(ms); | ||
| imgArray=ms.ToArray(); | ||
| //image.Save()"output/fb.png"); // Automatic encoder selected based on extension. | ||
| } | ||
| //ImageConverter converter = new ImageConverter(); | ||
| //byte[] imgArray = (byte[])converter.ConvertTo(image, typeof(byte[])); | ||
| if(type==null) | ||
| await_imageService.SaveImageAsync(ImageTypes.Avatar,id,imgArray); | ||
| else | ||
| await_imageService.SaveImageAsync((ImageTypes)type.Value,id,imgArray); | ||
| varbaseUrl=Config.SystemBehaviorConfig.ResgridApiBaseUrl; | ||
| stringurl; | ||
| if(type==null) | ||
| url=baseUrl+"/api/v4/Avatars/Get?id="+id; | ||
| else | ||
| url=baseUrl+"/api/v4/Avatars/Get?id="+id+"?type="+type.Value; | ||
| varobj=new | ||
| { | ||
| status=CroppicStatuses.Success, | ||
| url=url, | ||
| width=width, | ||
| height=height | ||
| }; | ||
| returnCreatedAtAction(nameof(Upload),new{id=obj.url},obj); | ||
| } |
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.
Verify URL construction and consider additional validation.
The Upload endpoint has a few areas to review:
Line 110: URL construction may produce malformed URLs. When
typeis not null, the URL becomes...?id=X?type=Y(two question marks). This should be&type=instead:-url = baseUrl + "/api/v4/Avatars/Get?id=" + id + "?type=" + type.Value;+url = baseUrl + "/api/v4/Avatars/Get?id=" + id + "&type=" + type.Value;
Line 69: Content type validation only checks prefix
"image/". Consider validating against a whitelist of allowed types (e.g.,image/jpeg,image/png,image/gif) to prevent potential issues with exotic image formats.Lines 79-93: The
usingstatement correctly disposes of the Image, but consider wrapping the MemoryStream in ausingstatement as well for explicit disposal.
Apply this diff to fix the URL construction issue:
if (type == null) url = baseUrl + "/api/v4/Avatars/Get?id=" + id; else-url = baseUrl + "/api/v4/Avatars/Get?id=" + id + "?type=" + type.Value;+url = baseUrl + "/api/v4/Avatars/Get?id=" + id + "&type=" + type.Value;
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| publicasyncTask<ActionResult>Upload([FromQuery]stringid,int?type) | |
| { | |
| varimg=HttpContext.Request.Form.Files.Count>0? | |
| HttpContext.Request.Form.Files[0]:null; | |
| // check for a valid mediatype | |
| if(!img.ContentType.StartsWith("image/")) | |
| returnBadRequest(); | |
| // load the image from the upload and generate a new filename | |
| //var image = Image.FromStream(img.OpenReadStream()); | |
| varextension=Path.GetExtension(img.FileName); | |
| byte[]imgArray; | |
| intwidth=0; | |
| intheight=0; | |
| using(Imageimage=Image.Load(img.OpenReadStream())) | |
| { | |
| //image.Mutate(x => x | |
| // .Resize(image.Width / 2, image.Height / 2) | |
| // .Grayscale()); | |
| width=image.Width; | |
| height=image.Height; | |
| MemoryStreamms=newMemoryStream(); | |
| awaitimage.SaveAsPngAsync(ms); | |
| imgArray=ms.ToArray(); | |
| //image.Save()"output/fb.png"); // Automatic encoder selected based on extension. | |
| } | |
| //ImageConverter converter = new ImageConverter(); | |
| //byte[] imgArray = (byte[])converter.ConvertTo(image, typeof(byte[])); | |
| if(type==null) | |
| await_imageService.SaveImageAsync(ImageTypes.Avatar,id,imgArray); | |
| else | |
| await_imageService.SaveImageAsync((ImageTypes)type.Value,id,imgArray); | |
| varbaseUrl=Config.SystemBehaviorConfig.ResgridApiBaseUrl; | |
| stringurl; | |
| if(type==null) | |
| url=baseUrl+"/api/v4/Avatars/Get?id="+id; | |
| else | |
| url=baseUrl+"/api/v4/Avatars/Get?id="+id+"?type="+type.Value; | |
| varobj=new | |
| { | |
| status=CroppicStatuses.Success, | |
| url=url, | |
| width=width, | |
| height= height | |
| }; | |
| returnCreatedAtAction(nameof(Upload),new{id=obj.url},obj); | |
| } | |
| varbaseUrl=Config.SystemBehaviorConfig.ResgridApiBaseUrl; | |
| stringurl; | |
| if(type==null) | |
| url=baseUrl+"/api/v4/Avatars/Get?id="+id; | |
| else | |
| url=baseUrl+"/api/v4/Avatars/Get?id="+id+"&type="+type.Value; | |
| varobj=new | |
| { | |
| status=CroppicStatuses.Success, | |
| url=url, | |
| width=width, | |
| height= height | |
| }; | |
| returnCreatedAtAction(nameof(Upload),new{id=obj.url},obj); |
🤖 Prompt for AI Agents
In Web/Resgrid.Web.Services/Controllers/v4/AvatarsController.cs around lines63-121, fix the malformed URL and tighten resource and content-type handling:build the response URL so the second query parameter uses & (e.g., if type !=null append "&type=" + type.Value) or better use Uri/Query construction to avoiddouble '?' issues; validate uploaded content types against a whitelist (e.g.,allow only image/jpeg, image/png, image/gif) instead of justStartsWith("image/"); and wrap the MemoryStream in a using block so it isdisposed explicitly after image.SaveAsPngAsync(ms) before converting to a bytearray.Uh oh!
There was an error while loading.Please reload this page.
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
Web/Resgrid.Web.Services/Controllers/v4/AvatarsController.cs (3)
69-70:Tighten content type validation.Content type validation only checks if the string starts with
"image/", which could allow exotic or unsupported formats. Consider validating against a whitelist of supported types (e.g.,image/jpeg,image/png,image/gif).Apply this diff to validate against a whitelist:
-if (!img.ContentType.StartsWith("image/"))+var allowedTypes = new[] { "image/jpeg", "image/jpg", "image/png", "image/gif" };+if (!allowedTypes.Contains(img.ContentType.ToLowerInvariant())) return BadRequest();Based on past review comments.
88-90:Wrap MemoryStream in using block.The MemoryStream at line 88 should be wrapped in a
usingstatement for explicit disposal rather than relying on garbage collection.Apply this diff:
-MemoryStream ms = new MemoryStream();-await image.SaveAsPngAsync(ms);-imgArray = ms.ToArray();+using (var ms = new MemoryStream())+{+await image.SaveAsPngAsync(ms);+imgArray = ms.ToArray();+}Based on past review comments.
131-132:Query parsing is fragile.The query parsing at line 132 uses
Replace("?id=", ""), which is fragile and may fail if the query string contains additional parameters or differs in format (e.g.,?id=123&type=1would result in123&type=1).Consider using
System.Web.HttpUtility.ParseQueryStringor a more robust parsing approach:-var originalUri = new Uri(model.imgUrl);-var originalId = originalUri.Query.Replace("?id=", "");+var originalUri = new Uri(model.imgUrl);+var queryParams = System.Web.HttpUtility.ParseQueryString(originalUri.Query);+var originalId = queryParams["id"];Or using built-in URI query parsing if available in your framework version.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Web/Resgrid.Web.Services/Controllers/v4/AvatarsController.cs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.cs: Prefer functional patterns and immutable data where appropriate in C#
Use modern C# features appropriately
Use meaningful, descriptive names for types, methods, and parameters; avoid unclear abbreviations
Separate state from behavior (e.g., use records for state and place operations in separate static classes)
Prefer pure methods over methods with side effects
Use extension methods appropriately for domain-specific operations
Design for testability; avoid hidden dependencies inside methods and prefer explicit, pure functions
Minimize constructor injection; keep the number of injected dependencies small
Prefer composition with interfaces to extend behavior
Files:
Web/Resgrid.Web.Services/Controllers/v4/AvatarsController.cs
🧬 Code graph analysis (1)
Web/Resgrid.Web.Services/Controllers/v4/AvatarsController.cs (3)
Web/Resgrid.Web/Areas/User/Controllers/ConnectController.cs (6)
HttpGet(39-60)HttpGet(62-93)HttpGet(192-225)HttpGet(227-236)HttpGet(238-250)HttpGet(272-301)Core/Resgrid.Config/SystemBehaviorConfig.cs (1)
SystemBehaviorConfig(11-209)Core/Resgrid.Framework/Logging.cs (2)
Logging(15-178)LogException(65-77)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (csharp)
- GitHub Check: Analyze (csharp)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
Web/Resgrid.Web.Services/Controllers/v4/AvatarsController.cs (6)
28-34:LGTM!Constructor injection is minimal with a single dependency, and the static field for caching the default profile image is a sensible performance optimization.
36-57:LGTM!The Get endpoint correctly retrieves avatar images and gracefully falls back to a default profile image when no avatar is found. Content type handling is appropriate.
107-110:URL construction issue resolved.The URL construction now correctly uses
"&type="for the second query parameter, fixing the previous issue where a double question mark would have been generated.Based on past review comments.
138-159:Critical issues from past review resolved.The Crop method now correctly:
- Wraps all MemoryStreams and Image instances in
usingblocks for proper disposal (lines 138-151)- Assigns
imgArray = ms2.ToArray()using the correct MemoryStream (line 149)- Logs exceptions with context and returns an appropriate 500 error response (lines 155-159)
Based on past review comments.
170-176:LGTM!The lazy initialization pattern for the default profile image is appropriate and provides good performance by avoiding repeated embedded resource reads.
179-183:LGTM!The
CroppicStatuseshelper class appropriately defines status constants with internal visibility.
Approve |
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.
This PR is approved.
713677c intomasterUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Style
Documentation