[Refactor, Test] Notice 도메인 작업 #113 #116

seunghyun333 · 2023-12-03T15:14:07Z

작업 PR
#113

작업내역
Notice Controller 및 Service Refactoring
Notice기능의 TestCase제작

다음 차례는 ,,, 두근두근

컨트롤러 -> 서비스 수정

donsonioc2010

이번엔 리뷰다라봤슴미담.

수정할지 말지는 승현님 마음이기떄문에, 일단 Approve는 해놓는걸로... :)

src/main/java/bleuauction/bleuauction_be/server/notice/controller/NoticeController.java

donsonioc2010 · 2023-12-03T16:32:55Z

src/main/java/bleuauction/bleuauction_be/server/notice/controller/NoticeController.java

-    if(loginUser.get().getMemberCategory() == A) {
-    notice.setMemberNo(loginUser.get());
-
-    if (multipartFiles != null && multipartFiles.size() > 0) {
-      ArrayList<Attach> attaches = new ArrayList<>();
-      for (MultipartFile multipartFile : multipartFiles) {
-        if (multipartFile.getSize() > 0) {
-          Attach attach = ncpObjectStorageService.uploadFile(new Attach(),
-                  "bleuauction-bucket", "notice/", multipartFile);
-          notice.addNoticeAttach(attach);
-        }
-      }
-    }


donsonioc2010 · 2023-12-03T16:35:07Z

src/main/java/bleuauction/bleuauction_be/server/notice/controller/NoticeController.java

      List<Notice> notices = noticeService.findNoticesByStatus(NoticeStatus.Y);
      return notices;


동일한 코드가 되지않나 싶습니다 ㅋㅋ

Suggested change

List<Notice> notices = noticeService.findNoticesByStatus(NoticeStatus.Y);

return notices;

return noticeService.findNoticesByStatus(NoticeStatus.Y);

donsonioc2010 · 2023-12-03T16:35:24Z

src/main/java/bleuauction/bleuauction_be/server/notice/controller/NoticeController.java

  }


 // 삭제
-  @PostMapping("/api/notice/delete/{noticeNo}")
+  @DeleteMapping("/{noticeNo}")


donsonioc2010 · 2023-12-03T16:37:18Z

src/main/java/bleuauction/bleuauction_be/server/notice/controller/NoticeController.java

-    } else {
-      return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body("관리자 권한이 필요합니다");
-    }
+    noticeService.deleteNotice(noticeNo,loginUser.get());


Optional을 사용하는 안좋은 예라고 말씀드릴수 있을 것 같습니다 ㅎㅎ;;

src/main/java/bleuauction/bleuauction_be/server/notice/service/NoticeService.java

src/main/java/bleuauction/bleuauction_be/server/notice/controller/NoticeController.java

donsonioc2010 · 2023-12-03T17:01:58Z

src/main/java/bleuauction/bleuauction_be/server/notice/service/NoticeService.java

+  public Long enroll(Notice notice, List<MultipartFile> multipartFiles, Member member) {
+
+    if(member.getMemberCategory() == MemberCategory.A) {
+      notice.setMemberNo(member);
+      noticeRepository.save(notice);
+
+      if (multipartFiles != null && multipartFiles.size() > 0) {
+        ArrayList<Attach> attaches = new ArrayList<>();
+        for (MultipartFile multipartFile : multipartFiles) {
+          if (multipartFile.getSize() > 0) {
+            Attach attach = ncpObjectStorageService.uploadFile(new Attach(),
+                    "bleuauction-bucket", "notice/", multipartFile);
+            notice.addNoticeAttach(attach);
+          }
+        }
+      }
+
+    }
    return notice.getNoticeNo();
+
  }


동일한 기능인데 전 이렇게 할 것 같아요.

if문을 계속 타고 들어가면 가독성이 떨어지다 보니 최대한 제거하는 방향으로 작성을 하려고 전 지향합니다 ㅎㅎ :)

번외로 스트림 익혀보는것도 권장합니다 ㅋㅋ

Suggested change

public Long enroll(Notice notice, List<MultipartFile> multipartFiles, Member member) {

if(member.getMemberCategory() == MemberCategory.A) {

notice.setMemberNo(member);

noticeRepository.save(notice);

if (multipartFiles != null && multipartFiles.size() > 0) {

ArrayList<Attach> attaches = new ArrayList<>();

for (MultipartFile multipartFile : multipartFiles) {

if (multipartFile.getSize() > 0) {

Attach attach = ncpObjectStorageService.uploadFile(new Attach(),

"bleuauction-bucket", "notice/", multipartFile);

notice.addNoticeAttach(attach);

}

}

}

}

return notice.getNoticeNo();

}

public Long enroll(Notice notice, List<MultipartFile> multipartFiles, Member member) {

if (!MemberCategory.A.equals(member.getMemberCategory())) {

// 관리자가 아니기 떄문에 예외처리 로직

}

notice.setMemberNo(member);

if (multipartFiles != null && !multipartFiles.isEmpty()) {

multipartFiles.stream()

.filter(file -> file.getSize() > 0)

.forEach(multipartFile ->

notice.addNoticeAttach(ncpObjectStorageService.uploadFile(new Attach(),

"bleuauction-bucket", "notice/", multipartFile))

);

}

return noticeRepository.save(notice).getNoticeNo();

}

donsonioc2010 · 2023-12-03T17:04:34Z

