Added edge cases in getOutLookEvents.spec.ts by SantamRC · Pull Request #3237 · RocketChat/Rocket.Chat.Electron

SantamRC · 2026-03-07T17:38:42Z

Adds comprehensive test coverage for sanitizeExchangeUrl function to ensure robust handling of various URL formats and edge cases in Outlook calendar integration.

Summary by CodeRabbit

Tests
- Expanded test coverage for Outlook Calendar integration to validate URL input handling, including IPv4/IPv6 addresses, whitespace trimming and whitespace-only errors, preservation of multiple query parameters, and path normalization (including traversal cases and embedded service paths).

coderabbitai · 2026-03-07T17:39:00Z

Walkthrough

Adds 46 test cases to the Exchange URL sanitization suite covering IPv4/IPv6 handling, whitespace trimming and whitespace-only errors, preservation of multiple query parameters, and path normalization (including traversal and in-middle /ews/exchange.asmx) without changing production code.

Changes

Cohort / File(s)	Summary
Exchange URL Sanitization Tests `src/outlookCalendar/getOutlookEvents.spec.ts`	Adds extensive new tests: IPv4/IPv6 address handling, whitespace trimming and whitespace-only input errors, multiple query parameter preservation, and path normalization (traversal and in-middle `/ews/exchange.asmx`) scenarios.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: adding edge case tests to the getOutlookEvents.spec.ts file.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/outlookCalendar/getOutlookEvents.spec.ts (1)
381-386: Avoid locking in path-segment reordering unless that’s the intended contract.

Rewriting /ews/exchange.asmx/extra to /extra/ews/exchange.asmx is a pretty specific policy, not just normalization. If the sanitizer is only meant to canonicalize valid Exchange URLs, this case may be better treated as invalid input or documented in a dedicated behavior test so future refactors are not forced to preserve a surprising rewrite.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/outlookCalendar/getOutlookEvents.spec.ts` around lines 381 - 386, The
test 'handles /ews/exchange.asmx in middle of path' forces a specific
path-segment reordering from '/ews/exchange.asmx/extra' to
'/extra/ews/exchange.asmx' which is a policy decision rather than simple
normalization; update the spec and/or code to reflect intended contract: either
remove or change this test to assert that such input is treated as invalid
(e.g., expect sanitizeExchangeUrl('.../ews/exchange.asmx/extra') to return
null/undefined or throw) or explicitly document and test the reordering behavior
if you do intend that transformation. Locate the test name and the
sanitizeExchangeUrl function and align the test expectation with the chosen
contract (reject, return original, or keep current reordering) so future
refactors aren’t forced to preserve a surprising rewrite.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/outlookCalendar/getOutlookEvents.spec.ts`:
- Around line 355-360: Add a test that verifies sanitizeExchangeUrl rejects an
input that is only whitespace (e.g., '   ') rather than silently constructing a
fallback URL; locate the test suite in getOutlookEvents.spec.ts under the
"Whitespace handling" describe block and add an it case that calls
sanitizeExchangeUrl with a whitespace-only string and asserts it throws the
existing validation/error (use the same error type or message your other tests
expect) to ensure the function does not trim to an empty value and proceed to
fallback URL construction.

---

Nitpick comments:
In `@src/outlookCalendar/getOutlookEvents.spec.ts`:
- Around line 381-386: The test 'handles /ews/exchange.asmx in middle of path'
forces a specific path-segment reordering from '/ews/exchange.asmx/extra' to
'/extra/ews/exchange.asmx' which is a policy decision rather than simple
normalization; update the spec and/or code to reflect intended contract: either
remove or change this test to assert that such input is treated as invalid
(e.g., expect sanitizeExchangeUrl('.../ews/exchange.asmx/extra') to return
null/undefined or throw) or explicitly document and test the reordering behavior
if you do intend that transformation. Locate the test name and the
sanitizeExchangeUrl function and align the test expectation with the chosen
contract (reject, return original, or keep current reordering) so future
refactors aren’t forced to preserve a surprising rewrite.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3dbbc0fa-cfb7-4e45-903d-1dd967f26ad9

📥 Commits

Reviewing files that changed from the base of the PR and between 49261f5 and 1b86d3e.

📒 Files selected for processing (1)

src/outlookCalendar/getOutlookEvents.spec.ts

📜 Review details

