-
-
Notifications
You must be signed in to change notification settings - Fork 76
CU-868fnz2a8 Fixed Voice bug, updated Countly and changed Calendar to… #247
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughExpands calendar upcoming range from 7 to 14 days. Sanitizes NatureOfCall text in Twilio controller outputs using StringHelpers.StripHtmlTagsCharArray. Enhances Countly initialization in the user layout with additional telemetry options (debug, tracking, consent, error tracking, force_post, ignore_bots). No public API surface changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User Browser
participant C as Countly SDK
participant S as Countly Server
Note over U,C: Page load
U->>C: initialize({ app_key, url, debug, track_* , require_consent,<br/>force_post, ignore_bots, ignore_referrers })
alt require_consent enabled
C-->>U: Await user consent
opt consent granted
C->>S: POST session start / pageview (force_post)
C->>S: POST link/form/error events (as configured)
end
else consent not required
C->>S: POST session start / pageview (force_post)
C->>S: POST link/form/error events (as configured)
end
Note over C,S: ignore_bots and ignore_referrers applied client-side
sequenceDiagram
autonumber
participant T as TwilioController
participant H as StringHelpers
participant R as Responder
R->>T: Request (SMS/Voice)
T->>T: Retrieve call data (Name, NatureOfCall, etc.)
T->>H: StripHtmlTagsCharArray(NatureOfCall)
H-->>T: SanitizedNature
T->>R: Compose response using sanitized NatureOfCall
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Core/Resgrid.Services/CalendarService.cs (1)
230-231: Bug: timezone set on base item instead of recurrence item inside loopThese two assignments target calendarItem, not calItem, so recurrences won’t carry the updated time zone.
- calendarItem.StartTimezone = timeZone; - calendarItem.EndTimezone = timeZone; + calItem.StartTimezone = timeZone; + calItem.EndTimezone = timeZone;
🧹 Nitpick comments (6)
Core/Resgrid.Services/CalendarService.cs (3)
73-73: 14‑day window looks fine; confirm desired range semanticsUnderlying range filter uses Start >= startDate AND End <= endDate. Long or spanning events that cross the window edges will be excluded. If you intend “events starting in the next 14 days” or “any overlap with the window,” consider adjusting the predicate (see next comment).
54-58: Use overlap or “starts in window” predicate to avoid dropping spanning eventsToday: ci.Start >= startDate && ci.End <= endDate. Suggested options:
- Starts-in-window: ci.Start >= startDate && ci.Start <= endDate
- Any-overlap: ci.Start < endDate && ci.End > startDate
Apply one of these if that matches product intent.
- where ci.Start >= startDate && ci.End <= endDate + // Option A: starts within window + where ci.Start >= startDate && ci.Start <= endDateor
- where ci.Start >= startDate && ci.End <= endDate + // Option B: any overlap with window + where ci.Start < endDate && ci.End > startDate
11-11: Remove unused Twilio using from services layerTwilio.TwiML.Voice isn’t used here; drop the using to keep dependencies lean.
Web/Resgrid.Web/Areas/User/Views/Shared/_UserLayout.cshtml (1)
68-80: Countly feature flags may be ignored; call track methods explicitlyThe Web SDK typically expects calling APIs like
Countly.track_sessions()/track_pageview()rather than booleans in init. To be safe, keep init minimal and call features after:- Countly.init({ - app_key: "...", - url: "...", - debug: false, - ignore_referrers: [], - track_sessions: true, - track_pageview: true, - track_links: true, - track_forms: true, - track_errors: true, - require_consent: false, - force_post: true, - ignore_bots: true - }); + Countly.init({ + app_key: "...", + url: "...", + debug: false, + ignore_referrers: [], + require_consent: false, + force_post: true, + ignore_bots: true + }); + Countly.track_sessions(); + Countly.track_pageview(); + Countly.track_links(); + Countly.track_forms(); + Countly.track_errors();Please verify your SDK version supports the boolean options if you prefer that style.
Web/Resgrid.Web.Services/Controllers/TwilioController.cs (2)
523-526: Sanitize Nature in both branchesElse branch still uses raw
call.NatureOfCall. Sanitize for consistency and to avoid odd TTS output.- else - sb.Append(string.Format("{0}, Priority {1} Nature {2}", call.Name, call.GetPriorityText(), call.NatureOfCall)); + else + sb.Append(string.Format("{0}, Priority {1} Nature {2}", call.Name, call.GetPriorityText(), StringHelpers.StripHtmlTagsCharArray(call.NatureOfCall)));
353-355: Also sanitize Call Detail “Nature” in SMS responseThis path still returns raw HTML. Align with other changes.
- callText.Append("-----Nature-----" + Environment.NewLine); - callText.Append(call.NatureOfCall + Environment.NewLine); + callText.Append("-----Nature-----" + Environment.NewLine); + callText.Append(StringHelpers.StripHtmlTagsCharArray(call.NatureOfCall) + Environment.NewLine);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Core/Resgrid.Services/CalendarService.cs(1 hunks)Web/Resgrid.Web.Services/Controllers/TwilioController.cs(3 hunks)Web/Resgrid.Web/Areas/User/Views/Shared/_UserLayout.cshtml(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/TwilioController.csCore/Resgrid.Services/CalendarService.cs
🧬 Code graph analysis (1)
Web/Resgrid.Web.Services/Controllers/TwilioController.cs (2)
Core/Resgrid.Framework/StringHelpers.cs (2)
StringHelpers(22-331)StripHtmlTagsCharArray(27-58)Core/Resgrid.Model/Call.cs (1)
GetPriorityText(251-276)
⏰ 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). (3)
- GitHub Check: build
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (csharp)
🔇 Additional comments (4)
Core/Resgrid.Services/CalendarService.cs (1)
521-523: Verify DateTime.Within usage
if (timestamp == convertedTime.Within(TimeSpan.FromMinutes(30)))reads odd unless Within returns a DateTime. If Within returns bool, this is a logic bug. Please confirm the extension’s signature and adjust accordingly.Web/Resgrid.Web/Areas/User/Views/Shared/_UserLayout.cshtml (1)
68-80: Confirm consent requirements
require_consent: falsemay conflict with GDPR/CCPA in some regions. Confirm legal posture and consider gating tracking until consent is recorded.Web/Resgrid.Web.Services/Controllers/TwilioController.cs (2)
320-321: Good hardening: sanitize Nature for SMS active-calls listUsing
StripHtmlTagsCharArrayhere prevents HTML artifacts in outbound texts.
713-714: Good hardening: sanitize Nature in inbound-voice active-calls listConsistent with SMS path; avoids speaking HTML tags.
|
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.
… 14 days.
Summary by CodeRabbit