Skip to content

Conversation

@eeeeeaaan
Copy link
Contributor

@eeeeeaaan eeeeeaaan commented Dec 28, 2025

#️⃣연관된 이슈

#229

📝작업 내용

공통 리팩토링 사항을 적용
인증 과정을 redis session으로 전환

🔎코드 설명(스크린샷(선택))

비고 (Optional)

참고했던 링크 등 참고 사항을 적어주세요. 코드 리뷰하는 사람이 참고해야 하는 내용을 자유로운 형식으로 적을 수 있습니다.

@eeeeeaaan eeeeeaaan self-assigned this Dec 28, 2025
@eeeeeaaan eeeeeaaan added the ♻️ refactor Refactor code label Dec 28, 2025
@eeeeeaaan eeeeeaaan changed the title [refactor/#229] 1차 리팩 [refactor/#229] 1차 리팩(1/4회의 수정사항 반영 완료) Jan 6, 2026
Copy link
Member

@2ghrms 2ghrms left a comment

Choose a reason for hiding this comment

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

고생하셨습니다~~ PR 제목의 키워드만 대문자로 수정하면 좋을 것 같아요~

@Transactional
public StudentResponseDTO.myPartnership getMyPartnership(Long studentId, int year, int month) {
List<PartnershipUsage> usages = partnershipUsageRepository.findByYearAndMonth(studentId, year, month);
@Transactional(readOnly=true)
Copy link
Member

Choose a reason for hiding this comment

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

@transactional 관련해서 가능하면 클래스에 달아주고 필요시 메소드별로 readOnly를 오버라이딩 해주는게 좋을 것 같습니다

}


public static PaperContentResponseDTO toContentResponse(PaperContent content) {
Copy link
Member

Choose a reason for hiding this comment

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

converter를 더이상 사용하지 않기로 했으니 convert 관련 로직은 해당 DTO 내부로 옮기면 될 것 같습니다~

// addUserToSession에서 자동으로 키가 생성되므로 이 메서드는 비워두거나,
// 만료 시간 설정 등 초기화 로직을 넣을 수 있습니다.
// 예: 10분 후 만료
private String getInfoKey(Long sessionId) {
Copy link
Member

Choose a reason for hiding this comment

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

private 메소드는 맨 뒤로 이동 시키는 게 가독성 및 추상화, Step-down Rule에 의해 관례적이니 클래스의 아래쪽으로 이동시키면 좋을 것 같습니다~

// 세션 생성 직후 만료 시간을 5분으로 설정
timeoutManager.scheduleTimeout(sessionId, Duration.ofMinutes(10));// TODO: 나중에 5분으로 변경

// 세션 여는 대표자는 제일 먼저 인증
Copy link
Member

Choose a reason for hiding this comment

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

잘은 모르지만 여기 로직이 삭제된 이유 알 수 있을까요?

} else {
messagingTemplate.convertAndSend("/certification/progress/" + sessionId,
new CertificationProgressResponseDTO("progress", currentCertifiedNumber, null, sessionManager.snapshotUserIds(sessionId)));
new CertificationProgressResponseDTO("progress", currentCount, null, currentCertifiedUserIds));
Copy link
Member

Choose a reason for hiding this comment

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

이런식으로 null이 들어간 건 필드를 지우는게 좋을 것 같습니다~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

♻️ refactor Refactor code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants