-
Notifications
You must be signed in to change notification settings - Fork 1
[REVIEW] V2 대비 리팩토링 리뷰 #228
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
base: review1
Are you sure you want to change the base?
Conversation
…o refactor/#77-기능-수정
인증 과정 수정
[Refactor/#77] Notification 기능 수정
[DEPLOY] v1.4 notification refactor
…into feat/#24-map-api
- 채팅 api 수정
…into refactor/#38-chatting-change-securityutil-to-principaldetails # Conflicts: # src/main/resources/application.yml # src/test/java/com/assu/server/ServerApplicationTests.java
- Major 와 Department 매핑 - signupSsuStudent 메소드에 적용
- 소프트 삭제 방식 (deletedAt 필드) - 탈퇴 회원 재로그인 시 자동 복구 - 회원탈퇴 API 엔드포인트 추가 - 토큰 무효화 및 보안 처리
- 소프트 삭제 방식 (deletedAt 필드) - 탈퇴 회원 재로그인 시 자동 복구 - 회원탈퇴 API 엔드포인트 추가 - 토큰 무효화 및 보안 처리
[DEPLOY] v1.5
[FIX] 알리고 수정 및 로그인 반환 수정
[DEPLOY] v1.5 알리고 fix & 로그인 반환 에러 수정
[Refactor/#85] 웹소켓 인증과 잡다한 로직 수정
…into refactor/#38-chatting-change-securityutil-to-principaldetails # Conflicts: # src/main/java/com/assu/server/global/apiPayload/code/status/ErrorStatus.java
Refactor/#98 dashboard
…urityutil-to-principaldetails [Refactor] 채팅 api 수정
[DEPLOY] v1.6 Chatting 수정
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.
사용 안한다면 삭제요청
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.
어노테이션 엔터
src/main/java/com/assu/server/domain/partnership/controller/PaperController.java
Show resolved
Hide resolved
| return null; | ||
| } | ||
|
|
||
| private static String buildPaperContentText(PaperContent content, List<String> goodsList, Integer peopleValue) { |
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.
제휴 바뀐다면 여기도 수정 필요
src/main/java/com/assu/server/domain/partnership/repository/PartnershipRepository.java
Show resolved
Hide resolved
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.
사용하는건가요?
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.
사용안하면 삭제 필요
src/main/java/com/assu/server/domain/partnership/service/PaperQueryServiceImpl.java
Show resolved
Hide resolved
| private final PartnerRepository partnerRepository; | ||
| private final StoreRepository storeRepository; | ||
|
|
||
| private final AmazonS3Manager amazonS3Manager; |
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.
변수 선언은 상단에 한번에 하는게 보기 좋을듯
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.
서희랑 나눠서 작업하느라 갈렸나봐요 수정하겠습니다
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.
컨버터 사용하기
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.
명세에 맞게 수정
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.
주석삭제
|
|
||
| import java.util.List; | ||
|
|
||
| public class ReviewRequestDTO { |
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.
공백 왜이럼ㅋㅋ
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.
명세에 맞게 수정
| @RequiredArgsConstructor | ||
| @Builder | ||
| @Getter | ||
| public static class todayBest{ |
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.
대문자
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.
여백체크
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.
아래 싹 안쓰는거면 삭제 필요
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.
사용하나요?
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.
에러처리할 때 기본적으로 http 메서드 자체는 200을 리턴하고, 내부 파라미터 code를 무슨 에러인지 (401)리턴하는 방식도 고려해보면 좋을 것 같음
| import com.assu.server.domain.user.entity.enums.Major; | ||
| import com.assu.server.domain.user.entity.enums.University; | ||
|
|
||
| // PaperQueryServiceImpl 이 AdminService 참조 중 -> 순환참조 문제 발생하지 않도록 주의 |
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.
주석 지우기
src/main/java/com/assu/server/domain/auth/dto/signup/AdminSignUpRequest.java
Show resolved
Hide resolved
src/main/java/com/assu/server/domain/auth/dto/signup/PartnerSignUpRequest.java
Show resolved
Hide resolved
| import jakarta.persistence.MapsId; | ||
| import jakarta.persistence.OneToOne; | ||
| import jakarta.persistence.Id; | ||
| import jakarta.persistence.*; |
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.
안쓰는 모듈 지우기 혹은 통일하기
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.
alt + ctrl + O (window 기준)
cmd + ctrl + O (맥북기준은 찾아보기)
| import java.util.Optional; | ||
|
|
||
| import com.assu.server.domain.admin.entity.Admin; | ||
| import com.assu.server.domain.common.enums.ActivationStatus; |
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.
안쓰는 모듈 지우기
src/main/java/com/assu/server/domain/user/repository/PartnershipUsageRepository.java
Show resolved
Hide resolved
src/main/java/com/assu/server/domain/user/repository/StudentRepository.java
Show resolved
Hide resolved
src/main/java/com/assu/server/domain/user/repository/StudentRepository.java
Show resolved
Hide resolved
src/main/java/com/assu/server/domain/user/service/StudentServiceImpl.java
Show resolved
Hide resolved
src/main/java/com/assu/server/domain/user/service/StudentServiceImpl.java
Show resolved
Hide resolved
|
|
||
| @Enumerated(EnumType.STRING) | ||
| @NotNull | ||
| private University university; |
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.
@NotNull의 표시를 통해 필드값 주입 명확히 할 수 있도록 할 것
| private final AdminRepository adminRepository; | ||
| private final PartnerRepository partnerRepository; | ||
| @Override | ||
| @Transactional |
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.
이와 같은 부분 서비스단에 @transactional 달아준 후에 해당하는 부분만 readOnly로 명시할 것
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.
필드 개수와 상관없이 바꿀거면 전부 다 바꾸는게 좋아보입니다
|
|
||
| private LocalDateTime phoneVerifiedAt; | ||
|
|
||
| private String profileUrl; |
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.
NotNull의 표시를 통해 필드값 주입 명확히 할 수 있도록 할 것
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.
완성된 제휴 문장 백엔드에서 완성해서 String으로 프론트 보내기
| List<StoreUsageWithPaper> findUsageByStoreIncludingZero(@Param("adminId") Long adminId); | ||
|
|
||
| interface StoreUsageWithPaper { | ||
| Long getPaperId(); // 🆕 추가: Paper ID |
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.
이모지
src/main/java/com/assu/server/domain/notification/dto/NotificationRequestDTO.java
Show resolved
Hide resolved
src/main/java/com/assu/server/domain/partnership/dto/PaperContentRequestDTO.java
Show resolved
Hide resolved
| @OneToMany(mappedBy = "review", cascade = CascadeType.ALL, fetch = FetchType.LAZY) | ||
| private List<ReviewPhoto> imageList = new ArrayList<>(); | ||
|
|
||
| public List<ReviewPhoto> getImageList() { |
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.
위치 마음에 안듧
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.
오잉?
BAEK0111
left a comment
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.
👍
leesumin0526
left a comment
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.
이수민 리팩 진행 중
No description provided.