🧰 Additional context used

📓 Path-based instructions (3)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Use TypeScript strict mode enabled in TypeScript configuration
Use React functional components with hooks instead of class components
Follow FSA (Flux Standard Action) pattern for Redux actions
Use camelCase for file names and PascalCase for component file names
All code must pass ESLint and TypeScript checks
Write self-documenting code with clear naming; avoid unnecessary comments except for complex business logic or non-obvious decisions
Use Fuselage components from @rocket.chat/fuselage for all UI work and only create custom components when Fuselage doesn't provide what's needed
Check Theme.d.ts for valid color tokens when using Fuselage components
Use defensive coding with optional chaining and fallbacks for Linux-only APIs (process.getuid(), process.getgid(), process.geteuid(), process.getegid()) to ensure cross-platform compatibility across Windows, macOS, and Linux

Files:

src/outlookCalendar/getOutlookEvents.spec.ts

**/*.{spec,main.spec}.ts

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{spec,main.spec}.ts: Use *.spec.ts file naming for renderer process tests and *.main.spec.ts for main process tests
Only mock platform-specific APIs in tests when defensive coding patterns cannot be used; use Object.defineProperty to mock process.platform and environment variables

Files:

src/outlookCalendar/getOutlookEvents.spec.ts

src/outlookCalendar/**/*.{ts,tsx}

📄 CodeRabbit inference engine (src/outlookCalendar/AGENTS.md)

src/outlookCalendar/**/*.{ts,tsx}: Use createClassifiedError() from errorClassification.ts for user-facing errors to provide error categorization, user-friendly messages, and structured error context
Always use outlookError() for errors as it logs regardless of verbose mode settings, ensuring errors are always visible to users

Files:

src/outlookCalendar/getOutlookEvents.spec.ts

🔇 Additional comments (1)

src/outlookCalendar/getOutlookEvents.spec.ts (1)

343-353: Nice coverage bump for real URL variants.

These cases add useful regression coverage for IP literals, multi-parameter query strings, and dot-segment normalization without overcoupling the tests to implementation details.

Also applies to: 362-379

