Skip to content

Conversation

@k906506
Copy link
Collaborator

@k906506 k906506 commented May 25, 2025

#️⃣ 연관된 이슈

동기부여 관련 신규 기능 개발 #72

📝 작업 내용

  1. 피드백 카드 CRU
  • 피드백 카드 생성 로직 추가
  • 피드백 카드 확인 로직 추가 (피드백 카드의 title과 content는 이 때 정의됨)
  1. 배포 전 확인 사항
  • QA 테스트를 위해 기준일자를 20250526으로 설정 (오픈 전에 수정!)
  • FeedbackTemplateProvider 수정 (피드백 카드 title, content 관련)

@k906506 k906506 requested a review from Jeongminyooa May 25, 2025 11:45
@k906506 k906506 self-assigned this May 25, 2025
@k906506 k906506 added the enhancement New feature or request label May 25, 2025
@github-actions
Copy link

github-actions bot commented May 25, 2025

Unit Test Results

1 tests   1 ✔️  0s ⏱️
1 suites  0 💤
1 files    0

Results for commit e2508b0.

♻️ This comment has been updated with latest results.

Copy link
Collaborator

@Jeongminyooa Jeongminyooa left a comment

Choose a reason for hiding this comment

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

내꺼 코드 부분이랑 싱크 맞춰야 하는 부분들 파악하느라 리뷰가 하루 늦어졌어 ㅠㅠ! 우선 어떤 플로우로 짰는지는 파악은 마쳤고, 내 부분 코드도 싱크 맞추는 작업이 하루 정도 더 소요될 듯해..

리뷰 남긴거 위주로 확인 한번만 부탁할게욥!

Copy link
Collaborator

Choose a reason for hiding this comment

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

해당 파일이랑 FeedbackTemplateProvider 가 entity 패키지 보다는 별도 data_provider 등 패키지로 분류되는게 맞을 것 같아!

// 오늘 기록된 소비 확인
// ver 2.0.0 이후로 기록된 소비만
LocalDateTime createdAt = requestDTO.getRecords().get(0).getDate().atStartOfDay(); // 실제 생성일자말고 사용자가 기록하려는 일자
LocalDateTime baseTime = LocalDateTime.of(2025, 5, 26, 0, 0); // 2025-05-30 00:00
Copy link
Collaborator

Choose a reason for hiding this comment

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

요거는 주석이랑 세팅된 값이랑 다른거 아닌가?!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

응응 맞아!
일단 우리 오픈 일정이 30일이라서 그걸로 적어놓은거고, QA 테스트하려면 기준일자 세팅이 필요해서 일단 26일로 설정해놨어!
이건 QA 테스트 완료되면 수정 에정! (비슷한 로직이 ExpenseService에도 있어 이것도 같이 수정)

List<Feedback> findFeedbackByUserIdAndCreatedAt(Long userId, LocalDateTime date);

@Query("SELECT f FROM Feedback f JOIN f.user u WHERE u.id = :userId ORDER BY f.createdDate DESC")
List<Feedback> findFeedbackByUserIdAndCreatedDate(Long userId, LocalDateTime date);
Copy link
Collaborator

Choose a reason for hiding this comment

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

findFeedbackByUserIdOrderbyCreatedDate -> And로 표현하니까 살짝 헷갈려서 OrderBy 표현해도 좋을 듯!!

if (feedbacks == null || feedbacks.isEmpty()) {
return false;
} else {
for (Feedback feedback : feedbacks) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 로직은 가장 최근 받은 피드백이 open 이면 getNotOpenedFeedbackfalse 라는 로직인건가??

if (feedbacks.get(0).isOpened()) return false;

위랑 다른건가 싶어서!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

오 정확해 이렇게 수정해야겠다

}
}

public Boolean getNotOpenedFeedback(String userKey) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

해당 함수도 boolean 리턴이니까 isNotOpenedFeedback 네이밍 통일해주면 좋을 듯!


Feedback notOpenedFeedback = new Feedback();

for (Feedback feedback : feedbacks) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 부분도 동일하게 feedbacks의 첫번째 엘리먼트의 isOpened()의 t/f 확인해서 분기 태워도 되지 않을까?

// 3. 획득한 피드백의 템플릿 확인
List<FeedbackTemplate> remainTemplates;

List<String> usedTitles = feedbackRepository.findFeedbackByUserIdUsedTitle(user.getId());
Copy link
Collaborator

Choose a reason for hiding this comment

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

지금 로직이면 usedTitles가 소비 기록이 계속 쌓이고 피드백을 계속 받으면 모든 피드백 문구에 대해 used 상태일 것 같은데, 그렇게 되면 결국 밑에 필터링하는 로직 자체가 무의미해질 수도 있을 것 같아.

피드백 문구에 대한 재사용 방지 정책이 월 단위로 초기화가 되는건지도 고려되면 좋을 듯!

Copy link
Collaborator

Choose a reason for hiding this comment

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

결국 5번에서 filter 의미가 사라지고 아래 코드만 쓰이는 셈 되지 않을까 싶은 우려가 있어 남겨봅니다.

// 모두 사용했으면 해당 카테고리에 해당하는 템플릿 전체
			if (notUsedTemplates.isEmpty()) {
				notUsedTemplates = allTemplates;
			}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

맞아!
현재 로직 기준으로는 6개 이상부터는 리뷰 달아준 대로 해당 로직만 타게 됨...
일단 API 배포에 우선 순위를 둬서 이 로직도 최대한 빨리 수정해볼게 (매섭군요!)

Boolean isNotOpened = feedbackService.getNotOpenedFeedback(userKey);
Boolean isFirstOpened = feedbackService.isFirstOpenedFeedback(userKey);

if (!isNotOpened && isFirstOpened) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 부분이 잘 이해가 안가는데 혹시 추가 설명해줄 수 있으려나..?!

Copy link
Collaborator

Choose a reason for hiding this comment

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

피드백이 다 open된 상태이고, 피드백 받은 것이 1개도 없을 때
isFirstOpened가 false가 되는건가..?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

기존에 findFeedbackByUserIdAndCreatedAt호출해서 데이터를 갖고 오다보니
isNotOpen = false, isFirstOpened = true인 케이스가 발생해서 작성해둔 구문이었는데, 이거 지금은 지우는게 맞다...!


// 1. 피드백 조회 성공 -> 200
return ResponseEntity.ok(HttpStatusDTO.response(HttpStatus.OK.value(), "성공", feedbackNotOpenedResponseDTO));
} catch (Exception e) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 catch는 어떤 조건일 때 예외를 잡는거야?? service 단에서 throw 하는 부분이 없는 것 같아서 물어봐유

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

userService의 getUser(userKey)에서 유저 정보가 없는 경우에 대한 Exception!

@k906506 k906506 merged commit ca2aa09 into dev May 27, 2025
2 checks passed
@Jeongminyooa Jeongminyooa deleted the feature-72 branch June 16, 2025 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants