Vk/fd84 everything claud - #6
Merged
Merged
Conversation
Diagnoses three data issues in the solution detail page: 1. Frontend tags rendering (JSON string iterated char-by-char) 2. Backend SolutionVO missing stats/flair/badges/topic fields 3. Seed data lacking actual code implementations
…layers Frontend: add parseTags() safe JSON parsing for tags field (was iterating string char-by-char via v-for), fix stats mapping from flat fields instead of missing nested object, add comments to inline type definitions. Backend: extend SolutionVO with likes/dislikes/comments/score/userVote, inject EdgeOperationMapper into SolutionServiceImpl toVO() to populate vote counts from edge_operations and comment count from solution_comments. Database: V13 migration enriches all 8 solution seed data with complete content including approach explanation, full code implementation, and complexity analysis.
The sol-008 content contained unescaped single quotes ('1', '0') that
caused SQL syntax errors during Flyway migration. Escaped as ''1'' and
''0'' per MySQL string literal conventions.
…review issues - Extract SolutionApiItem interface and transformApiSolution() to eliminate 3 duplicate mapping blocks (~105 lines removed, 354→220 lines) - Add .map(String) to parseTags() for type safety against non-string JSON values - Remove unused edge-operations imports from solution.ts - Remove unused userVote field and java.util.List import from SolutionVO.java
DavidHLP
added a commit
that referenced
this pull request
Jun 8, 2026
…_ADMIN + rankings 404 + is_deleted filter + Unicode title + @Audited + new ErrorCode + controller XSS guard + mutator tests) 修复 docs/contests-api-test-report.md 报告的 7 处缺陷 (1 HIGH + 6 MEDIUM/LOW)： 缺陷 #1 (HIGH) — DELETE 软删除 is_deleted 字段未持久化 - 根因：Contest.isDeleted 字段 @TableLogic + mapper.updateById(entity) 自动忽略逻辑删除字段 - 修复：改用 mapper.update(null, LambdaUpdateWrapper.set(...)) 显式写入 缺陷 #2 — PATCH on FINISHED/RUNNING 比赛无状态守卫 - 修复：service 层加 status guard，仅允许 UPCOMING 修改 - 新增 ErrorCode CONTEST_ONLY_UPDATE_UPCOMING (70006) 区分注册与修改语义 缺陷 #3 — service 层 hasRole("ADMIN") 拒绝 SUPER_ADMIN，与控制器 @PreAuthorize 不一致 - 修复：SecurityUtil 新增 hasAnyRole(String...) 工具方法，ContestServiceImpl 7 处 mutator 替换 缺陷 #4 — CreateContestDTO/UpdateContestDTO title @pattern 拒绝 CJK 字符 - 修复：正则放宽为 [\p{L}\p{N}\s\p{P}]+ 接受 Unicode 字母 - AdminContestController 新增 rejectUnsafeTitleChars helper 显式排除 < 和 > 缺陷 #5 — GET /admin/contest/{id}/rankings 对不存在 contest 返 200+空 - 修复：getAdminContestRanking 入口加 contest 存在性 + is_deleted 校验 缺陷 #6 — addProblem/removeProblem/startContest/endContest 未校验 is_deleted - 修复：4 个 mutator 入口加 .filter(c -> !isDeleted).orElseThrow(COMPETITION_NOT_FOUND) 缺陷 #7 — @Audited 注解缺失导致 audit_logs 无 contest 记录 - 修复：7 个 mutator 方法加 @Audited + AuditContext.setOldValues/setNewValues/setUserId - 复用 AuditActionUtil.CREATE_CONTEST/UPDATE_CONTEST/DELETE_CONTEST 常量 新增测试 ContestServiceImplMutatorTest：5 case 覆盖缺陷 #2/#5/#6 的状态守卫 + 软删除过滤 + 404 路径。 缺陷 #1 的 LambdaUpdateWrapper.getSqlSet() 断言需 @SpringBootTest 上下文，保留为 follow-up， 运行时 curl 验证已确认三列同改 (docs/contests-api-test-report.md §3.5)。 验证： - mvnw compile -B：0 错误 - mvnw test -Dtest=ContestServiceImplMutatorTest：5/5 通过，2.0s - 运行时 curl 5/5 critical 缺陷修复确认 (PM2 重启 ulticode-9001) Artifacts: - Plan: .claude/PRPs/plans/completed/admin-contests-mutator-fixes.plan.md - Report: .claude/PRPs/reports/admin-contests-mutator-fixes-report.md - Review v1: .claude/reviews/admin-contests-mutator-review.md - Review v2: .claude/reviews/admin-contests-mutator-review-v2.md - Test report: docs/contests-api-test-report.md
DavidHLP
pushed a commit
that referenced
this pull request
Jun 11, 2026
… handler, compute real progress Fixes 6 issues from docs/achievement-api-test-report-2026-06-11.md plus 2 newly-discovered typeHandler bugs (Bug #7, Bug #8) surfaced during validation. CRITICAL #1 — escape `key` column via @TableField(value = "`key`") - Achievement.java:19 — affects MyBatis-Plus auto-generated column lists in selectById / selectPage / selectBatchIds. - Without this, every SQL that lists the achievements columns fails with 'You have an error in your SQL syntax ... near key,...'. CRITICAL #2 — escape `key` in @select SQL - AchievementMapper.findByKey:24 — WHERE clause now backticks. - Fixes admin POST /achievements which called findByKey for dedup. HIGH #3 — T2b /achievements/{invalid} now returns 404 (was 500) - Follows from CRITICAL #1+#2: the BadSqlGrammarException no longer fires before the business-level ACHIEVEMENT_NOT_FOUND can be raised. MEDIUM #4 — BadSqlGrammarException → DATABASE_ERROR (50001) - GlobalExceptionHandler:227 — new handler preserves root cause in server log while returning generic 'Database error' to clients. - Pairs with existing MyBatisSystemException / BindingException / DataIntegrityViolationException handlers (no fallback to 50000 'Unknown error'). LOW #5 — getUserAchievements progress is no longer hard-coded to 0 - AchievementServiceImpl:280 — mirrors getUserProgress logic, reads criteria.type from JSON Map, dispatches to SubmissionMapper counter. LOW #6 — re-scoped Javadoc: getUserAchievements and getUserProgress are NOT duplicates (verified UserController /me/achievements/progress uses the latter). Both serve different endpoints with different DTO shapes. Bug #7 — findAllActive missing JacksonTypeHandler - AchievementMapper:46 — @select bypassed @TableField(typeHandler= JacksonTypeHandler.class), returning criteria=null. - Converted to default method via BaseMapper.selectList. Bug #8 — findByKey same typeHandler bypass - Discovered by AchievementMapperIT (L2) on first run; findByKey_criteriaIsDeserializedAsMap failed. - AchievementMapper:24 — also converted to default method. - Latent: only caller (create) never read criteria. Refactors - M1 (review): getUserAchievements extracted buildProgressDTO helper (52 → 16 lines). - L1 (review): findAllActive uses LambdaQueryWrapper<Achievement> method refs instead of string column names. - L3 (review): Javadoc clarified the two service methods. Tests + AchievementMapperSQLGuardTest: 3 reflection-guard tests (CRITICAL #1, CRITICAL #2, no @select on findAllActive/findByKey). + AchievementMapperIT: 5 Testcontainers MySQL 8.0 IT tests verifying JacksonTypeHandler runtime behavior + Bug #7/#8 regression guards. + AchievementServiceTest: 3 progress tests + 2 new mock fields (SubmissionMapper, ContestParticipantMapper). + GlobalExceptionHandlerTest: 2 BadSqlGrammarException tests. Validation - compile / package: BUILD SUCCESS - Unit tests: 36/36 pass - Testcontainers IT: 5/5 pass - 8 curl integration tests on worktree:9091: all match expected HTTP codes (200/400/404/409) and real progress values (admin 6 accepted problems → progress=6 for all 3 rows). Refs: docs/achievement-api-test-report-2026-06-11.md, .claude/PRPs/plans/completed/achievement-api-fixes.plan.md, .claude/PRPs/reports/achievement-api-fixes-report.md, .claude/reviews/achievement-api-fixes-review.md
DavidHLP
pushed a commit
that referenced
this pull request
Jun 11, 2026
…dation Merges the fix/achievement-api worktree changes: - CRITICAL #1, #2 (MySQL reserved word `key`) - HIGH #3 (T2b 404) - MEDIUM #4 (BadSqlGrammarException handler) - LOW #5 (real progress computation) - LOW #6 (Javadoc re-scoped) - Bug #7 (findAllActive typeHandler) - Bug #8 (findByKey typeHandler, IT-discovered) M1/L1/L2/L3/L4 from .claude/reviews/achievement-api-fixes-review.md all addressed. Test coverage: 36 unit + 5 Testcontainers IT.
DavidHLP
pushed a commit
that referenced
this pull request
Jun 13, 2026
… perf + per-channel tests New test files (5): - NotificationDispatcherTest: 8 tests covering ADR-004 §4 #2 (channel failure isolation), #3 (intentId idempotency), #6 (latency < 50ms), category preference suppression, ledger short-circuit, failure_reason truncation, ledger state sanity. - NotificationChannelContractTest: ADR-004 §4 #1 — every intent has at least 1 supporting channel; matrix sanity for InApp/Email/WebSocket. - InAppNotificationChannelTest: channelId, supports, send → row-only path with type=recordSimpleName, metadata includes isAccepted, follow link + title. - EmailNotificationChannelTest: channelId, supports matrix, in-flight rejection, missing-email BusinessException, send routes to EmailService.sendEmail with templateId. - WebSocketNotificationChannelTest: channelId, achievement emits BadgeEarnedPayload (badgeTier slug), submission emits NotificationPayload with isAccepted, contest reminder type tag. EmailNotificationChannel.userMapper: package-private (was private) so unit tests in the same package can inject a mock directly. Production wiring is still via @Autowired(required=false). mvn test on the 5 new test classes + the 4 existing legacy tests: - 5 new classes: 28 tests, 0 failures - 4 existing tests: 30 tests, 0 failures (legacy flag-off path verified) Pre-existing ContestPublicControllerTest failure (ApplicationContext load issue, unrelated to ADR-004) confirmed to fail on main without my changes. Refs: docs/adr/ADR-004-notification-intents.md §4.
DavidHLP
pushed a commit
that referenced
this pull request
Jun 13, 2026
…nedAt) 2 review findings about intentId() collisions on the ledger UNIQUE (intent_id, channel_id) constraint: - SubmissionCompletedIntent: factory of() now throws IllegalStateException when submission.getGeneration() is null, instead of silently falling back to 0L. Two distinct submissions (one real g=0, one with null generation) would have produced identical intentIds, and the second dispatch would have been silently dropped by tryClaim returning 0. The null-generation case indicates a hydration bug, not user data, so fail-fast is correct. Fixes finding #5. - AchievementEarnedIntent: added earnedAt field, intentId now includes the millisecond timestamp (:at<epochMs>). Without this, a re-issued event (tier-up promotion, system re-trigger) for the same achievement collapsed under the existing intentId and was silently dropped on every channel. The natural key for achievement notifications now includes the issue time, matching the AchievementEarnedEvent one-publish-per-earn invariant. Fixes finding #6. Test updates: 5 test files updated to pass the new earnedAt=Instant.now() arg. The sample intents in the channel contract test and the per-channel tests use the new field; the legacy follow-system test path is unchanged (FollowReceivedIntent has no earnedAt). mvn test on the notification/follow/submission/email slice: green. Refs: docs/adr/ADR-004-notification-intents.md (M4d-1 review findings #5, #6).
DavidHLP
added a commit
that referenced
this pull request
Jun 13, 2026
… perf + per-channel tests New test files (5): - NotificationDispatcherTest: 8 tests covering ADR-004 §4 #2 (channel failure isolation), #3 (intentId idempotency), #6 (latency < 50ms), category preference suppression, ledger short-circuit, failure_reason truncation, ledger state sanity. - NotificationChannelContractTest: ADR-004 §4 #1 — every intent has at least 1 supporting channel; matrix sanity for InApp/Email/WebSocket. - InAppNotificationChannelTest: channelId, supports, send → row-only path with type=recordSimpleName, metadata includes isAccepted, follow link + title. - EmailNotificationChannelTest: channelId, supports matrix, in-flight rejection, missing-email BusinessException, send routes to EmailService.sendEmail with templateId. - WebSocketNotificationChannelTest: channelId, achievement emits BadgeEarnedPayload (badgeTier slug), submission emits NotificationPayload with isAccepted, contest reminder type tag. EmailNotificationChannel.userMapper: package-private (was private) so unit tests in the same package can inject a mock directly. Production wiring is still via @Autowired(required=false). mvn test on the 5 new test classes + the 4 existing legacy tests: - 5 new classes: 28 tests, 0 failures - 4 existing tests: 30 tests, 0 failures (legacy flag-off path verified) Pre-existing ContestPublicControllerTest failure (ApplicationContext load issue, unrelated to ADR-004) confirmed to fail on main without my changes. Refs: docs/adr/ADR-004-notification-intents.md §4.
DavidHLP
added a commit
that referenced
this pull request
Jun 13, 2026
…nedAt) 2 review findings about intentId() collisions on the ledger UNIQUE (intent_id, channel_id) constraint: - SubmissionCompletedIntent: factory of() now throws IllegalStateException when submission.getGeneration() is null, instead of silently falling back to 0L. Two distinct submissions (one real g=0, one with null generation) would have produced identical intentIds, and the second dispatch would have been silently dropped by tryClaim returning 0. The null-generation case indicates a hydration bug, not user data, so fail-fast is correct. Fixes finding #5. - AchievementEarnedIntent: added earnedAt field, intentId now includes the millisecond timestamp (:at<epochMs>). Without this, a re-issued event (tier-up promotion, system re-trigger) for the same achievement collapsed under the existing intentId and was silently dropped on every channel. The natural key for achievement notifications now includes the issue time, matching the AchievementEarnedEvent one-publish-per-earn invariant. Fixes finding #6. Test updates: 5 test files updated to pass the new earnedAt=Instant.now() arg. The sample intents in the channel contract test and the per-channel tests use the new field; the legacy follow-system test path is unchanged (FollowReceivedIntent has no earnedAt). mvn test on the notification/follow/submission/email slice: green. Refs: docs/adr/ADR-004-notification-intents.md (M4d-1 review findings #5, #6).
DavidHLP
added a commit
that referenced
this pull request
Jul 3, 2026
管理后台鉴权初始化（CRITICAL #1）： - management/src/main.ts: 新增完整 auth bootstrap 流程 - management/src/stores/auth.ts: 新增 setupSessionExpiredCallback + clearUser WebSocket Token 安全泄露（CRITICAL #2）： - console/src/lib/socket.ts: 移除 URL ?token= 参数，token 改走 connectHeaders - console/src/composables/contest/useContestSocket.ts: 同上 OAuth 重定向 URL 统一（CRITICAL #4）： - management/src/views/auth/components/OAuthButton.vue: 相对路径→绝对路径 Router Guard 错误处理（HIGH #5）： - console/src/router/index.ts: catch 块增加 ApiError instanceof 判断 WebSocket Token 传递优化（HIGH #6）： - useContestSocket.ts: 已正确从 socket.ts 导入 getTokenFromCookie verifyAuth 绕过 axios 拦截器（HIGH #7）： - console/src/utils/auth.ts: fetch() → apiGet() 性能优化（HIGH #9）： - console/src/contexts/AuthContext.ts: setInterval(1000) → watch(isAuthenticated) 代码质量提升（MEDIUM）： - console/src/stores/auth.ts: 移除 LOGOUT_KEY localStorage 追踪 - 新增 hasAuthCookie() 提前检查，173 行净减少 - clearUser() 重置 status="idle" 允许重新初始化 - 移除所有调试 console.log Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
DavidHLP
added a commit
that referenced
this pull request
Jul 3, 2026
…_ADMIN + rankings 404 + is_deleted filter + Unicode title + @Audited + new ErrorCode + controller XSS guard + mutator tests) 修复 docs/contests-api-test-report.md 报告的 7 处缺陷 (1 HIGH + 6 MEDIUM/LOW)： 缺陷 #1 (HIGH) — DELETE 软删除 is_deleted 字段未持久化 - 根因：Contest.isDeleted 字段 @TableLogic + mapper.updateById(entity) 自动忽略逻辑删除字段 - 修复：改用 mapper.update(null, LambdaUpdateWrapper.set(...)) 显式写入 缺陷 #2 — PATCH on FINISHED/RUNNING 比赛无状态守卫 - 修复：service 层加 status guard，仅允许 UPCOMING 修改 - 新增 ErrorCode CONTEST_ONLY_UPDATE_UPCOMING (70006) 区分注册与修改语义 缺陷 #3 — service 层 hasRole("ADMIN") 拒绝 SUPER_ADMIN，与控制器 @PreAuthorize 不一致 - 修复：SecurityUtil 新增 hasAnyRole(String...) 工具方法，ContestServiceImpl 7 处 mutator 替换 缺陷 #4 — CreateContestDTO/UpdateContestDTO title @pattern 拒绝 CJK 字符 - 修复：正则放宽为 [\p{L}\p{N}\s\p{P}]+ 接受 Unicode 字母 - AdminContestController 新增 rejectUnsafeTitleChars helper 显式排除 < 和 > 缺陷 #5 — GET /admin/contest/{id}/rankings 对不存在 contest 返 200+空 - 修复：getAdminContestRanking 入口加 contest 存在性 + is_deleted 校验 缺陷 #6 — addProblem/removeProblem/startContest/endContest 未校验 is_deleted - 修复：4 个 mutator 入口加 .filter(c -> !isDeleted).orElseThrow(COMPETITION_NOT_FOUND) 缺陷 #7 — @Audited 注解缺失导致 audit_logs 无 contest 记录 - 修复：7 个 mutator 方法加 @Audited + AuditContext.setOldValues/setNewValues/setUserId - 复用 AuditActionUtil.CREATE_CONTEST/UPDATE_CONTEST/DELETE_CONTEST 常量 新增测试 ContestServiceImplMutatorTest：5 case 覆盖缺陷 #2/#5/#6 的状态守卫 + 软删除过滤 + 404 路径。 缺陷 #1 的 LambdaUpdateWrapper.getSqlSet() 断言需 @SpringBootTest 上下文，保留为 follow-up， 运行时 curl 验证已确认三列同改 (docs/contests-api-test-report.md §3.5)。 验证： - mvnw compile -B：0 错误 - mvnw test -Dtest=ContestServiceImplMutatorTest：5/5 通过，2.0s - 运行时 curl 5/5 critical 缺陷修复确认 (PM2 重启 ulticode-9001) Artifacts: - Plan: .claude/PRPs/plans/completed/admin-contests-mutator-fixes.plan.md - Report: .claude/PRPs/reports/admin-contests-mutator-fixes-report.md - Review v1: .claude/reviews/admin-contests-mutator-review.md - Review v2: .claude/reviews/admin-contests-mutator-review-v2.md - Test report: docs/contests-api-test-report.md
DavidHLP
added a commit
that referenced
this pull request
Jul 3, 2026
… handler, compute real progress Fixes 6 issues from docs/achievement-api-test-report-2026-06-11.md plus 2 newly-discovered typeHandler bugs (Bug #7, Bug #8) surfaced during validation. CRITICAL #1 — escape `key` column via @TableField(value = "`key`") - Achievement.java:19 — affects MyBatis-Plus auto-generated column lists in selectById / selectPage / selectBatchIds. - Without this, every SQL that lists the achievements columns fails with 'You have an error in your SQL syntax ... near key,...'. CRITICAL #2 — escape `key` in @select SQL - AchievementMapper.findByKey:24 — WHERE clause now backticks. - Fixes admin POST /achievements which called findByKey for dedup. HIGH #3 — T2b /achievements/{invalid} now returns 404 (was 500) - Follows from CRITICAL #1+#2: the BadSqlGrammarException no longer fires before the business-level ACHIEVEMENT_NOT_FOUND can be raised. MEDIUM #4 — BadSqlGrammarException → DATABASE_ERROR (50001) - GlobalExceptionHandler:227 — new handler preserves root cause in server log while returning generic 'Database error' to clients. - Pairs with existing MyBatisSystemException / BindingException / DataIntegrityViolationException handlers (no fallback to 50000 'Unknown error'). LOW #5 — getUserAchievements progress is no longer hard-coded to 0 - AchievementServiceImpl:280 — mirrors getUserProgress logic, reads criteria.type from JSON Map, dispatches to SubmissionMapper counter. LOW #6 — re-scoped Javadoc: getUserAchievements and getUserProgress are NOT duplicates (verified UserController /me/achievements/progress uses the latter). Both serve different endpoints with different DTO shapes. Bug #7 — findAllActive missing JacksonTypeHandler - AchievementMapper:46 — @select bypassed @TableField(typeHandler= JacksonTypeHandler.class), returning criteria=null. - Converted to default method via BaseMapper.selectList. Bug #8 — findByKey same typeHandler bypass - Discovered by AchievementMapperIT (L2) on first run; findByKey_criteriaIsDeserializedAsMap failed. - AchievementMapper:24 — also converted to default method. - Latent: only caller (create) never read criteria. Refactors - M1 (review): getUserAchievements extracted buildProgressDTO helper (52 → 16 lines). - L1 (review): findAllActive uses LambdaQueryWrapper<Achievement> method refs instead of string column names. - L3 (review): Javadoc clarified the two service methods. Tests + AchievementMapperSQLGuardTest: 3 reflection-guard tests (CRITICAL #1, CRITICAL #2, no @select on findAllActive/findByKey). + AchievementMapperIT: 5 Testcontainers MySQL 8.0 IT tests verifying JacksonTypeHandler runtime behavior + Bug #7/#8 regression guards. + AchievementServiceTest: 3 progress tests + 2 new mock fields (SubmissionMapper, ContestParticipantMapper). + GlobalExceptionHandlerTest: 2 BadSqlGrammarException tests. Validation - compile / package: BUILD SUCCESS - Unit tests: 36/36 pass - Testcontainers IT: 5/5 pass - 8 curl integration tests on worktree:9091: all match expected HTTP codes (200/400/404/409) and real progress values (admin 6 accepted problems → progress=6 for all 3 rows). Refs: docs/achievement-api-test-report-2026-06-11.md, .claude/PRPs/plans/completed/achievement-api-fixes.plan.md, .claude/PRPs/reports/achievement-api-fixes-report.md, .claude/reviews/achievement-api-fixes-review.md
DavidHLP
added a commit
that referenced
this pull request
Jul 3, 2026
…dation Merges the fix/achievement-api worktree changes: - CRITICAL #1, #2 (MySQL reserved word `key`) - HIGH #3 (T2b 404) - MEDIUM #4 (BadSqlGrammarException handler) - LOW #5 (real progress computation) - LOW #6 (Javadoc re-scoped) - Bug #7 (findAllActive typeHandler) - Bug #8 (findByKey typeHandler, IT-discovered) M1/L1/L2/L3/L4 from .claude/reviews/achievement-api-fixes-review.md all addressed. Test coverage: 36 unit + 5 Testcontainers IT.
DavidHLP
added a commit
that referenced
this pull request
Jul 3, 2026
… perf + per-channel tests New test files (5): - NotificationDispatcherTest: 8 tests covering ADR-004 §4 #2 (channel failure isolation), #3 (intentId idempotency), #6 (latency < 50ms), category preference suppression, ledger short-circuit, failure_reason truncation, ledger state sanity. - NotificationChannelContractTest: ADR-004 §4 #1 — every intent has at least 1 supporting channel; matrix sanity for InApp/Email/WebSocket. - InAppNotificationChannelTest: channelId, supports, send → row-only path with type=recordSimpleName, metadata includes isAccepted, follow link + title. - EmailNotificationChannelTest: channelId, supports matrix, in-flight rejection, missing-email BusinessException, send routes to EmailService.sendEmail with templateId. - WebSocketNotificationChannelTest: channelId, achievement emits BadgeEarnedPayload (badgeTier slug), submission emits NotificationPayload with isAccepted, contest reminder type tag. EmailNotificationChannel.userMapper: package-private (was private) so unit tests in the same package can inject a mock directly. Production wiring is still via @Autowired(required=false). mvn test on the 5 new test classes + the 4 existing legacy tests: - 5 new classes: 28 tests, 0 failures - 4 existing tests: 30 tests, 0 failures (legacy flag-off path verified) Pre-existing ContestPublicControllerTest failure (ApplicationContext load issue, unrelated to ADR-004) confirmed to fail on main without my changes. Refs: docs/adr/ADR-004-notification-intents.md §4.
DavidHLP
added a commit
that referenced
this pull request
Jul 3, 2026
…nedAt) 2 review findings about intentId() collisions on the ledger UNIQUE (intent_id, channel_id) constraint: - SubmissionCompletedIntent: factory of() now throws IllegalStateException when submission.getGeneration() is null, instead of silently falling back to 0L. Two distinct submissions (one real g=0, one with null generation) would have produced identical intentIds, and the second dispatch would have been silently dropped by tryClaim returning 0. The null-generation case indicates a hydration bug, not user data, so fail-fast is correct. Fixes finding #5. - AchievementEarnedIntent: added earnedAt field, intentId now includes the millisecond timestamp (:at<epochMs>). Without this, a re-issued event (tier-up promotion, system re-trigger) for the same achievement collapsed under the existing intentId and was silently dropped on every channel. The natural key for achievement notifications now includes the issue time, matching the AchievementEarnedEvent one-publish-per-earn invariant. Fixes finding #6. Test updates: 5 test files updated to pass the new earnedAt=Instant.now() arg. The sample intents in the channel contract test and the per-channel tests use the new field; the legacy follow-system test path is unchanged (FollowReceivedIntent has no earnedAt). mvn test on the notification/follow/submission/email slice: green. Refs: docs/adr/ADR-004-notification-intents.md (M4d-1 review findings #5, #6).
DavidHLP
added a commit
that referenced
this pull request
Jul 10, 2026
…te class Architecture review candidate #6 (partial — interface extraction): PermissionService was a 248 LOC concrete @service with no interface, violating the project's Java convention that all Service classes must expose an interface with an Impl suffix (01-java-programming.md §四.16). Extract PermissionService interface with all 6 public methods: - Checking: getUserPermissions, getUserPermissionStrings, hasPermission - Cache: invalidateCache - Assignment: assignPermission - Revocation: revokePermission Create PermissionServiceImpl in service/impl/ with the full implementation. All 7 callers depend on the interface — Spring wires transparently. Verified: mvn compile succeeds, 11 PermissionServiceTest tests pass.
DavidHLP
added a commit
that referenced
this pull request
Jul 10, 2026
Architecture review candidate #6 — management/src/utils/sanitize-markdown.ts renderSafeMarkdown() called renderMarkdown() from shared/markdown-utils (which already sanitizes via DOMPurify) and then ran DOMPurify AGAIN with a locally-defined config. The same anti-pattern existed on the console side via sanitizeHtml(renderMarkdown(...)) in 4 .vue files. Both apps now call renderMarkdown() directly from @/shared/markdown-utils, which owns the sanitization seam. Deletes: - console/src/utils/sanitize.ts (143 LoC, near-identical config) - management/src/utils/sanitize-markdown.ts (141 LoC, double sanitize) management/src/utils/sanitize.ts (sanitizeI18nHtml + sanitizeCodeHtml) is preserved — those are legitimately separate sanitization purposes (i18n inline formatting + hljs token output), not markdown. Verified: console 355 tests pass, management 275 tests pass, both type-check clean. markdown-security.spec.ts now exercises the actual deep module (shared renderMarkdown) rather than the redundant wrapper. Wins (in glossary terms): locality — one sanitization config; leverage — one interface (renderMarkdown), N call sites; removes ~280 LoC of duplicate config that could silently drift.
DavidHLP
added a commit
that referenced
this pull request
Jul 10, 2026
#1 SecurityUtil → CurrentUserProvider port - New CurrentUserProvider interface in common/auth/ - SecurityCurrentUserProvider @component adapter (wraps SecurityContextHolder) - 37 files migrated from static SecurityUtil calls to injected interface - Eliminates duplicated userId==null guards across controller layer #2 DashboardMapper+Service → DashboardStatsProjection - New DashboardStatsProjection interface + DefaultDashboardStatsProjection - Absorbs 7 private sub-aggregators + 6 mapper default methods - DashboardService + DashboardServiceImpl deleted - DashboardController now injects the projection directly - Mirrors ADR-0011 *Projection pattern #3 AdminCommentService → CommentModerator polymorphic seam - New CommentModerator interface with ForumCommentModerator + SolutionCommentModerator - AdminCommentServiceImpl refactored to thin router - Eliminates 5x duplicated forum/solution type-string switch #5 SubmissionMapper.calculateStreak → SubmissionStreakCalculator - New SubmissionStreakCalculator interface + JdbcSubmissionStreakCalculator - Streak algorithm (recursive CTE) now behind a JVM-testable interface - 3 production callers redirected + 3 test classes updated #7 Frontend wrapper cleanup - 8 re-export wrapper files deleted (markdown, useTheme, useTypographyDensity, cn) - tsconfig path remaps @/lib/utils → @/shared/auth-core/src/utils - shared/sidebar-menu cn() consolidated to import from auth-core - LOCALE_HEADER_KEY duplicate declarations removed #8 OAuthService → OAuthClient port + adapters - New OAuthClient interface + OAuthTokenResponse/OAuthUserInfo DTOs - GithubOAuthClient + GoogleOAuthClient @component adapters - OAuthService refactored to thin coordinator (221→195 LoC) - 11 unit tests covering dispatch, callback, DB upsert Production code compiles (BUILD SUCCESS). 53 test compilation errors from constructor signature changes — mechanical @mock field additions. Candidates #4 (analytics projections) and #6 (contest.ts split) pending.
DavidHLP
added a commit
that referenced
this pull request
Jul 10, 2026
…(arch-review #6) The ContestSubscribeAuthInterceptor used to hold both the STOMP translation AND the ContestParticipantMapper lookup. Move the eligibility rules into a transport-agnostic com.ulticode.modules.contest.subscription.ContestSubscriptionPolicy module: - New ContestSubscriptionPolicy interface with evaluate(ContestSubscribeRequest) returning a SubscriptionDecision (allow / deny_* verdict + reason). - DefaultContestSubscriptionPolicy owns the mapper and the participant-row check; the policy tests no longer need a STOMP broker. - ContestSubscribeAuthInterceptor is now a thin STOMP adapter: parse the destination, extract the user from the session, build a ContestSubscribeRequest, call the policy, translate the verdict back into a STOMP ERROR frame. The reject-vs-allow behaviour is preserved verbatim. Future transports (SSE, gRPC server-stream) can build the same ContestSubscribeRequest from their per-protocol data and reuse the policy without dragging in spring-messaging.
DavidHLP
added a commit
that referenced
this pull request
Jul 10, 2026
#6) - useLocale.ts: expand destructured i18n helpers (t/te/tm/rt/n/d) to one per line for readability - i18n/index.ts: split 3-arg import from @/shared/locale-preference/src to one per line (prettier wrap); collapse the local re-export that fits back to a single line
DavidHLP
added a commit
that referenced
this pull request
Jul 16, 2026
Implements candidates from /tmp/architecture-review-1783827842.html: #1 AdminBulkExecutor (admin/bulk): lifts the duplicated per-item try/catch loop + counting + optional existence-guard out of AdminForum/Comment/ Solution/Problem bulk actions into one consumer-owned executor with a canonical ItemOutcome/Run. batchRejudge withdrawn with evidence (rich RejudgeResult shape ≠ void-action aggregated contract). #2 ProblemExportService + ExportPayload: moves format validation, the 10k size cap, CSV header/escaping, and the LocalDate.now() time-leak out of AdminProblemController into a Clock-injected service; controller shrinks to response-writing. #3 LegacyRejudgeStrategy + RejudgePolicy.rejudge dispatcher: moves the 50-line inline legacy rejudge state machine out of AdminSubmissionServiceImpl behind the policy port; fenced vs legacy selection now lives in DefaultRejudgePolicy. AdminSubmissionServiceImpl.rejudge = 3-line dispatch. #4 createAuthStore factory (shared/auth-core): owns the duplicated login/logout/fetchUser/loadPermissions/initialize/clearUser/hasPermission chain + CSRF contract once; management store migrated. Console deferred (already composable-shaped with a richer status state machine). #6 AdminContestReadPort + adapter: ADR-0011 phase 2 (contest) — admin contest reads cross the seam instead of reaching into ContestProblemMapper. #5 (AdminCrudListView wrapper) withdrawn: scaffolded then deleted after code-review flagged it as dead code (0 views wired); the genuine shared seam already lives in DataTable + useDataTable + DataTableToolbar. Validation: mvn compile + test-compile green; 37 targeted admin/submission tests pass (incl. new AdminBulkExecutorTest); management eslint green on auth store. code-review skill run (Standards pass, Spec gaps documented in-code). Co-Authored-By: Claude <noreply@anthropic.com>
DavidHLP
added a commit
that referenced
this pull request
Jul 16, 2026
Extract shared/i18n-completeness owning locale consistency, code-to-locale coverage, dynamic-prefix detection, and report formatting. Console and management check.ts become thin adapters supplying locale trees + source root; the management i18n-coverage and naming-convention specs now hit the production interface instead of reimplementing flatten/scan/extract. Closes candidate #6 of the 2026-07-15 architecture review.
DavidHLP
added a commit
that referenced
this pull request
Jul 20, 2026
Closes the Standards-axis judgement calls the two-axis review raised
(0 hard violations; these are the value-bearing ones):
- C06 data clump: ModerationServiceImpl.applyAction took 8 positional
params (action, moderatorId, note, durationDays, now, queueId,
actionId, item). Introduced a narrow ActionRequest value record for
the per-action inputs so the switch reads (request, item). The record
is a pure value, not the old ActionContext service-callback proxy.
- C01 primitive obsession + edge case: buildScoredContestProblems used a
bare literal 100 for the default score and a bare (char)('A'+i) index
that silently overflowed to non-letters past 26 problems. Named the
default DEFAULT_PROBLEM_SCORE and extracted problemIndex(i) which
returns A-Z for slots 0-25 and a deterministic P<n> label beyond,
instead of garbage chars.
- C02 magic numbers: useProblemRun carried bare "javascript", 60, 5000.
Named DEFAULT_RUN_LANGUAGE, RATE_LIMIT_FALLBACK_WAIT_SECONDS,
RATE_LIMIT_TOAST_DURATION_MS.
Behaviour preserved. #2 (parseRuntimeMs home) and #6 (getSocketManager
name) left: parseRuntimeMs's docstring already distinguishes the sandbox
wire format from the queue parser, and renaming getSocketManager would
churn four callers for a cosmetic gain.
Validated: ModerationServiceImplTest(11) + AdminContestMutationServiceImplTest
green; console type-check + eslint clean; problem-detail suite(12) green.
Co-Authored-By: Claude <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.
No description provided.