Repository navigation
refactor code structure and css reuse - #25
Conversation
There was a problem hiding this comment.
Code Review
This pull request optimizes code structure and CSS reuse by extracting scoped styles from Vue SFCs into adjacent CSS files, introducing reusable UI utility classes, and refactoring AdminServiceImpl to delegate dashboard and CSV export generation to dedicated builder classes (AdminDashboardBuilder and AdminCsvExportBuilder). The review feedback highlights critical runtime issues in the new builders, specifically potential NullPointerException failures when using List.of(...) with nullable database fields in the CSV export methods, and recommends enhancing asLocalDateTime to support Timestamp and Date types to ensure robust date parsing across different database drivers.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| rows.stream().map(this::sanitizeAdminUser).map(row -> List.of( | ||
| row.get("id"), | ||
| row.get("username"), | ||
| row.get("nickname"), | ||
| row.get("status"), | ||
| row.get("role"), | ||
| row.get("verificationStatus"), | ||
| row.get("privilegeLabel"), | ||
| row.get("creditLevel"), | ||
| row.get("isRestricted"), | ||
| row.get("registeredAt"), | ||
| row.get("lastLoginAt") | ||
| )).toList() |
There was a problem hiding this comment.
Using List.of(...) here will throw a NullPointerException at runtime if any of the mapped database values (such as nickname, verificationStatus, privilegeLabel, creditLevel, or lastLoginAt) are null. Since database rows often contain nullable columns, it is safer to use java.util.Arrays.asList(...) which permits null elements. The downstream CSV builder already handles null values gracefully.
| rows.stream().map(this::sanitizeAdminUser).map(row -> List.of( | |
| row.get("id"), | |
| row.get("username"), | |
| row.get("nickname"), | |
| row.get("status"), | |
| row.get("role"), | |
| row.get("verificationStatus"), | |
| row.get("privilegeLabel"), | |
| row.get("creditLevel"), | |
| row.get("isRestricted"), | |
| row.get("registeredAt"), | |
| row.get("lastLoginAt") | |
| )).toList() | |
| rows.stream().map(this::sanitizeAdminUser).map(row -> java.util.Arrays.asList( | |
| row.get("id"), | |
| row.get("username"), | |
| row.get("nickname"), | |
| row.get("status"), | |
| row.get("role"), | |
| row.get("verificationStatus"), | |
| row.get("privilegeLabel"), | |
| row.get("creditLevel"), | |
| row.get("isRestricted"), | |
| row.get("registeredAt"), | |
| row.get("lastLoginAt") | |
| )).toList() |
| rows.stream().map(row -> List.of( | ||
| row.get("id"), | ||
| row.get("title"), | ||
| row.get("categoryName"), | ||
| row.get("sellerUserId"), | ||
| row.get("sellerName"), | ||
| row.get("shopId"), | ||
| row.get("shopName"), | ||
| row.get("productType"), | ||
| row.get("status"), | ||
| row.get("reviewStatus"), | ||
| row.get("price"), | ||
| row.get("stock"), | ||
| row.get("viewCount"), | ||
| row.get("favoriteCount"), | ||
| row.get("createdAt"), | ||
| row.get("updatedAt") | ||
| )).toList() |
There was a problem hiding this comment.
Using List.of(...) here will throw a NullPointerException at runtime if any of the mapped database values (such as categoryName, sellerName, shopId, shopName, reviewStatus, favoriteCount, or updatedAt) are null. Using java.util.Arrays.asList(...) is safer as it permits null elements, which are already handled gracefully by the downstream CSV builder.
| rows.stream().map(row -> List.of( | |
| row.get("id"), | |
| row.get("title"), | |
| row.get("categoryName"), | |
| row.get("sellerUserId"), | |
| row.get("sellerName"), | |
| row.get("shopId"), | |
| row.get("shopName"), | |
| row.get("productType"), | |
| row.get("status"), | |
| row.get("reviewStatus"), | |
| row.get("price"), | |
| row.get("stock"), | |
| row.get("viewCount"), | |
| row.get("favoriteCount"), | |
| row.get("createdAt"), | |
| row.get("updatedAt") | |
| )).toList() | |
| rows.stream().map(row -> java.util.Arrays.asList( | |
| row.get("id"), | |
| row.get("title"), | |
| row.get("categoryName"), | |
| row.get("sellerUserId"), | |
| row.get("sellerName"), | |
| row.get("shopId"), | |
| row.get("shopName"), | |
| row.get("productType"), | |
| row.get("status"), | |
| row.get("reviewStatus"), | |
| row.get("price"), | |
| row.get("stock"), | |
| row.get("viewCount"), | |
| row.get("favoriteCount"), | |
| row.get("createdAt"), | |
| row.get("updatedAt") | |
| )).toList() |
| private LocalDateTime asLocalDateTime(Object value) { | ||
| if (value instanceof LocalDateTime localDateTime) { | ||
| return localDateTime; | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
The current implementation of asLocalDateTime only handles instances of LocalDateTime. Depending on the database driver and configuration, date/time fields retrieved from the database (such as submittedAt) might be returned as java.sql.Timestamp or java.util.Date. If so, this method will return null, causing date-based calculations on the dashboard to silently fail or return zero values. Supporting these types explicitly makes the method much more robust.
private LocalDateTime asLocalDateTime(Object value) {
if (value instanceof LocalDateTime localDateTime) {
return localDateTime;
}
if (value instanceof java.sql.Timestamp timestamp) {
return timestamp.toLocalDateTime();
}
if (value instanceof java.util.Date date) {
return date.toInstant().atZone(java.time.ZoneId.systemDefault()).toLocalDateTime();
}
return null;
}
What changed
Type
Testing
cd backend && mvnw.cmd test— all passcd frontend && npm test— all passcd frontend && npm run build— no errorsChecklist
CHANGELOG.mdprepended (skip for docs-only)docs/09-api-spec/).httpsmoke file updated if endpoint added or changed (docs/06-http/)docs/08-tasks/)Notes