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

feat : 그룹 삭제 기능 추가 #405 #436

Merged
merged 8 commits into from
May 16, 2024

Conversation

seokho-1116
Copy link
Collaborator

작업 내용 (Content)

  • 그룹 삭제 기능 추가

링크 (Links)

기타 사항 (Etc)

  • 그룹 삭제 시 연쇄 삭제 또는 외래키 제약으로 삭제 실패를 방지하기 위해 더 강한 제약이 필요해 제약조건 적용 후 더 자세하게 문서화함. 아래는 그룹 삭제에 필요한 조건들
    1. 그룹이 존재해야 함.
    2. 그룹 삭제를 요청한 사용자가 그룹장이여야 함.
    3. 그룹에 그룹원이 존재하지 않아야 함.
    4. 그룹에 그룹 캡슐이 존재하지 않아야 함.
    5. 위의 4가지 조건을 만족하면 그룹 삭제
  • 추후 그룹 캡슐 삭제가 필요해 보임

Merge 전 필요 작업 (Checklist before merge)

  • 그룹 관련 작업이 연관되어 있어 그룹 관련 순차적으로 머지 후 최종 머지 필요(ARCH-93-B-feat/group_delete -> 최종)

희망 리뷰 완료 일 (Expected due date)

@seokho-1116 seokho-1116 added the feat 새로운 기능 label May 15, 2024
@seokho-1116 seokho-1116 self-assigned this May 15, 2024
Comment on lines +93 to +95
private void checkGroupOwnership(Long memberId, List<MemberGroup> groupMembers) {
final boolean isGroupOwner = groupMembers.stream()
.anyMatch(mg -> mg.getMember().getId().equals(memberId) && mg.getIsOwner());
Copy link
Collaborator

Choose a reason for hiding this comment

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

93번째 줄부터 매개변수에 final 붙여야겠당

Comment on lines +41 to +42
class GroupCommandServiceTest {

Copy link
Collaborator

Choose a reason for hiding this comment

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

여기서 notOwnerGroupMember() 메서드들 같은걸 Fixture에 안넣고 여기에 넣은 이유가 있어? 이건 걍 궁금

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이거 옮길께 나도 모르게 그냥 테스트에 작성해버림

Comment on lines +24 to +39
public static Member memberWithMemberId(long memberId) {
try {
Member member = member((int) memberId);
setFieldValue(member, "id", memberId);
return member;
} catch (Exception e) {
throw new RuntimeException(e);
}
}

private static void setFieldValue(Object instance, String fieldName, Object value)
throws NoSuchFieldException, IllegalAccessException {
Field field = instance.getClass().getDeclaredField(fieldName);
field.setAccessible(true);
field.set(instance, value);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

기존의 Fixture에서 Member를 만드는 방법을 사용안하고 이렇게 했을 때의 장점이 뭐야?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이거 memberId 넣을 수 있는 생성자가 없어서 이렇게 한거 기존 Member를 테스트위해서 수정할 수 없어성

Copy link
Collaborator

Choose a reason for hiding this comment

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

아 좋다! 굿굿

Comment on lines +24 to +39
public static Member memberWithMemberId(long memberId) {
try {
Member member = member((int) memberId);
setFieldValue(member, "id", memberId);
return member;
} catch (Exception e) {
throw new RuntimeException(e);
}
}

private static void setFieldValue(Object instance, String fieldName, Object value)
throws NoSuchFieldException, IllegalAccessException {
Field field = instance.getClass().getDeclaredField(fieldName);
field.setAccessible(true);
field.set(instance, value);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

아 좋다! 굿굿

@seokho-1116 seokho-1116 merged commit 8e0155f into develop_back_core May 16, 2024
2 checks passed
@seokho-1116 seokho-1116 deleted the ARCH-93-B-feat/group_delete_v3 branch May 16, 2024 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat 새로운 기능
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants