Skip to content
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

[♻️ refactor/#96] 스크랩 관련 API -> path variable 수정 및 color enum 필드 수정 #125

Merged
merged 2 commits into from
Sep 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -29,20 +29,20 @@ public ResponseEntity<SuccessResponse> createScrap(
return ResponseEntity.ok(SuccessResponse.of(SUCCESS_CREATE_SCRAP));
}

@DeleteMapping("/scraps/{scrapId}")
@DeleteMapping("/scraps/{internshipAnnouncementId}")
public ResponseEntity<SuccessResponse> deleteScrap(
@AuthenticationPrincipal Long userId,
@PathVariable Long scrapId) {
scrapService.deleteScrap(scrapId, userId);
@PathVariable Long internshipAnnouncementId) {
scrapService.deleteScrap(internshipAnnouncementId, userId);
return ResponseEntity.ok(SuccessResponse.of(SUCCESS_DELETE_SCRAP));
}

@PatchMapping("/scraps/{scrapId}")
@PatchMapping("/scraps/{internshipAnnouncementId}")
public ResponseEntity<SuccessResponse> updateScrapColor(
@AuthenticationPrincipal Long userId,
@PathVariable Long scrapId,
@PathVariable Long internshipAnnouncementId,
@RequestBody UpdateScrapRequestDto request) {
scrapService.updateScrapColor(scrapId, request, userId);
scrapService.updateScrapColor(internshipAnnouncementId, request, userId);
return ResponseEntity.ok(SuccessResponse.of(SUCCESS_UPDATE_SCRAP));
}
}
22 changes: 11 additions & 11 deletions src/main/java/org/terning/terningserver/domain/enums/Color.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,18 @@
@Getter
public enum Color {

RED(0, "ED4E54"),
ORANGE1(1, "EE7647"),
ORANGE2(2, "F3A649"),
YELLOW(3, "F5E660"),
GREEN1(4, "C4E953"),
GREEN2(5, "84D558"),
BLUE1(6, "45D0CC"),
BLUE2(7, "4AA9F2"),
PURPLE(8, "9B64E2"),
PINK(9, "F260AC");
RED("red", "ED4E54"),
ORANGE("orange", "F3A649"),
LIGHT_GREEN("lightgreen", "C4E953"),
MINT("mint", "45D0CC"),
PURPLE("purple", "9B64E2"),
CORAL("coral", "EE7647"),
YELLOW("yellow", "F5E660"),
GREEN("green", "84D558"),
BLUE("blue", "4AA9F2"),
PINK("pink", "F260AC");

private final int key;
private final String name;
private final String value;

public String getColorValue() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package org.terning.terningserver.dto.scrap.request;

public record CreateScrapRequestDto(
int color
String color
) {

}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package org.terning.terningserver.dto.scrap.request;

public record UpdateScrapRequestDto(
int color
String color
) {
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public enum ErrorMessage {
NOT_FOUND_INTERN_CATEGORY(404, "해당 인턴 공고는 존재하지 않습니다"),
NOT_FOUND_INTERN_EXCEPTION(404, "해당 인턴 공고는 존재하지 않습니다"),
NOT_FOUND_USER_EXCEPTION(404, "해당 유저가 존재하지 않습니다"),
NOT_FOUND_SCRAP(404, "해당 스크랩 id에 대한 스크랩 정보가 존재하지 않습니다"),
NOT_FOUND_SCRAP(404, "스크랩 정보가 존재하지 않습니다"),
FORBIDDEN_DELETE_SCRAP(403, "해당 유저가 스크랩하지 않았으므로 스크랩 취소가 불가합니다");

private final int status;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ public interface ScrapRepository extends JpaRepository<Scrap, Long>, ScrapReposi

// List<Scrap> findByUserIdAndInternshipAnnouncement_Deadline(Long userId, LocalDate deadline);

void deleteByInternshipAnnouncementIdAndUserId(Long internshipId, Long userId);

List<Scrap> findByUserIdAndInternshipAnnouncement_DeadlineBetween(Long userId, LocalDate start, LocalDate end);

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import org.terning.terningserver.dto.calendar.response.MonthlyListResponseDto;
import org.terning.terningserver.dto.user.response.UpcomingScrapResponseDto;
import org.terning.terningserver.exception.CustomException;
import org.terning.terningserver.exception.enums.ErrorMessage;
import org.terning.terningserver.repository.internship_announcement.InternshipRepository;
import org.terning.terningserver.repository.scrap.ScrapRepository;
import org.terning.terningserver.repository.user.UserRepository;
Expand Down Expand Up @@ -135,17 +136,17 @@ public void createScrap(Long internshipAnnouncementId, CreateScrapRequestDto req

@Override
@Transactional
public void deleteScrap(Long scrapId, Long userId) {
Scrap scrap = findScrap(scrapId);
public void deleteScrap(Long internshipAnnouncementId, Long userId) {
Scrap scrap = findScrap(internshipAnnouncementId, userId);
scrap.getInternshipAnnouncement().updateScrapCount(-1);
verifyScrapOwner(scrap, userId);
scrapRepository.deleteById(scrapId);
scrapRepository.deleteByInternshipAnnouncementIdAndUserId(internshipAnnouncementId, userId);
}

@Override
@Transactional
public void updateScrapColor(Long scrapId, UpdateScrapRequestDto request, Long userId) {
Scrap scrap = findScrap(scrapId);
public void updateScrapColor(Long internshipAnnouncementId, UpdateScrapRequestDto request, Long userId) {
Scrap scrap = findScrap(internshipAnnouncementId, userId);
verifyScrapOwner(scrap, userId);
scrap.updateColor(findColor(request.color()));
}
Expand All @@ -157,9 +158,9 @@ private void verifyScrapOwner(Scrap scrap, Long userId) {
}
}

private Color findColor(int color) {
private Color findColor(String color) {
return Arrays.stream(Color.values())
.filter(c-> c.getKey() == color)
.filter(c-> c.getName().equals(color))
.findAny().get();
}

Comment on lines +161 to 166
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. .findAny().get() 을 사용하는 경우, Invalid한 color값이 요청으로 들어오면 오류가 발생할 수 있어 이부분에 대한 예외처리가 있으면 좋을 것 같습니다..!
  2. 색상 변경은 빈번하게 일어나는 기능이라고 생각이 드는데, Color.values()를 매번 순회하는 대신, Map을 사용해 캐싱을 하는 방식도 생각해보면 좋을 것 같아요!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 동의합니다!

Copy link
Member Author

@JungYoonShin JungYoonShin Sep 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • 1번에 대해서는 request dto에서 유효성 검증을 하면 좋을 것 같다는 생각인데요!
    저희가 아직 validation exceptionGlobalExceptionHandler에서 전역적으로 처리하지 않고 있어서 추후에 해당 처리가 된 후에 바로 적용하겠습니다!

    👉더 좋은 의견있다면 달아주세요 ㅎㅎ

  • 2번은 저도 생각못했는데 한 번 찾아보고 적용해볼게요 감사합니다!

Expand All @@ -173,8 +174,8 @@ private User findUser(Long userId) {
.orElseThrow(() -> new CustomException(NOT_FOUND_USER_EXCEPTION));
}

private Scrap findScrap(Long scrapId) {
return scrapRepository.findById(scrapId)
private Scrap findScrap(Long internshipAnnouncementId, Long userId) {
return scrapRepository.findByInternshipAnnouncementIdAndUserId(internshipAnnouncementId, userId)
.orElseThrow(() -> new CustomException(NOT_FOUND_SCRAP));
}
}
Expand Down
Loading