chore(sonar): hoist constant array in reschedule test (CA1861) (#243)#402
Conversation
Clears the last CA1861 code smell (external_roslyn:CA1861, introduced by #398's coverage tests): hoists the constant `{ "Monday", "Tuesday" }` array literal passed as the reschedule payload's `days` argument into a descriptively-named `private static readonly string[]` field, matching the #387/#397 pattern. Behavior-preserving: same values, order, and element type; full suite green. Refs thomasluizon/orbit-ui-mobile#243 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review: PR #402
Scope: PR #402 in thomasluizon/orbit-api (chore(sonar): hoist constant array in reschedule test (CA1861) (#243))
Recommendation: APPROVE
Summary
Single-file, test-only chore: hoists the inline new[] { "Monday", "Tuesday" } array literal in AiRescheduleSuggestionServiceGenerationTests.GenerateAsync_ValidPayload_ReturnsClampedSuggestion into a private static readonly string[] SuggestedDayNames field, matching the existing UserId/Today static-field pattern already used in the same class. This clears a Sonar CA1861 ("prefer static readonly over inline array") smell with zero behavioral change — the serialized JSON payload and the test's assertions are unchanged.
Findings
Critical
None.
High
None.
Medium
None.
Low / Info
None — nothing worth surfacing under the rubric's signal gate for a mechanical, zero-behavior-change hoist.
Subagents
| Agent | Verdict |
|---|---|
| security-reviewer | N/A — diff touches only tests/, no src/ code changed |
| contract-aligner | N/A — no DTO, Controller route, or packages/shared surface touched |
Validation
| Check | Result |
|---|---|
| Build (dotnet) | N/A — skipped per CI adaptation; Build runs as a separate required check |
| Tests (dotnet) | N/A — skipped per CI adaptation; Unit Tests runs as a separate required check |
Deferred — N/A dimensions & files not verdicted
- Dimension 8 (DESIGN.md/AI-slop) — N/A, no
apps/*UI files in diff. - Dimension 9 (Parity) — N/A, frontend-only dimension; no
apps/**files in diff. - Dimension 10 (i18n) — N/A, frontend-only dimension; no user-facing strings in diff.
- Dimension 11 (Contract drift) — N/A, no DTO/schema files in diff; not verifiable in CI (sibling
orbit-ui-mobilerepo not checked out) but moot since nothing in that surface changed. - Dimension 14 (FEATURES.md parity) — N/A, pure test refactor with no user-facing behavior change.
- All other dimensions (1-7, 12, 13) were checked against the one changed file,
tests/Orbit.Infrastructure.Tests/Services/AiRescheduleSuggestionServiceGenerationTests.cs, and verdicted clean. - Every changed file in the diff (1 of 1) received a verdict — nothing deferred there.
What's good
- Matches the existing static-readonly-field convention already used for
UserIdandTodayin the same test class, rather than inventing a new pattern. - Minimal, single-purpose diff scoped exactly to the Sonar finding it's fixing — no scope creep.
Recommendation
Safe to merge as-is. No action required.
|
1 participant
Clears the last CA1861 code smell (introduced by #398's tests). Refs thomasluizon/orbit-ui-mobile#243