-
Notifications
You must be signed in to change notification settings - Fork 0
feat: 카테고리 색상 기능 추가 & 리마인드 아티클 조회 수정 #92
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
8bfa996
cfd7626
aa7e28f
acf0b4e
e9b339f
020ef7a
f856c7c
5d52389
99b4332
1bc095d
8eb5de1
9606f9c
9a5a26d
3fdba82
2aae766
45d1121
9943984
699bcd7
d78e807
06d8bfd
db74d5e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -15,15 +15,20 @@ public record ArticleUpdateRequest( | |||||||||
| @NotNull(message = "카테고리 ID는 필수입니다") | ||||||||||
| @Positive(message = "카테고리 ID는 양수여야 합니다") | ||||||||||
| Long categoryId, | ||||||||||
|
|
||||||||||
| @Schema(description = "메모", example = "수정된 메모입니다") | ||||||||||
| @Size(max = 500, message = "메모는 500자 이하로 입력해주세요") | ||||||||||
| String memo, | ||||||||||
|
|
||||||||||
| @Schema(description = "리마인더 시간", example = "2025-12-31T23:59:00") | ||||||||||
|
|
||||||||||
| @Schema(description = "현재 시간", example = "2025-12-31T23:59:00") | ||||||||||
| @NotNull(message = "현재시간은 필수입니다") | ||||||||||
| LocalDateTime now, | ||||||||||
|
|
||||||||||
|
Comment on lines
+23
to
+26
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Client-supplied “now” is a trust boundary risk Allowing the client to dictate “now” enables back/forward-dating and inconsistent reminder behavior. Prefer server-generated time (via Clock) in the application layer; if you must accept it, strictly bound acceptable drift. Option A — remove now from the API and compute server-side: - @Schema(description = "현재 시간", example = "2025-12-31T23:59:00")
- @NotNull(message = "현재시간은 필수입니다")
- LocalDateTime now,And adjust toCommand below. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||
| @Schema(description = "리마인드 시간", example = "2025-12-31T23:59:00") | ||||||||||
| LocalDateTime remindTime | ||||||||||
|
|
||||||||||
| ) { | ||||||||||
| public ArticleUpdateCommand toCommand() { | ||||||||||
| return new ArticleUpdateCommand(categoryId, memo, remindTime); | ||||||||||
| return new ArticleUpdateCommand(categoryId, memo, now, remindTime); | ||||||||||
| } | ||||||||||
|
Comment on lines
+32
to
33
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Adjust toCommand if “now” is removed If you adopt Option A, update the command mapping accordingly. - return new ArticleUpdateCommand(categoryId, memo, now, remindTime);
+ return new ArticleUpdateCommand(categoryId, memo, remindTime);📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||
| } | ||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,8 +1,10 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| package com.pinback.api.auth.controller; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.web.bind.annotation.GetMapping; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.web.bind.annotation.PostMapping; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.web.bind.annotation.RequestBody; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.web.bind.annotation.RequestMapping; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.web.bind.annotation.RequestParam; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.web.bind.annotation.RestController; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.pinback.api.auth.dto.request.SignUpRequest; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -33,12 +35,12 @@ public ResponseDto<SignUpResponse> signUp( | |||||||||||||||||||||||||||||||||||||||||||||||||
| return ResponseDto.created(response); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| @Operation(summary = "로그인", description = "이메일로 로그인합니다") | ||||||||||||||||||||||||||||||||||||||||||||||||||
| @PostMapping("/signin") | ||||||||||||||||||||||||||||||||||||||||||||||||||
| @Operation(summary = "토큰 재발급", description = "이메일로 토큰을 발급합니다.") | ||||||||||||||||||||||||||||||||||||||||||||||||||
| @GetMapping("/token") | ||||||||||||||||||||||||||||||||||||||||||||||||||
| public ResponseDto<TokenResponse> signIn( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| @Valid @RequestBody SignUpRequest request | ||||||||||||||||||||||||||||||||||||||||||||||||||
| @Valid @RequestParam String email | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion @Valid on String is a no-op; add concrete constraints. Add @Email and @notblank to enforce format and presence. Keep normalization server-side. Add imports outside this hunk: import jakarta.validation.constraints.Email;
import jakarta.validation.constraints.NotBlank;Diff within this line is included in the main change above. 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| TokenResponse response = authUsecase.getToken(request.email()); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| TokenResponse response = authUsecase.getToken(email); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return ResponseDto.ok(response); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+38
to
45
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blocker: GET + email-only token issuance is insecure (account takeover, user enumeration, cacheable). Issuing tokens with just an email via GET enables:
Action:
Apply within this hunk: - @Operation(summary = "토큰 재발급", description = "이메일로 토큰을 발급합니다.")
- @GetMapping("/token")
- public ResponseDto<TokenResponse> signIn(
- @Valid @RequestParam String email
- ) {
- TokenResponse response = authUsecase.getToken(email);
+ @Operation(summary = "토큰 재발급", description = "이메일로 토큰을 발급합니다.")
+ @PostMapping("/token")
+ public ResponseDto<TokenResponse> issueToken(
+ @RequestParam @NotBlank @Email String email
+ ) {
+ TokenResponse response = authUsecase.getToken(email.trim().toLowerCase());
return ResponseDto.ok(response);
}Also ensure global/security layer sets Cache-Control: no-store and adds rate limiting (e.g., IP+email). If this endpoint replaces a prior signin flow, consider a deprecation window or a compatibility shim. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| package com.pinback.application.article.dto; | ||
|
|
||
| import org.springframework.data.domain.Page; | ||
|
|
||
| import com.pinback.domain.article.entity.Article; | ||
|
|
||
| public record RemindArticlesWithCountDto( | ||
| long readCount, | ||
| long unreadCount, | ||
| Page<Article> articles | ||
| ) { | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ | |
| public record ArticleUpdateCommand( | ||
| Long categoryId, | ||
| String memo, | ||
| LocalDateTime now, | ||
| LocalDateTime remindTime | ||
| ) { | ||
|
Comment on lines
5
to
10
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Avoid client-supplied “now”; compute on the server (inject Clock). Passing I can propose a minimal refactor in the use case to switch to |
||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,22 +2,25 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.time.LocalDateTime; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.pinback.application.category.dto.response.CategoryResponse; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.pinback.domain.article.entity.Article; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| public record ArticleResponse( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| long articleId, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| String url, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| String memo, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| LocalDateTime createdAt, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| boolean isRead | ||||||||||||||||||||||||||||||||||||||||||||||||||
| boolean isRead, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| CategoryResponse category | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| public static ArticleResponse from(Article article) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return new ArticleResponse( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| article.getId(), | ||||||||||||||||||||||||||||||||||||||||||||||||||
| article.getUrl(), | ||||||||||||||||||||||||||||||||||||||||||||||||||
| article.getMemo(), | ||||||||||||||||||||||||||||||||||||||||||||||||||
| article.getCreatedAt(), | ||||||||||||||||||||||||||||||||||||||||||||||||||
| article.isRead() | ||||||||||||||||||||||||||||||||||||||||||||||||||
| article.isRead(), | ||||||||||||||||||||||||||||||||||||||||||||||||||
| CategoryResponse.from(article.getCategory()) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+14
to
24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Null-safe category mapping to avoid NPEs
Apply: - article.isRead(),
- CategoryResponse.from(article.getCategory())
+ article.isRead(),
+ (article.getCategory() != null ? CategoryResponse.from(article.getCategory()) : null)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| package com.pinback.application.article.dto.response; | ||
|
|
||
| import java.time.LocalDateTime; | ||
|
|
||
| import com.pinback.application.category.dto.response.CategoryResponse; | ||
| import com.pinback.domain.article.entity.Article; | ||
|
|
||
| public record RemindArticleResponse( | ||
| long articleId, | ||
| String url, | ||
| String memo, | ||
| LocalDateTime createdAt, | ||
| boolean isRead, | ||
| LocalDateTime remindAt, | ||
| CategoryResponse category | ||
| ) { | ||
| public static RemindArticleResponse from(Article article) { | ||
| return new RemindArticleResponse( | ||
| article.getId(), | ||
| article.getUrl(), | ||
| article.getMemo(), | ||
| article.getCreatedAt(), | ||
| article.isRead(), | ||
| article.getRemindAt(), | ||
| CategoryResponse.from(article.getCategory()) | ||
| ); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| package com.pinback.application.article.dto.response; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| public record TodayRemindResponse( | ||
| long readArticleCount, | ||
| long unreadArticleCount, | ||
| List<RemindArticleResponse> articles | ||
| ) { | ||
| public static TodayRemindResponse of(long readArticleCount, long unreadArticleCount, List<RemindArticleResponse> articles) { | ||
| return new TodayRemindResponse(readArticleCount, unreadArticleCount, articles); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -44,7 +44,7 @@ public void updateArticle(User user, long articleId, ArticleUpdateCommand comman | |||||
| Category category = getCategoryPort.getCategoryAndUser(command.categoryId(), user); | ||||||
| article.update(command.memo(), category, command.remindTime()); | ||||||
|
|
||||||
| handleReminderUpdate(article, user, command.remindTime(), remindTimeChanged, articleId); | ||||||
| handleReminderUpdate(article, user, command.now(), command.remindTime(), remindTimeChanged, articleId); | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Time spoofing risk: using command.now() to gate scheduling. A malicious or skewed client can set Apply this diff to stop threading client time into scheduling: - handleReminderUpdate(article, user, command.now(), command.remindTime(), remindTimeChanged, articleId);
+ handleReminderUpdate(article, user, command.remindTime(), remindTimeChanged, articleId);And adjust the method signature per the next comment. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| } | ||||||
|
|
||||||
| private void validateMemoLength(String memo) { | ||||||
|
|
@@ -53,12 +53,12 @@ private void validateMemoLength(String memo) { | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| private void handleReminderUpdate(Article article, User user, LocalDateTime remindTime, | ||||||
| private void handleReminderUpdate(Article article, User user, LocalDateTime now, LocalDateTime remindTime, | ||||||
| boolean remindTimeChanged, long articleId) { | ||||||
|
Comment on lines
+56
to
57
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Derive “now” on server (Clock) inside handler; keep tests deterministic. Inject a -private void handleReminderUpdate(Article article, User user, LocalDateTime now, LocalDateTime remindTime,
- boolean remindTimeChanged, long articleId) {
+private void handleReminderUpdate(Article article, User user, LocalDateTime remindTime,
+ boolean remindTimeChanged, long articleId) {
if (remindTimeChanged) {
manageArticleReminderPort.cancelArticleReminder(articleId, user.getId());
- if (remindTime != null && !remindTime.isBefore(now)) {
+ var now = LocalDateTime.now(clock);
+ if (remindTime != null && !remindTime.isBefore(now)) {
PushSubscription subscriptionInfo = getPushSubscription.findPushSubscription(user);
manageArticleReminderPort.scheduleArticleReminder(article, user, subscriptionInfo.getToken());
}
}
}Additional changes (outside selected lines): // imports
import java.time.Clock;
// field
private final Clock clock;I can push a patch that adds Also applies to: 61-64 🤖 Prompt for AI Agents |
||||||
| if (remindTimeChanged) { | ||||||
| manageArticleReminderPort.cancelArticleReminder(articleId, user.getId()); | ||||||
|
|
||||||
| if (remindTime != null && !remindTime.isBefore(LocalDateTime.now())) { | ||||||
| if (remindTime != null && !remindTime.isBefore(now)) { | ||||||
| PushSubscription subscriptionInfo = getPushSubscription.findPushSubscription(user); | ||||||
| manageArticleReminderPort.scheduleArticleReminder(article, user, subscriptionInfo.getToken()); | ||||||
| } | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,10 +1,13 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| package com.pinback.application.category.dto.response; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.pinback.domain.category.enums.CategoryColor; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public record CreateCategoryResponse( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Long id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String name | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Long categoryId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String categoryName, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String categoryColor | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public static CreateCategoryResponse of(Long id, String name) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new CreateCategoryResponse(id, name); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public static CreateCategoryResponse of(Long categoryId, String categoryName, CategoryColor categoryColor) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new CreateCategoryResponse(categoryId, categoryName, categoryColor.toString()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+3
to
12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Use
Apply: package com.pinback.application.category.dto.response;
-import com.pinback.domain.category.enums.CategoryColor;
+import com.pinback.domain.category.enums.CategoryColor;
+import java.util.Objects;
public record CreateCategoryResponse(
Long categoryId,
String categoryName,
String categoryColor
) {
public static CreateCategoryResponse of(Long categoryId, String categoryName, CategoryColor categoryColor) {
- return new CreateCategoryResponse(categoryId, categoryName, categoryColor.toString());
+ Objects.requireNonNull(categoryColor, "categoryColor");
+ return new CreateCategoryResponse(categoryId, categoryName, categoryColor.name());
}
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| package com.pinback.application.category.port.out; | ||
|
|
||
| import java.util.Set; | ||
|
|
||
| import com.pinback.domain.category.enums.CategoryColor; | ||
| import com.pinback.domain.user.entity.User; | ||
|
|
||
| public interface CategoryColorServicePort { | ||
|
|
||
| Set<CategoryColor> getUsedColorsByUser(User user); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Guard page/size bounds (min/max) at the edge.
If PageQuery doesn’t validate, add @Min/@max to prevent heavy queries and negative indexes.
🏁 Script executed:
Length of output: 395
🏁 Script executed:
Length of output: 728
Constrain page/size inputs with bean validation. Add
@PositiveOrZerotopageNumberand@Min(1)/@Max(100)topageSizeon thePageQueryrecord, and enable validation in the controller (e.g.@Validatedon the class and@Validon thePageQueryparameter).🤖 Prompt for AI Agents