src/main/java/bleuauction/bleuauction_be/server/notice/service/NoticeService.java

+  public void deleteNotice(Long noticeNo, Member member) {
+    if (member.getMemberCategory()==MemberCategory.A) {
+      Notice notice = noticeRepository.findByNoticeNo(noticeNo);
+      notice.delete();
+      if (notice.getNoticeAttaches() != null && !notice.getNoticeAttaches().isEmpty()) {
+        for (Attach attach : notice.getNoticeAttaches()) {
+          attachService.changeFileStatusToDeleteByFileNo(attach.getFileNo());
+        }
+      }
+    }
  }


요기도 요론식으로, If문을 제거해볼 수 있을 것 같아요.

Suggested change

public void deleteNotice(Long noticeNo, Member member) {

if (member.getMemberCategory()==MemberCategory.A) {

Notice notice = noticeRepository.findByNoticeNo(noticeNo);

notice.delete();

if (notice.getNoticeAttaches() != null && !notice.getNoticeAttaches().isEmpty()) {

for (Attach attach : notice.getNoticeAttaches()) {

attachService.changeFileStatusToDeleteByFileNo(attach.getFileNo());

}

}

}

}

@Transactional

public void deleteNotice(Long noticeNo, Member member) {

if(!MemberCategory.A.equals(member.getMemberCategory())) {

// 관리자가 아니기 떄문에, 예외처리

}

Notice notice = noticeRepository.findByNoticeNo(noticeNo);

notice.delete();

if (notice.getNoticeAttaches() != null && !notice.getNoticeAttaches().isEmpty()) {

for (Attach attach : notice.getNoticeAttaches()) {

attachService.changeFileStatusToDeleteByFileNo(attach.getFileNo());

}

}

}

donsonioc2010 · 2023-12-03T17:05:25Z

src/main/java/bleuauction/bleuauction_be/server/notice/service/NoticeService.java

+
+    Notice existingnotice = noticeRepository.findByNoticeNo(updatedNotice.getNoticeNo());
+
+    if(member.getMemberCategory() == A) {


요기도 흐흐흐

seunghyun333 added 6 commits December 1, 2023 22:14

notice Controller mapping 수정

cbd8c73

notice repository 인터페이스로 수정

dc9a6f1

[Refactor, Test] Notice 도메인 작업 #113

c456b36

컨트롤러 -> 서비스 수정

Merge branch 'dev' into feat/113_NoticeRefactor

14d9eea

dev merge

a62fd0c

local-yml 내 db용으로 고쳤던 것 원래대로 수정

59f0c7b

seunghyun333 requested review from donsonioc2010 and itkw87 December 3, 2023 15:14

NoticeRepository 파일 명 수정

85e9427

donsonioc2010 mentioned this pull request Dec 3, 2023

[Feat, Refactor] Member Architecture Refactoring #117

Merged

donsonioc2010 approved these changes Dec 3, 2023

View reviewed changes

seunghyun333 added 5 commits December 6, 2023 00:37

종원 코드리뷰 참고 1차 수정/ if문 수정 전

bab0ee7

종원 코드리뷰 참고 수정

7d1585f

test 수정

c2471e5

Notice file delete Controller 수정

1030559

Notice Service update 수정

a1f3ea3

seunghyun333 merged commit 0918ead into dev Dec 7, 2023

seunghyun333 deleted the feat/113_NoticeRefactor branch December 7, 2023 04:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Refactor, Test] Notice 도메인 작업 #113 #116

[Refactor, Test] Notice 도메인 작업 #113 #116

seunghyun333 commented Dec 3, 2023

donsonioc2010 left a comment •

edited

Loading

donsonioc2010 Dec 3, 2023

donsonioc2010 Dec 3, 2023 •

edited

Loading

donsonioc2010 Dec 3, 2023

donsonioc2010 Dec 3, 2023

donsonioc2010 Dec 3, 2023

donsonioc2010 Dec 3, 2023

donsonioc2010 Dec 3, 2023

		List<Notice> notices = noticeService.findNoticesByStatus(NoticeStatus.Y);
		return notices;

	List<Notice> notices = noticeService.findNoticesByStatus(NoticeStatus.Y);
	return notices;
	return noticeService.findNoticesByStatus(NoticeStatus.Y);


		Notice existingnotice = noticeRepository.findByNoticeNo(updatedNotice.getNoticeNo());

		if(member.getMemberCategory() == A) {

[Refactor, Test] Notice 도메인 작업 #113 #116

[Refactor, Test] Notice 도메인 작업 #113 #116

Conversation

seunghyun333 commented Dec 3, 2023

donsonioc2010 left a comment • edited Loading

Choose a reason for hiding this comment

donsonioc2010 Dec 3, 2023

Choose a reason for hiding this comment

donsonioc2010 Dec 3, 2023 • edited Loading

Choose a reason for hiding this comment

donsonioc2010 Dec 3, 2023

Choose a reason for hiding this comment

donsonioc2010 Dec 3, 2023

Choose a reason for hiding this comment

donsonioc2010 Dec 3, 2023

Choose a reason for hiding this comment

donsonioc2010 Dec 3, 2023

Choose a reason for hiding this comment

donsonioc2010 Dec 3, 2023

Choose a reason for hiding this comment

donsonioc2010 left a comment •

edited

Loading

donsonioc2010 Dec 3, 2023 •

edited

Loading