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

[Team#03][BE] 4주차 PR입니다 #75

Merged
merged 11 commits into from
Aug 20, 2023

Conversation

CDBchan
Copy link

@CDBchan CDBchan commented Aug 18, 2023

👋 To Reviewer

안녕하세요, 새리~
마지막 주차도 잘부탁 드립니다.

요번주 구현한 기능 및 구현하면서 고민했던 부분을 PR에 작성했습니다.
배포한 서버 주소도 전달드립니다!

  • 서버 주소: http://54.180.146.130/
    (아직 몇몇 기능만 됩니다. 백엔드 기능은 있는데 프론트엔드 기능이 미완료인 부분도 있습니다.)
    로컬 회원가입 폼이 아직 변경이 안되어서 깃허브로 로그인 하셔야 합니다.

이번주는 따로 고민 했던 부분이 없습니다. Mybatis, redis, oauth등을 사용하여 구현하는게 처음이라 천천히 공부하면서
차근차근 구현했던것 같습니다.
지금까지 저희 프로젝트 리뷰 해주셔서 감사합니다 새리!

🔑 Key changes

  • 로그인 기능 구현
  • 이슈 필터링후 조회 기능 구현
  • 파일 전송 기능 구현
  • 메인 페이지 동적 및 정적 필터 기능 구현
  • 예외 처리 구현
  • redis 를 사용하여 로그아웃 기능 구현

남은 기능

  • 테스트 코드
  • 오가니제이션 CRUD

오가니제이션 CRUD 를 구현하기 위해서 FE 쪽에서 추가 화면이 필요한데, 저희 FE가 한분이라 기존에 저희가 작업한 부분을 다보여 주는데도
무리가 있어 오가니제이션 CRUD는 구현을 하지 못했습니다.
테스트 코드는 추후 천천히 짜보겠습니다!

CDBchan and others added 11 commits August 14, 2023 13:40
response에 memberId, nickname, imgUrl을 포함하도록 수정했다.
* feat: 이슈 필터링 조회 기능 구현

* refactor: Jinny 리뷰 반영
레이블, 마일스톤, 담당자가 없는 이슈 필터링 기능 구현
* chore: 파일 업로드를 위한 환경 설정

* feat: 파일 업로드 기능 구현

* refactor: 코멘트에서 fileUrl 관련 내용 제외
* feat: 메인페이지 동적 필터 조회 기능 구현

* chore: EOL 추가
* refactor: member Service를 Jwt Service로 분리

* refactor: memberRepository 메서드명 수정

* feat: 예외 초기 세팅

* feat: jwtFilter 예외 처리

* feat: 로그인 및 로그아웃 에 관한 예외처리

* feat: Oauth서비스 관련 예외처리

* feat: 토큰관련 Exception 추가

* refactor: 예외 상수명 변경

* feat: 마일스톤 관련 예외 처리

* feat: 레이블 관련 예외 처리

* feat: 코멘트 관련 예외 처리

* feat: organization 관련 예외 처리

* feat: issue관련 예외 처리

* refactor: 쿼리에 집계함수 사용시 Long을 반환하도록 수정

기존엔 Optional<Long> 으로 되어있었는데 집계함수 사용시 결과값이 null이 될 일이 없다. 따라서 Long을 반환하도록 수정한다.

* feat: IncorrectResultSizeDataAccessException handler 추가

* feat: @Valid 를 사용하여 입력값 검증 추가

* refactor: jinny 리뷰 반영
* feat: 정적필터 조회기능 구현

* refactor: @Valid 예외 수정 및 comment 삭제기능 리팩토링
* feat: redis 초기설정

* feat: 로그아웃 기능 구현

* refactor: Request 에서 token 및 claim 추출 메서드 util 클래스로 생성

* refactor: issue 상세페이지 오류 수정

issue에 milestone 이 지정되있지 않을때 이슈상세페이지에 필요한 데이터를 찾는 부분에서 발생한 오류 수정

* refactor: requestUtil에 불필요한 파싱 삭제
* chore: test 초기세팅

* test: commentControllerTest 작성

* test: CommonControllerTest 작성

* test: filterControlllerTest 작성

* test: milestone 테스트 작성
Copy link

@sallyjellyy sallyjellyy left a comment

Choose a reason for hiding this comment

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

이번 프로젝트 마지막 리뷰네요! 👏
이제 과정 수료 전 프로젝트 하나만을 앞두고 계신 걸로 알고있는데 다들 지금까지 잘 하셨으니 남은 프로젝트도 무사히 진행하실거라고 생각해요. 이번 프로젝트가 끝나도 궁금한 점이나 필요한 게 있으시면 부담 가지지 말고 언제든 디엠 주세요.
그 동안 고생 많으셨습니다!

private final MilestoneRepository milestoneRepository;
private final LabelRepository labelRepository;

@Transactional(readOnly = true)
public NavigationResponse getNavigationInfo(String organizationTitle) {
Long organizationId = organizationRepository.findBy(organizationTitle).orElseThrow();
Long organizationId = organizationService.getOrganizationIdBy(organizationTitle);

Choose a reason for hiding this comment

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

👍


@RequiredArgsConstructor
@RestController
public class AwsController {

Choose a reason for hiding this comment

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

presentation layer이 가져야하는 역할에 대해 한 번 알아보시고 네이밍 고려해보시는거 추천드립니다.

Comment on lines +17 to +22
@PostMapping("/api/images")
public ResponseEntity<String> uploads3(@RequestParam("file") MultipartFile file) throws IOException {
String url = s3Service.save(file);
return ResponseEntity.ok(url);
}
}

Choose a reason for hiding this comment

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

서비스쪽 로직도 체크해봤는데 아무 파일타입이나 다 들어갈 수 있을 것 같네요! 해당 메서드의 역할이 이미지 업로드이니만큼 간단하게나마 확장자명으로라도 검증을 해보면 어떨까 합니다.

@@ -15,6 +20,7 @@
@RestController
public class FilterController {

public static final String MEMBER_ID = "memberId";

Choose a reason for hiding this comment

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

이 친구는.. 뭘까요? (파일 내에서 안 쓰이고 있는 것 같아요!)

@@ -31,10 +37,10 @@ public class IssueController {
private final IssueService issueService;

@PostMapping("/api/{organizationTitle}/issues")
public ResponseEntity<Map<String, Long>> create(@RequestBody IssueCreateRequest issueCreateRequest,
public ResponseEntity<Map<String, Long>> create(@Valid @RequestBody IssueCreateRequest issueCreateRequest,

Choose a reason for hiding this comment

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

👍

Comment on lines +9 to +11
List<IssueAssigneeVo> findAllByIssueId(Long issueId);

List<IssueAssigneeVo> findAllByOrganizationId(Long organizationId);

Choose a reason for hiding this comment

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

👍

Comment on lines +25 to +31
@ExceptionHandler(IncorrectResultSizeDataAccessException.class)
public ResponseEntity<Map<String, String>> incorrectResultSizeDataAccessExceptionHandler(
IncorrectResultSizeDataAccessException e) {
log.info("sql 예외발생! expectedSize: " + e.getExpectedSize() + " actualSize: " + e.getActualSize());
return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
.body(Collections.singletonMap("errorMessage", "DB에서 예외가 발생했습니다."));
}

Choose a reason for hiding this comment

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

이 예외 혹시 지난주에 물어보셨던 queryForObject()와 연관된 게 맞을까요? 맞다면 여기가 아니라 레파지토리에서 핸들되어야 하지 않을까 싶습니다. 조회한 값이 아예 없는 경우 요청에서 원하는 데이터가 없는거니까 404를 띄우는게 더 적절하다고도 생각합니다. DB에서 예외가 발생한다는 메세지도 어떤 의미에서 쓰신건지는 알겠으나 메세지 전달할 때 DB 자체의 문제로 인식될 수 있을 것 같아 이 상황에서는 적합하지 않다고 생각해요.

Copy link
Author

Choose a reason for hiding this comment

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

넵 맞습니다!
모든 Repository에서 IncorrectResultSizeDataAccessException.class를 하나하나 잡는것보다 @ControllerAdvice 에서 한번에 예외를 잡을수 있다면 잡는게 맞는거 같다 생각해 위처럼 예외를 잡았습니다.

새리말씀 듣고보니 위 예외 메세지는 DB자체의 문제로 인식이 될거같아 변경을 하겠습니다. 감사합니다!

@sallyjellyy sallyjellyy merged commit 56a7f17 into codesquad-members-2023:team-03 Aug 20, 2023
kses1010 pushed a commit that referenced this pull request Aug 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants