-
Notifications
You must be signed in to change notification settings - Fork 1
feat: all user group/#691 #709
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: develop
Are you sure you want to change the base?
Changes from all commits
04e66c4
a6c78ac
ae7759f
eacc318
9aad857
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| package com.process.clash.application.group.exception.exception.conflict; | ||
|
|
||
| import com.process.clash.application.common.exception.exception.ConflictException; | ||
| import com.process.clash.application.group.exception.statuscode.GroupStatusCode; | ||
|
|
||
| public class CannotQuitGlobalGroupException extends ConflictException { | ||
| public CannotQuitGlobalGroupException() { | ||
| super(GroupStatusCode.CANNOT_QUIT_GLOBAL_GROUP); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,11 +2,13 @@ | |||||||||||||||||||||
|
|
||||||||||||||||||||||
| import com.process.clash.application.common.actor.Actor; | ||||||||||||||||||||||
| import com.process.clash.application.common.exception.exception.ValidationException; | ||||||||||||||||||||||
| import com.process.clash.application.group.exception.exception.conflict.CannotQuitGlobalGroupException; | ||||||||||||||||||||||
| import com.process.clash.application.group.exception.exception.conflict.GroupDuplicateNameException; | ||||||||||||||||||||||
| import com.process.clash.application.group.exception.exception.conflict.GroupMemberLimitReachedException; | ||||||||||||||||||||||
| import com.process.clash.application.group.exception.exception.conflict.GroupOwnerCannotQuitException; | ||||||||||||||||||||||
| import com.process.clash.application.group.exception.exception.forbidden.GroupAccessDeniedException; | ||||||||||||||||||||||
| import com.process.clash.domain.group.entity.Group; | ||||||||||||||||||||||
| import com.process.clash.domain.group.enums.GroupCategory; | ||||||||||||||||||||||
| import org.springframework.stereotype.Component; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| @Component | ||||||||||||||||||||||
|
|
@@ -43,8 +45,23 @@ public void validateMemberLimit(int maxMembers, int memberCount) { | |||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| public void canQuitGroup(Actor actor, Group group) { | ||||||||||||||||||||||
| if (group.category() == GroupCategory.GLOBAL) { | ||||||||||||||||||||||
| throw new CannotQuitGlobalGroupException(); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| if (group.ownerId().equals(actor.id())) { | ||||||||||||||||||||||
| throw new GroupOwnerCannotQuitException(); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| public void validateNotGlobal(Group group) { | ||||||||||||||||||||||
| if (group.category() == GroupCategory.GLOBAL) { | ||||||||||||||||||||||
| throw new GroupAccessDeniedException(); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
Comment on lines
+56
to
+60
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Refactoring
Suggested change
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| public void validateNotGlobal(GroupCategory category) { | ||||||||||||||||||||||
| if (category == GroupCategory.GLOBAL) { | ||||||||||||||||||||||
| throw new GroupAccessDeniedException(); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,7 @@ public void execute(DeleteGroupData.Command command) { | |
| Group group = groupRepositoryPort.findById(command.groupId()) | ||
| .orElseThrow(GroupNotFoundException::new); | ||
|
|
||
| groupPolicy.validateNotGlobal(group); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| groupPolicy.validateOwnership(command.actor(), group); | ||
| List<Long> memberUserIds = groupRepositoryPort.findMemberUserIdsByGroupIds(List.of(group.id())); | ||
| groupRepositoryPort.deleteById(group.id()); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -30,6 +30,8 @@ public void execute(UpdateGroupData.Command command) { | |||||||
| .orElseThrow(GroupNotFoundException::new); | ||||||||
|
|
||||||||
| int currentMemberCount = groupRepositoryPort.countMembers(command.groupId()); | ||||||||
| policy.validateNotGlobal(group); | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current validation only prevents modifying an existing
Suggested change
|
||||||||
| policy.validateNotGlobal(command.category()); | ||||||||
| policy.validateOwnership(command.actor(), group); | ||||||||
| policy.validateMaxMembers(command.maxMembers()); | ||||||||
| policy.validateMemberLimit(group.maxMembers(), currentMemberCount); | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,5 +5,6 @@ public enum GroupCategory { | |
| CLUB, | ||
| TEAM, | ||
| NARSHA, | ||
| ETC | ||
| ETC, | ||
| GLOBAL | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| -- 1. groups 테이블의 fk_user_id 컬럼을 nullable로 변경 | ||
| ALTER TABLE groups ALTER COLUMN fk_user_id DROP NOT NULL; | ||
|
|
||
| -- 2. 전체 유저 그룹 생성 (owner 없음) | ||
| INSERT INTO groups (created_at, updated_at, name, description, max_members, password, password_required, category, fk_user_id) | ||
| VALUES (NOW(), NOW(), '전체 유저', '모든 클래시 유저가 자동으로 참여하는 그룹입니다.', 2147483647, '', false, 'GLOBAL', NULL); | ||
|
|
||
| -- 3. 기존 활성 유저 전원을 GLOBAL 그룹에 추가 | ||
| INSERT INTO group_members (fk_group_id, fk_user_id) | ||
| SELECT (SELECT id FROM groups WHERE category = 'GLOBAL'), u.id | ||
| FROM users u | ||
| WHERE u.user_status = 'ACTIVE'; |
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.
The
findByCategorymethod returns anOptional, which implies that only one group can exist per category. While this is correct for theGLOBALcategory, other categories likeTEAMorCLUBtypically have multiple groups. Calling this method with such categories will result in aNonUniqueResultExceptionat runtime. Consider renaming this tofindGlobalGroup()or changing the return type toList<GroupJpaEntity>to avoid unexpected crashes. Additionally, adding a unique constraint on thecategorycolumn for theGLOBALvalue in the database migration would further ensure data integrity.