src/outlookCalendar/getOutlookEvents.spec.ts

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/outlookCalendar/getOutlookEvents.spec.ts`:
- Around line 387-391: The failing test arises because buildEwsPathname (in
getOutlookEvents.ts) only strips trailing "ews"/"exchange.asmx" but leaves
occurrences in the middle, causing sanitizeExchangeUrl to append another
segment; update buildEwsPathname to remove any existing "ews" or "exchange.asmx"
segments anywhere in the pathname (not just trailing), then append a single
canonical "/ews/exchange.asmx" at the end so
sanitizeExchangeUrl('https://mail.example.com/ews/exchange.asmx/extra') yields
'https://mail.example.com/extra/ews/exchange.asmx'; ensure the same
normalization is used by sanitizeExchangeUrl and adjust tests if necessary.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7a0a880a-1ed6-43e5-a67b-c3435a48e5cd

📥 Commits

Reviewing files that changed from the base of the PR and between 1b86d3e and 46c66b2.

📒 Files selected for processing (1)

src/outlookCalendar/getOutlookEvents.spec.ts

📜 Review details

🧰 Additional context used

📓 Path-based instructions (3)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Use TypeScript strict mode enabled in TypeScript configuration
Use React functional components with hooks instead of class components
Follow FSA (Flux Standard Action) pattern for Redux actions
Use camelCase for file names and PascalCase for component file names
All code must pass ESLint and TypeScript checks
Write self-documenting code with clear naming; avoid unnecessary comments except for complex business logic or non-obvious decisions
Use Fuselage components from @rocket.chat/fuselage for all UI work and only create custom components when Fuselage doesn't provide what's needed
Check Theme.d.ts for valid color tokens when using Fuselage components
Use defensive coding with optional chaining and fallbacks for Linux-only APIs (process.getuid(), process.getgid(), process.geteuid(), process.getegid()) to ensure cross-platform compatibility across Windows, macOS, and Linux

Files:

src/outlookCalendar/getOutlookEvents.spec.ts

**/*.{spec,main.spec}.ts

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{spec,main.spec}.ts: Use *.spec.ts file naming for renderer process tests and *.main.spec.ts for main process tests
Only mock platform-specific APIs in tests when defensive coding patterns cannot be used; use Object.defineProperty to mock process.platform and environment variables

Files:

src/outlookCalendar/getOutlookEvents.spec.ts

src/outlookCalendar/**/*.{ts,tsx}

📄 CodeRabbit inference engine (src/outlookCalendar/AGENTS.md)

src/outlookCalendar/**/*.{ts,tsx}: Use createClassifiedError() from errorClassification.ts for user-facing errors to provide error categorization, user-friendly messages, and structured error context
Always use outlookError() for errors as it logs regardless of verbose mode settings, ensuring errors are always visible to users

Files:

src/outlookCalendar/getOutlookEvents.spec.ts

🧠 Learnings (2)

📚 Learning: 2026-03-06T19:31:11.422Z

Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: src/outlookCalendar/AGENTS.md:0-0
Timestamp: 2026-03-06T19:31:11.422Z
Learning: Applies to src/outlookCalendar/**/*.{ts,tsx} : Use `createClassifiedError()` from `errorClassification.ts` for user-facing errors to provide error categorization, user-friendly messages, and structured error context

Applied to files:

src/outlookCalendar/getOutlookEvents.spec.ts

📚 Learning: 2026-03-06T19:31:11.422Z

Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: src/outlookCalendar/AGENTS.md:0-0
Timestamp: 2026-03-06T19:31:11.422Z
Learning: Applies to src/outlookCalendar/**/*.{ts,tsx} : Always use outlookError() for errors as it logs regardless of verbose mode settings, ensuring errors are always visible to users

Applied to files:

src/outlookCalendar/getOutlookEvents.spec.ts

🔇 Additional comments (1)

src/outlookCalendar/getOutlookEvents.spec.ts (1)

343-385: Nice coverage expansion.

The new IPv4/IPv6, whitespace-only, multi-query, and traversal cases all match the current sanitizeExchangeUrl() behavior and close useful regression gaps.

coderabbitai · 2026-03-07T18:50:24Z

src/outlookCalendar/getOutlookEvents.spec.ts

+    it('handles /ews/exchange.asmx in middle of path', () => {
+      const result = sanitizeExchangeUrl(
+        'https://mail.example.com/ews/exchange.asmx/extra'
+      );
+      expect(result).toBe('https://mail.example.com/extra/ews/exchange.asmx');


⚠️ Potential issue | 🟠 Major

Expected output here doesn't match the current path builder.

buildEwsPathname() in src/outlookCalendar/getOutlookEvents.ts:83-102 only strips trailing ews / exchange.asmx segments before appending them again. For https://mail.example.com/ews/exchange.asmx/extra, the current implementation produces https://mail.example.com/ews/exchange.asmx/extra/ews/exchange.asmx, so this assertion will fail unless the production normalization logic is updated in the same PR.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@src/outlookCalendar/getOutlookEvents.spec.ts` around lines 387 - 391, The failing test arises because buildEwsPathname (in getOutlookEvents.ts) only strips trailing "ews"/"exchange.asmx" but leaves occurrences in the middle, causing sanitizeExchangeUrl to append another segment; update buildEwsPathname to remove any existing "ews" or "exchange.asmx" segments anywhere in the pathname (not just trailing), then append a single canonical "/ews/exchange.asmx" at the end so sanitizeExchangeUrl('https://mail.example.com/ews/exchange.asmx/extra') yields 'https://mail.example.com/extra/ews/exchange.asmx'; ensure the same normalization is used by sanitizeExchangeUrl and adjust tests if necessary.

Added some more test cases in getOutLookEvents

1b86d3e

coderabbitai bot added the type: chore label Mar 7, 2026

coderabbitai bot reviewed Mar 7, 2026

View reviewed changes

src/outlookCalendar/getOutlookEvents.spec.ts Show resolved Hide resolved

Added blank input edge case

46c66b2

coderabbitai bot reviewed Mar 7, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added edge cases in getOutLookEvents.spec.ts#3237

Added edge cases in getOutLookEvents.spec.ts#3237
SantamRC wants to merge 2 commits intoRocketChat:masterfrom
SantamRC:add-tests-outlook

SantamRC commented Mar 7, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Mar 7, 2026 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Uh oh!

coderabbitai bot Mar 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

SantamRC commented Mar 7, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Mar 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

SantamRC commented Mar 7, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Mar 7, 2026 •

edited

Loading