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

refactor: 내 피드 리스트 공개 상태 여부 정책 변경에 따른 API 수정 #290

Merged
merged 4 commits into from
Sep 8, 2024

Conversation

pparkjs
Copy link
Collaborator

@pparkjs pparkjs commented Aug 18, 2024

Description

🛠️ 변경한 사항

ex) 1번 리스트가 공개인 경우
ASIS

  • 공개 리스트를 비공개로 설정하려는 경우 기존에는 요청 body에 현재 상태(isPublic = true)를 보내서 API측에서 현재 상태에서 반대로(isPublic = false) 바꾸는 방식으로 진행함
    그런데 이 방식은 프론트측에서 리스트 상세에서 받는 isPublic과 뭔가 매칭이 안되서 햇갈린다고 통일을 요청함

TOBE

  • 해당 리스트가 공개일때 이를 비공개로 바꾸고자 할때 isPublic = false로 보냄( 내가 바꾸고 싶은 상태를 보내는 방식)

  • 해당 부분에 테스트 코드가 존재하지 않아서 테스트 코드를 추가함!

Relation Issues

@pparkjs pparkjs self-assigned this Aug 18, 2024
@pparkjs pparkjs requested a review from kdkdhoho as a code owner August 18, 2024 07:54
@pparkjs pparkjs linked an issue Aug 18, 2024 that may be closed by this pull request
1 task
@kdkdhoho kdkdhoho added 기능 and removed 테스트 labels Aug 30, 2024
Copy link
Collaborator

@kdkdhoho kdkdhoho left a comment

Choose a reason for hiding this comment

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

오랜만의 PR과 코드 리뷰.. 조금 어색하지만 오랜만에 하니 재밌네요 ㅎㅎ
기능 수정 내용 확인했고, 코드와 테스트 코드도 확인했습니다.

그런데 함께 의논하고 싶은 내용이 있어 코멘트 남겨두었으니 확인 부탁드릴게요!

@kdkdhoho
Copy link
Collaborator

아 그리고 "리팩터링, 테스트"로 되어 있던 Label을 "기능, 리팩터링"으로 변경했습니다.
작업 내역에 테스트가 포함되긴 했지만, PR의 주 목적은 기능 변경과 그에 따른 리팩터링이기 때문입니다!

Copy link
Collaborator

@kdkdhoho kdkdhoho left a comment

Choose a reason for hiding this comment

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

로직 너무너무 깔끔합니다~

두 가지 논의드릴 사항이 있어 리뷰 남겼습니다.
이 부분만 해결되면 바로 Approve 해도 될 것 같아요!

Comment on lines +308 to +313
public void changeVisibility(Long loginUserId, Long listId) {
User user = userRepository.getById(loginUserId);
ListEntity list = listRepository.getById(listId);
list.validateOwner(user);
list.updateVisibility();
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

상당히 깔끔하군요 👍

@@ -183,7 +183,7 @@ class ListEntityTest {
void 공개_여부를_수정할_수_있다() {
assertThat(list.isPublic()).isTrue();

list.updateVisibility(false);
list.updateVisibility();
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

@kdkdhoho kdkdhoho left a comment

Choose a reason for hiding this comment

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

반영해주셔서 감사합니다 ☺️
수고하셨어요!!

@@ -120,7 +120,7 @@ ResponseEntity<Void> deleteLists(
@RequestBody ListsDeleteRequest request
) {
listService.deleteLists(userId, request.listId());
return ResponseEntity.ok().build();
return ResponseEntity.noContent().build();
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@pparkjs pparkjs merged commit 56a278b into dev Sep 8, 2024
1 check passed
@pparkjs pparkjs deleted the refactor/289 branch September 8, 2024 07:03
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.

내 피드 공개 / 비공개 api의 isPublic 정책을 바꾼다.
2 participants