Refactor: shift admin lookups and camp link parsing#102
Merged
peterdrier merged 2 commits intomainfrom Apr 1, 2026
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
peterdrier
added a commit
that referenced
this pull request
Apr 8, 2026
- Item 1: Remove SystemClock.Instance bypass in Application state machine OnEntry callbacks; set ResolvedAt explicitly in Approve/Reject/Withdraw using injected IClock - Item 2: Remove dead Profile.ComputeMembershipStatus method and its tests (superseded by MembershipCalculator service) - Item 3: Replace DateTime.UtcNow with IClock in ProfileController and GuestController (file name generation and ticketing year display) - Item 6: Extract IVolunteerHistoryService interface in Application layer; register via interface in DI - Item 7: Add DomainConstants.GoogleGroupDomain constant; replace hardcoded "@nobodies.team" in Team.GoogleGroupEmail - Item 8: Remove SyncAllPermissionsAsync no-op stub from GoogleResourceProvisionJob (and unused IClock dependency) - Item 9: Change all 14 Hangfire jobs from concrete HumansMetricsService to IHumansMetrics interface - Item 12: Remove dead DbUpdateConcurrencyException catch blocks from ApplicationDecisionService (concurrency tokens were removed) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
peterdrier
added a commit
that referenced
this pull request
Apr 8, 2026
Item 4: Add tests for ProcessAccountDeletionsJob (12 tests) and SuspendNonCompliantMembersJob (10 tests) covering GDPR anonymization, grace periods, event holds, email notifications, cache invalidation, error resilience, and suspension workflow. Item 10: Add endpoint authorization tests (22 tests) verifying that critical controller actions have correct authorization policies, all POST actions have anti-forgery validation, and all non-anonymous endpoints require authentication. Item 17: Add Application.ValidateTier() domain guard rejecting Volunteer tier, called at both application creation points. Item 20: Extract TempDataKeys constants class, replace all TempData string literals in HumansControllerBase, TempDataAlertsViewComponent, and GoogleController. Item 22: Replace RedirectToAction string literals with nameof() for cross-controller redirects in 7 controllers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
peterdrier
added a commit
that referenced
this pull request
Apr 8, 2026
- Item 16: Replace long tuple return types with dedicated record DTOs (BirthdayProfileInfo, LocationProfileInfo, ReviewQueueData, ReviewDetailData, BoardVotingDashboardData, BoardMemberInfo, ConsentProgressInfo) in IProfileService and IOnboardingService - Item 18: Add CancellationToken to key GET controller actions in ProfileController, TeamController, OnboardingReviewController, AdminController and pass through to service/DB calls - Fix ToLowerInvariant() switch in ProfileService.GetFilteredHumansAsync to use OrdinalIgnoreCase string comparison per coding rules - Remove unnecessary GetCurrentUserAsync reload in RequestDeletion (tracked entity already updated by service) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
peterdrier
added a commit
that referenced
this pull request
Apr 8, 2026
- Item 1: Remove SystemClock.Instance bypass in Application state machine OnEntry callbacks; set ResolvedAt explicitly in Approve/Reject/Withdraw using injected IClock - Item 2: Remove dead Profile.ComputeMembershipStatus method and its tests (superseded by MembershipCalculator service) - Item 3: Replace DateTime.UtcNow with IClock in ProfileController and GuestController (file name generation and ticketing year display) - Item 6: Extract IVolunteerHistoryService interface in Application layer; register via interface in DI - Item 7: Add DomainConstants.GoogleGroupDomain constant; replace hardcoded "@nobodies.team" in Team.GoogleGroupEmail - Item 8: Remove SyncAllPermissionsAsync no-op stub from GoogleResourceProvisionJob (and unused IClock dependency) - Item 9: Change all 14 Hangfire jobs from concrete HumansMetricsService to IHumansMetrics interface - Item 12: Remove dead DbUpdateConcurrencyException catch blocks from ApplicationDecisionService (concurrency tokens were removed) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
peterdrier
added a commit
that referenced
this pull request
Apr 8, 2026
Item 4: Add tests for ProcessAccountDeletionsJob (12 tests) and SuspendNonCompliantMembersJob (10 tests) covering GDPR anonymization, grace periods, event holds, email notifications, cache invalidation, error resilience, and suspension workflow. Item 10: Add endpoint authorization tests (22 tests) verifying that critical controller actions have correct authorization policies, all POST actions have anti-forgery validation, and all non-anonymous endpoints require authentication. Item 17: Add Application.ValidateTier() domain guard rejecting Volunteer tier, called at both application creation points. Item 20: Extract TempDataKeys constants class, replace all TempData string literals in HumansControllerBase, TempDataAlertsViewComponent, and GoogleController. Item 22: Replace RedirectToAction string literals with nameof() for cross-controller redirects in 7 controllers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
peterdrier
added a commit
that referenced
this pull request
Apr 8, 2026
- Item 16: Replace long tuple return types with dedicated record DTOs (BirthdayProfileInfo, LocationProfileInfo, ReviewQueueData, ReviewDetailData, BoardVotingDashboardData, BoardMemberInfo, ConsentProgressInfo) in IProfileService and IOnboardingService - Item 18: Add CancellationToken to key GET controller actions in ProfileController, TeamController, OnboardingReviewController, AdminController and pass through to service/DB calls - Fix ToLowerInvariant() switch in ProfileService.GetFilteredHumansAsync to use OrdinalIgnoreCase string comparison per coding rules - Remove unnecessary GetCurrentUserAsync reload in RequestDeletion (tracked entity already updated by service) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
peterdrier
added a commit
that referenced
this pull request
Apr 9, 2026
* Codebase audit: fix high-priority tech debt items (#102) - Item 1: Remove SystemClock.Instance bypass in Application state machine OnEntry callbacks; set ResolvedAt explicitly in Approve/Reject/Withdraw using injected IClock - Item 2: Remove dead Profile.ComputeMembershipStatus method and its tests (superseded by MembershipCalculator service) - Item 3: Replace DateTime.UtcNow with IClock in ProfileController and GuestController (file name generation and ticketing year display) - Item 6: Extract IVolunteerHistoryService interface in Application layer; register via interface in DI - Item 7: Add DomainConstants.GoogleGroupDomain constant; replace hardcoded "@nobodies.team" in Team.GoogleGroupEmail - Item 8: Remove SyncAllPermissionsAsync no-op stub from GoogleResourceProvisionJob (and unused IClock dependency) - Item 9: Change all 14 Hangfire jobs from concrete HumansMetricsService to IHumansMetrics interface - Item 12: Remove dead DbUpdateConcurrencyException catch blocks from ApplicationDecisionService (concurrency tokens were removed) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Codebase audit: tests, magic strings, and domain guard (#102) Item 4: Add tests for ProcessAccountDeletionsJob (12 tests) and SuspendNonCompliantMembersJob (10 tests) covering GDPR anonymization, grace periods, event holds, email notifications, cache invalidation, error resilience, and suspension workflow. Item 10: Add endpoint authorization tests (22 tests) verifying that critical controller actions have correct authorization policies, all POST actions have anti-forgery validation, and all non-anonymous endpoints require authentication. Item 17: Add Application.ValidateTier() domain guard rejecting Volunteer tier, called at both application creation points. Item 20: Extract TempDataKeys constants class, replace all TempData string literals in HumansControllerBase, TempDataAlertsViewComponent, and GoogleController. Item 22: Replace RedirectToAction string literals with nameof() for cross-controller redirects in 7 controllers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Codebase audit: tuple types, CancellationToken, string comparison (#102) - Item 16: Replace long tuple return types with dedicated record DTOs (BirthdayProfileInfo, LocationProfileInfo, ReviewQueueData, ReviewDetailData, BoardVotingDashboardData, BoardMemberInfo, ConsentProgressInfo) in IProfileService and IOnboardingService - Item 18: Add CancellationToken to key GET controller actions in ProfileController, TeamController, OnboardingReviewController, AdminController and pass through to service/DB calls - Fix ToLowerInvariant() switch in ProfileService.GetFilteredHumansAsync to use OrdinalIgnoreCase string comparison per coding rules - Remove unnecessary GetCurrentUserAsync reload in RequestDeletion (tracked entity already updated by service) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix flaky ProcessAccountDeletionsJob test mock setup NSubstitute mock for audit log wasn't reliably matching user2's call on CI. Restructured to set a default (succeed) then override for user1 (throw), which is deterministic regardless of call order. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix ProcessAccountDeletionsJob: preserve unprocessed entities on failure ChangeTracker.Clear() detached ALL entities including not-yet-processed users, preventing them from being saved in subsequent iterations. Replace with targeted detachment of only modified/added/deleted entries, keeping unchanged entities tracked for subsequent loop iterations. Also fix the test mock to use a callback instead of NSubstitute argument matching, which is more deterministic. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GetRotaForTeamAsync,GetShiftForTeamAsync,GetSignupForTeamAsync,GetSignupBlockForTeamAsync, andParseTagIdshelpers to DRY up ~12 repeated fetch+team-check patterns inShiftAdminControllerParseCampLinksandIsHttpUrlhelpers to deduplicate identical LINQ in Create/Edit actions ofCampControllerDropped the "extract error keys" commit from the original Codex branch — it added 3 constant classes and touched 17 files to solve a problem that hasn't caused bugs.
Test plan
🤖 Generated with Claude Code