feat: group managment logic #534
Conversation
📝 Walkthrough워크스루이 풀 리퀘스트는 그룹 관리 기능에 대한 포괄적인 개선을 다룹니다. 주요 변경 사항은 다국어 지원 파일에 새로운 확인 메시지 추가, 그룹 관련 데이터 모델 및 엔티티 업데이트, 그룹 관리를 위한 새로운 블록 및 페이지 구현, 그리고 라우팅 구성 재구성을 포함합니다. 이러한 변경은 사용자 경험을 향상시키고 그룹 관리 기능의 유연성을 높이는 데 중점을 둡니다. 변경 사항
시퀀스 다이어그램sequenceDiagram
participant User
participant GroupManagementBloc
participant GroupRepository
participant API
User->>GroupManagementBloc: 그룹 삭제 요청
GroupManagementBloc->>GroupRepository: deleteGroup(uuid)
GroupRepository->>API: 그룹 삭제 API 호출
API-->>GroupRepository: 삭제 성공
GroupRepository-->>GroupManagementBloc: 삭제 완료
GroupManagementBloc->>User: 상태 업데이트
관련 가능성이 있는 PR
추천 리뷰어
시
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
dawnfire05
force-pushed
the
506-v4-group-management-page-logic
branch
from
3d20f0b to
179d22e
Compare
lib/app/modules/common/presentation/widgets/ziggle_row_button.dart
Outdated
Show resolved
Hide resolved
lib/app/modules/groups/presentation/pages/group_management_description_page.dart
Outdated
Show resolved
Hide resolved
lib/app/modules/groups/presentation/pages/group_management_name_page.dart
Outdated
Show resolved
Hide resolved
lib/app/modules/groups/presentation/pages/group_management_name_page.dart
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
dawnfire05
force-pushed
the
506-v4-group-management-page-logic
branch
2 times, most recently
from
5330450 to
465004c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
assets/i18n/strings_jp.i18n.json (1)
Line range hint
108-117: detail 섹션에 러시아어가 잘못 포함되어 있습니다.일본어 파일에 러시아어 텍스트가 잘못 들어가 있습니다. 일본어로 수정이 필요합니다.
다음과 같이 수정해주세요:
- "backLabel": "Список", - "title": "Информация о группе" + "backLabel": "リスト", + "title": "グループ情報"- "introduction": "Введение", - "notice": "Объявления", - "member": "Участники" + "introduction": "紹介", + "notice": "お知らせ", + "member": "メンバー"- "favorite": "Избранное" + "favorite": "お気に入り"
🧹 Nitpick comments (49)
lib/app/modules/groups/domain/repository/group_repository.dart (6)
15-17: watchGroups / getGroups 메서드 분리 상태
watchGroups()와getGroups()가 유사한 목적(그룹 목록 조회)을 가지므로, 스트림과 단일 조회 형태를 확실히 분리·관리하고 테스트 시나리오를 보강하면 좋겠습니다.
24-27: modifyName 메서드에 대한 유효성 검증 권장
새로운 이름의 값이 서버 단에서 거부될 가능성이 있다면, 클라이언트 차원에서 길이·포맷 검증 로직을 고려해 주세요.
32-35: modifyNotionLink 메서드 유효성 확인
Notion 링크가 올바른 형식인지 유효성 검사가 필요한지 여부를 고민해 보세요.
40-40: getMembers 메서드에서의 확장성
멤버 목록 조회 시 대규모 그룹에 대한 페이징 처리나 필터 기능이 필요한지 검토해 보시기 바랍니다.
56-56: deleteGroup 메서드 후속 처리
그룹 삭제 시 연관 데이터(멤버, 채팅 등)가 어떻게 정리되는지 검토가 필요하며, 서버에서 원자성을 보장하는지 확인해 주세요.
57-57: leaveGroup 메서드 로직 점검
그룹 탈퇴 시 관리자 없이 그룹이 남을 가능성, 혹은 유저 재가입 정책 등을 생각해 볼 필요가 있습니다.lib/app/modules/groups/presentation/pages/group_management_name_page.dart (2)
24-27: State 객체 생성
_GroupManagementNamePageState를 반환하는 구조가 적절합니다. 추가 로직이 필요하다면 개인화된 입력 검증 등을 고려해 보세요.
33-37: initState에서 addListener
_controller.addListener로 즉시 setState를 호출하는 구조는 편리하나, 성능 측면에서 변경 이벤트가 잦다면 Debounce 등을 고려할 수 있습니다.lib/app/modules/groups/data/repositories/rest_group_repository.dart (9)
26-35: createGroup 메서드 변경
이미지 업로드 후_refreshGroups()를 통해 스트림 데이터까지 갱신하는 구조입니다. API 응답이 실패할 경우 예외 처리를 어떻게 다룰지 추가 검토가 필요합니다.
54-57: getGroup 메서드
단순히_api.getGroup(uuid)만 호출하고 있습니다. 예외 캐치나 로깅 로직이 필요한지는 검토해 보세요.
76-79: modifyNotionLink 메서드
특정 Notion 페이지를 연결한 후 UI 갱신 시점이 중요하다면_refreshGroups()같은 로직이 DB나 캐시와 어떻게 반영될지 확인이 필요합니다.
88-92: createInviteLink 메서드
생성된code를 바로 반환하고 있습니다. UI에서 이 코드를 어떻게 보여주고 복사할지, 혹은 만료 시간 정보가 필요한지 검토해 보세요.
94-98: getMembers 메서드 미구현
throw UnimplementedError()로 되어 있으니, 실제 구현이 빠른 시일 내 필요합니다.
100-105: grantRoleToUser 메서드 미구현
역할 부여 자체가 중요한 기능이라면 우선순위를 높게 잡아 구현해야 할 것 같습니다.
107-111: leaveGroup 메서드 미구현
유저가 그룹을 탈퇴할 때 후속 처리(멤버 목록, 권한 해제 등) 로직이 필요합니다.
113-118: removeMember 메서드 미구현
관리자 권한 여부, 대상 사용자와의 관계 등을 고려하여 구현이 필요합니다.
121-124: removeRoleFromUser 메서드 미구현
역할 해제 기능도 필수적인 운영 로직이므로, 다른 메서드들과 함께 일관성 있게 처리해주세요.lib/app/modules/groups/presentation/pages/group_management_invitation_link_page.dart (2)
53-100: BlocBuilder 구조
초대 링크 처리 상태(GroupInviteState)에 따라 다이나믹하게 UI를 구성하고 있습니다.success시점에 버튼을 보여주는 로직이 의도대로 동작하는지, 에러 처리가 필요한지 고려 부탁드립니다.
103-108: 마무리 버튼
사용자가_duration선택 후 나가거나 단순히 닫을 수 있는 로직입니다. 초대 링크가 잘 생성됐는지 여부를 알려주는 피드백을 추가해도 좋겠습니다.lib/app/modules/groups/presentation/pages/group_management_page.dart (2)
9-9: 공통 presentation/extensions/confirm 추가
사용자에게 Dialog를 띄우는 확장 로직이라면, navigate와 중복 구현이 없는지 확인이 필요합니다.
32-46: BlocBuilder로 그룹 관리 상태 분기 처리
maybeWhen구문을 통해 초기/로딩/에러/성공 상태를 구분했습니다. 에러 메시지나 로딩 화면의 디자인을 UX 팀과 협의해 보는 것도 좋겠습니다.lib/app/modules/groups/domain/entities/member_list_entity.dart (1)
6-7: [생성자 파라미터]
현재 생성자는memberList를required로 받아 무조건 목록이 있어야 한다는 점이 명확해 좋습니다. 추후 회원이 없는 그룹의 처리가 필요하다면, 옵셔널 처리를 고려해볼 수도 있습니다.lib/app/modules/groups/domain/entities/member_entity.dart (1)
8-15: [생성자 요구사항 검증]
모든 필드가required로 설정되어 있어, 객체 생성 시 누락될 가능성이 줄어듭니다. 추후role이 비어 있을 수 있는 비즈니스 케이스가 생긴다면 옵셔널 처리를 재검토해야 합니다.lib/app/modules/groups/data/data_sources/models/group_invite_code_request_model.dart (1)
6-13: [duration 필드 유효성]
duration이 초, 분, 시간 등을 나타내는지 명시가 필요해 보입니다. 추후 혼동을 막기 위해 주석 또는 필드명 변경도 고려해주세요. JSON 변환 로직이 자동 생성 방식이므로, JSON 키-값 구조가 실제 API와 일치하는지 재확인하면 좋겠습니다.lib/app/modules/groups/presentation/layouts/group_management_shell_layout.dart (1)
16-19: BlocProvider를 통한 초기화 로직
load이벤트에group.uuid를 바로 전달하는 방식이 깔끔합니다. 다만,group.uuid가 비어 있을 경우를 처리해야 한다면 별도의 예외 처리가 필요할 수 있습니다.lib/app/modules/groups/domain/entities/group_entity.dart (1)
23-24: [생성자 변경] 필수(required) 파라미터와 nullable 파라미터 간 혼동 주의
생성자로 들어오는description,president,presidentUuid등이 nullable 파라미터인 만큼, 초기 로직에서 null 처리를 깜빡하지 않도록 주의가 필요합니다.Also applies to: 31-31
lib/app/modules/groups/data/data_sources/models/group_model.dart (2)
15-15: [필수 파라미터에 nullable 타입] 사용 시 주의
description,profileImageUrl등이 nullable이지만 생성자에서required로 선언되어 있습니다. 다른 레이어에서 올 때 null일 수 있으나, 여전히 꼭 전달해야 한다는 의미로 보입니다. 혹시required와 nullable의 조합이 혼동을 줄 가능성이 있는지 고민해 보세요. 또한president필드가PresidentModel타입으로 설정되어 있어, 기존 UUID 접근 로직과 충돌은 없는지 확인해 주세요.Also applies to: 17-17, 24-24
30-31: [오버라이드]presidentUuid가 null만 반환됨
기존에 String 타입으로 존재했던presidentUuid를 override에서 무조건 null로 반환하고 있습니다. 호출하는 측에서 이를 감안해 null 처리 로직을 준비해야 합니다.lib/app/modules/groups/data/data_sources/models/group_item_model.dart (1)
1-35: [신규 모델 추가]GroupItemModel구현 시 주의사항
GroupEntity를 구현하면서president,memberCount를 무조건null로 돌려주고 있습니다. 이후 확장이 필요할 경우, 실제 데이터를 매핑하도록 수정해야 할 수 있으니 주의하세요.- New file이므로, JSON 직렬화/역직렬화가 기대대로 동작하는지 테스트 커버리지를 확인해 보시는 걸 권장드립니다.
lib/app/modules/groups/presentation/blocs/group_invite_bloc.dart (2)
22-26: [이벤트 정의]create이벤트에 대한 상세 설명
이벤트 생성자가uuid와duration만 받고 있습니다. 추후 확장 시 다른 파라미터가 필요하다면 새 이벤트를 만들지 고민해 보세요.freezed를 사용 중이므로, 코드 중복 없이 이벤트를 확장할 수 있습니다.
27-32: [상태 정의] 에러 상태 추가 제안
initial,loading,success만 있는 상태 구조에서는 오류 상황이 발생했을 때 대응이 어려울 수 있습니다. 에러 상황을 다루는 별도의failure상태를 고려해 보세요.lib/app/modules/common/presentation/widgets/ziggle_row_button.dart (1)
38-38: leadingIcon 렌더링 조건
leadingIcon이null이 아닐 때만 표시하여 UI 레이아웃이 간결해집니다.lib/app/modules/groups/presentation/pages/group_management_member_page.dart (1)
35-41: 멤버 관리 카드 표시
GroupMemberCard.editMode를 통해 멤버 권한 변경이나 추방 기능을 노출할 수 있습니다. 실제 데이터 연동 시, 이름·이메일 등 정보를 안전하게 전달하고 중복 여부나 삭제 권한 등을 서버에서 추가 확인해야 합니다.lib/app/modules/groups/presentation/pages/group_management_notion_page.dart (1)
62-75: BlocBuilder 사용으로 즉시 상태 반영
버튼 비활성화 로직과 Bloc 액션(링크 업데이트) 처리를 결합하여 동적 UI 흐름을 자연스럽게 구성했습니다. 필요하다면 API 응답 시간에 따른 로딩 상태를 별도로 고려해 보세요.lib/app/modules/groups/presentation/pages/group_management_description_page.dart (2)
3-4: [사용되지 않는 의존성 관리] 불필요한 import 혹은 사용 모니터링 필요
flutter_bloc과locator가 적절히 사용되고 있는지 주기적으로 확인하세요. 사용되지 않는다면 정리하는 편이 좋습니다.
55-108: [UI 구성 최적화] SingleChildScrollView 사용 및 BlocBuilder로 상태 관리
- 내용이 많거나 화면이 작을 때도 스크롤이 가능하므로 사용자 경험이 개선됩니다.
- BlocBuilder로 버튼 활성화 여부를 제어해, 비어있거나 변경 사항이 없을 때는 액션을 방지합니다.
lib/app/modules/groups/presentation/blocs/group_management_bloc.dart (8)
11-17: [Bloc 생성자 로직] GroupRepository 주입 후 초기 상태 설정
의존성 주입이 정상적으로 동작하고 있으며, 처음에는GroupManagementState.initial()을 사용해 초기화합니다.
24-29: [이벤트: _UpdateProfileImage] 프로필 이미지를 업데이트하고 상태 관리
이미지 업로드가 실패할 경우 예외 처리가 필요합니다. 필요하다면 try-catch를 고려해 보세요.
40-50: [이벤트: _UpdateDescription] 예외 처리 시 예외 타입 확장
현재Exception을 대상으로 잡고 있는데, 더 세분화된 예외나 도메인 예외 클래스를 도입해보는 것도 좋습니다.
60-63: [이벤트: _GetMembers] 가져온 멤버 목록의 처리
멤버 데이터를 state로 반영하지 않고 있으니, 필요하다면 UI 업데이트 로직을 재검토할 수 있습니다.
64-72: [이벤트: _RemoveMember] 그룹원 추방 후 상태 갱신
제외 처리 후를 표현하는 Flow를 더 자세히 만들어도 좋겠습니다(예: 알림, 스낵바 등).
83-92: [이벤트: _RemoveRoleFromUser] 역할 제거 후 상태 갱신
역할 제거 시점에 추가 검증(관리자 권한 여부 등)이 필요한지 여부를 프로젝트 요구사항과 맞춰 확인해주세요.
93-96: [이벤트: _Delete] 그룹 삭제 시 후속 처리
삭제 성공 후에 다른 화면으로 이동하거나 UI를 갱신해야 한다면, 관련 로직을 별도 이벤트로 분리할 수 있습니다.
127-137: [GroupManagementState] 다양한 상태 정의
초기, 로딩, 성공, 에러 상태를 명확히 구분하고, 초대코드나 완료 상태도 별도로 둬 유지보수에 유리합니다.lib/app/modules/groups/presentation/pages/group_management_main_page.dart (2)
30-32: [코멘트]_Layout클래스 명에 대해 고민해 보세요.
UI 구성 요소를 의도적으로 분리했다면,_Layout이라는 추상적인 이름보다 의도를 명확히 표현하는 이름을 고려해 보세요.- class _Layout extends StatelessWidget { + class _GroupManagementLayout extends StatelessWidget {
73-128: [코멘트] BlocBuilder 내에서state.when사용은 상태별 분기가 명확하며, 읽기 쉽습니다.
에러 상태 처리와 로딩 애니메이션이 한눈에 파악되어 유지보수가 용이합니다. 다만, 에러 메시지를 좀 더 구체화하여 사용자 경험을 개선할 수 있습니다.- emit(_Error("No groups found")); + emit(_Error("그룹이 없습니다. 새 그룹을 생성하거나, 나중에 다시 시도해주세요."));lib/app/modules/groups/presentation/blocs/group_management_main_bloc.dart (2)
19-21: [코멘트]watchGroups()를 구독하여 실시간으로 업데이트를 반영하는 구조가 좋습니다.
단, 에러 상황이나 해제 처리를 적절히 고려해야 합니다.
23-43: [코멘트]_Load이벤트와_GroupsUpdated이벤트를 통해 그룹 목록을 로드하고 업데이트하는 로직이 명료합니다.
_Error상태 처리를 좀 더 구체화하면, 디버깅 시 유용할 수 있습니다.assets/i18n/strings_ru.i18n.json (1)
90-91: 초대 링크 관련 문자열이 적절히 구현되었습니다!초대 링크 복사 기능 추가는 사용자 편의성을 향상시킬 것으로 보입니다. 다만, 복사 성공 시 표시할 메시지도 추가하면 좋을 것 같습니다.
다음과 같은 메시지 추가를 고려해보세요:
{ "manage": { "invite": { "invitationLink": "Ссылка для приглашения", "copy": "Копировать ссылку на приглашение", + "copied": "Ссылка скопирована" } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
assets/icons/lock.svgis excluded by!**/*.svg
📒 Files selected for processing (33)
assets/i18n/strings.i18n.json(2 hunks)assets/i18n/strings_jp.i18n.json(2 hunks)assets/i18n/strings_ko.i18n.json(2 hunks)assets/i18n/strings_ru.i18n.json(2 hunks)lib/app/modules/common/presentation/widgets/ziggle_row_button.dart(3 hunks)lib/app/modules/groups/data/data_sources/models/group_invite_code_request_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/models/group_invite_code_response_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/models/group_item_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/models/group_list_model.dart(2 hunks)lib/app/modules/groups/data/data_sources/models/group_model.dart(2 hunks)lib/app/modules/groups/data/data_sources/models/modify_group_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/models/president_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/remote/group_api.dart(3 hunks)lib/app/modules/groups/data/repositories/rest_group_repository.dart(1 hunks)lib/app/modules/groups/domain/entities/group_entity.dart(1 hunks)lib/app/modules/groups/domain/entities/member_entity.dart(1 hunks)lib/app/modules/groups/domain/entities/member_list_entity.dart(1 hunks)lib/app/modules/groups/domain/entities/president_entity.dart(1 hunks)lib/app/modules/groups/domain/repository/group_repository.dart(1 hunks)lib/app/modules/groups/presentation/blocs/group_invite_bloc.dart(1 hunks)lib/app/modules/groups/presentation/blocs/group_management_bloc.dart(1 hunks)lib/app/modules/groups/presentation/blocs/group_management_main_bloc.dart(2 hunks)lib/app/modules/groups/presentation/layouts/group_management_shell_layout.dart(1 hunks)lib/app/modules/groups/presentation/pages/group_management_description_page.dart(3 hunks)lib/app/modules/groups/presentation/pages/group_management_invitation_link_page.dart(2 hunks)lib/app/modules/groups/presentation/pages/group_management_main_page.dart(1 hunks)lib/app/modules/groups/presentation/pages/group_management_member_page.dart(1 hunks)lib/app/modules/groups/presentation/pages/group_management_name_page.dart(2 hunks)lib/app/modules/groups/presentation/pages/group_management_notion_page.dart(2 hunks)lib/app/modules/groups/presentation/pages/group_management_page.dart(2 hunks)lib/app/modules/user/presentation/bloc/group_auth_bloc.dart(1 hunks)lib/app/modules/user/presentation/pages/profile_page.dart(3 hunks)lib/app/router.dart(1 hunks)
🔇 Additional comments (104)
lib/app/modules/groups/domain/repository/group_repository.dart (9)
3-5: 새로운 Entity import 확인
GroupEntity, GroupListEntity, MemberListEntity를 잘 불러오고 있는지 확인해주세요. 기존 Model 기반 코드와 충돌이 없는지 점검이 필요합니다.
8-14: createGroup 메서드 반환 타입 변경 검증
GroupModel → GroupEntity로 변경되었습니다. 기존 호출부가 모두 수정되었는지, 또한 예외 상황(예: image가 null일 때)을 충분히 처리하고 있는지 확인을 권장합니다.
18-19: getGroup 메서드 예외 처리
인자로 받은 uuid가 유효하지 않은 경우에 대한 예외 처리가 필요할 수 있습니다.
20-23: modifyProfileImage 필수 인자 확인
File image가 반드시 필요한 구조여서 null 전송 가능성이 없다면 괜찮지만, 호출부에서 누락되지 않도록 주의하십시오.
28-31: modifyDescription 메서드의 null 처리
description이 null일 때 서버 쪽에서 필드가 제거되는지, 혹은 빈 문자열로 처리되는지 확인이 필요합니다.
37-39: createInviteLink 메서드 보안 고려
초대 링크 발급 시 만료 시간(duration) 등 보안/관리 정책 논의가 필요할 수 있습니다.
41-44: removeMember 메서드 권한 체크
멤버 추방 시 호출자가 관리자 권한을 가지고 있는지 여부 등, 권한 로직을 API 차원에서 어떻게 보장할지 확인이 필요합니다.
45-49: grantRoleToUser 메서드 중복 부여 방지
역할을 부여할 때 이미 해당 역할이 주어져 있는 상황에 대한 체크 혹은 에러 처리가 필요한지 검토해 주세요.
50-54: removeRoleFromUser 메서드 예외 대응
해당 유저에게 부여되지 않은 역할을 제거하려고 할 때 에러가 발생하는지 여부를 확인해 보세요.
lib/app/modules/groups/presentation/pages/group_management_name_page.dart (8)
3-4: Bloc 활용을 위한 import 추가
flutter_bloc이 정상적으로 적용되어 있으면, 기존 로직과 충돌 없이 BlocBuilder 등을 활용할 수 있을 것입니다. 문제가 없는지 확인 부탁드립니다.
9-9: group_management_bloc.dart import
GroupManagementBloc이 정상적으로 연결되었는지, 의존성 주입(Locator)이 올바른 순서로 작동하는지 확인해 주세요.
14-14: StatelessWidget → StatefulWidget 전환
상태 관리를 위해 StatefulWidget이 필요한 점이 명확히 구조화되어 있어, 기존 코드나 라우팅 로직과 충돌이 없는지 살펴보시기 바랍니다.
15-19: 생성자: uuid와 name 필수 인자
빈 문자열 등 잘못된 값이 넘어갈 가능성을 고려해, 필요한 경우 사전에 유효성 검증을 추가하는 방안을 생각해 보세요.
21-23: final 필드 선언
uuid, name 필드가 final로 안전하게 선언되었습니다. 변경되지 않는 값이라면 적절하게 처리된 것으로 보입니다.
29-31: TextEditingController 초기화
widget.name으로 기본값을 설정하여 사용자가 기존 이름을 확인할 수 있게 하는 로직이 적절합니다.
69-72: ZiggleInput 컨트롤러 연동
hintText와 controller가 잘 연결되었습니다. 사용자 입력 시 ZiggleButton 활성화 로직과 연계해 동작하는지 테스트가 필요합니다.
74-91: BlocBuilder로 버튼 활성화 제어
disabled 조건이 _controller.text.isEmpty || _controller.text == widget.name로 명확히 관리되고 있습니다. 변경 이벤트에 따라 Bloc 이벤트 전송이 올바르게 동작하는지 점검해 주세요.
lib/app/modules/groups/data/repositories/rest_group_repository.dart (12)
4-4: rxdart 라이브러리 사용
BehaviorSubject를 통해 실시간 목록 업데이트를 관리하려는 의도가 명확합니다. 메모리 해제 시점에서 close() 호출이 누락되지 않도록 주의하세요.
6-8: 새로운 Model import
CreateGroupModel, GroupInviteCodeRequestModel, GroupListModel, ModifyGroupModel 등 여러 모델이 추가/수정되었습니다. 변경된 부분이 API 계약과 일치하는지 확인해 주세요.
11-13: Entity 참조 추가
GroupEntity, GroupListEntity, MemberListEntity 등 도메인에 맞는 Entity 처리가 잘 연동되었는지 점검 바랍니다.
16-16: @singleton 적용
@Injectable에서 @Singleton으로 교체되었습니다. 라이프사이클이나 주입 범위가 문제없이 의도대로 동작하는지 확인을 권장합니다.
20-21: BehaviorSubject 초기값
BehaviorSubject.seeded(GroupListEntity(list: []))로 빈 리스트를 초기값으로 두었습니다. 첫 구독 시 UI 표시가 잘 동작하는지 확인해 보세요.
[approve]
39-42: _refreshGroups 메서드
그룹 정보를 다시 불러온 뒤 _groupsSubject에 반영해주는 로직이 간결합니다. 다만, 연속 호출 시 서버 부하가 커질 수 있으니 호출 빈도도 고려해 보세요.
[approve]
45-46: watchGroups 구현
Subject의 스트림을 직접 반환하기 때문에, 참조가 해제되지 않는지(메모리 누수 등) 모니터링을 권장합니다.
48-51: getGroups 메서드
GroupListEntity 대신 GroupListModel을 곧바로 반환받고, 내부에서 GroupListEntity로 상호 변환하는 과정이 있는지 확인해주세요.
58-61: modifyProfileImage 메서드
이미지 업로드만 처리하고, 후속 _refreshGroups() 호출이 없습니다. 사용자가 즉시 반영된 UI를 볼 수 있어야 한다면, 추가 갱신 로직이 필요할 수 있습니다.
63-67: modifyName 메서드
_api.modifyGroup 후 _refreshGroups()를 호출해 그룹 목록을 업데이트합니다. 개별 그룹 정보만 다시 불러오는 메서드와의 일관성도 고려해 보세요.
[paid_attention]
70-73: modifyDescription 메서드
description이 null일 수 있으므로, 서버가 null을 어떻게 처리하는지(삭제 혹은 빈 문자열로 업데이트 등) 체크가 필요합니다.
81-85: deleteGroup 메서드
_api.deleteGroup 후 _refreshGroups()로 스트림을 갱신합니다. 그룹 삭제 후 사용자가 접근 중인 화면이 꼬이지 않도록 라우팅 처리도 점검해 주십시오.
lib/app/modules/groups/presentation/pages/group_management_invitation_link_page.dart (6)
3-5: Clipboard 사용 및 Bloc 추가
flutter/services.dart와 flutter_bloc을 활용한 초대 링크 복사 기능이 필요에 맞게 동작할지 테스트해 보세요.
10-10: GroupInviteBloc import
의존성 주입(Locator)나 BlocProvider 생성이 올바른 순서로 동작하는지, 충돌 없는지 확인 부탁드립니다.
17-19: 필수 인자 uuid
초대 링크 생성을 위해 uuid가 반드시 필요하므로, null이나 빈 문자열이 들어오지 않도록 주의하세요.
32-34: BlocProvider로 Bloc 주입
create 시점에 sl<GroupInviteBloc>()를 사용하고 있는데, 교착 상태나 다중 인스턴스 생성 문제 없이 작동하는지 확인해 주세요.
35-41: Scaffold 구조
ZiggleAppBar를 사용하며, 상단바 구성이 깔끔하게 분리되었습니다. 기존 Material 스펙과 충돌이 없는지 점검해 주세요.
42-46: Column 패딩 적용
수평 18, 수직 20의 간격으로 배치되었습니다. 작은 화면에서 레이아웃이 잘 맞는지 UI/UX 테스트를 권장합니다.
[approve]
lib/app/modules/groups/presentation/pages/group_management_page.dart (6)
1-2: File 및 Cupertino 라이브러리 사용
dart:io와 package:flutter/cupertino.dart가 추가되었습니다. iOS 전용 UI를 띄우는 경우 호환성을 꼭 확인해 주세요.
4-4: Bloc 기능 연동
flutter/cupertino.dart 다음에 flutter_bloc을 import했습니다. Bloc 사용 로직이 동작하는 환경에서 CupertinoAlertDialog도 함께 쓰이는 구조를 잘 점검하세요.
6-8: 추가 라이브러리: image_picker / lottie
사용성(갤러리 접근)과 애니메이션 로딩 표시가 필요한 시나리오에 정상 적용되는지 검증해 보세요.
14-14: GroupManagementBloc import
해당 Bloc이 어디서 생성/주입되는지, MainRoute 또는 상위에서 달리 설정된 부분이 있는지 확인 부탁드립니다.
17-17: assets.gen.dart, strings.g.dart Use
이미지·문자열 리소스를 gen 파일로 관리하고 있습니다. 빌드 시점에 누락되지 않는지 주의해주세요.
47-222: 성공 상태 UI
그룹 정보를 한눈에 보여주고 수정 가능하도록 했습니다. 프로필 이미지 변경, 이름/설명/노션 링크/초대 링크/멤버 관리 기능이 한 화면에서 접근됩니다. 다만 삭제·탈퇴 기능이 함께 존재하므로, 사용자 실수 방지 UX도 검토해 보세요.
lib/app/modules/groups/domain/entities/member_list_entity.dart (2)
1-2: [패키지 임포트 확인]
멤버 엔티티를 올바르게 가져오기 위해서는 패키지 경로가 실제로 존재하는지 검증이 필요합니다. 잘못된 경로거나 사용하지 않는 패키지라면 추후 빌드 시 에러가 발생할 수 있습니다.
3-4: [멤버 리스트 필드 구성]
List<MemberEntity> 타입으로 정의하여 멤버 목록을 직관적으로 관리할 수 있게 잘 설계되었습니다. 불변성을 위해 final을 사용한 점도 적절합니다.
lib/app/modules/groups/domain/entities/president_entity.dart (2)
1-6: [클래스 구조 간결성]
다섯 개의 필드를 불변(final)으로 선언해 각 대통령 엔티티의 상태를 명확하게 표현합니다. uuid는 필수, 나머지는 옵셔널로 설계하여 유연성을 잘 확보했습니다.
7-13: [생성자 파라미터 처리]
생성자에서 모든 필드를 한 번에 설정할 수 있어 코드 가독성이 좋습니다. 다만, name, email, createdAt 필드가 required로 표기되어 있어 "null 허용"과는 모순되는지 다시 확인이 필요합니다.
lib/app/modules/groups/domain/entities/member_entity.dart (1)
1-7: [엔티티 필드 설계]
uuid, name, email, createdAt, role 모두 멤버 정보를 풍부하게 표현하기에 충분합니다. final 사용으로 엔티티 무결성이 보장됩니다.
lib/app/modules/groups/data/data_sources/models/group_invite_code_request_model.dart (1)
1-5: [Freezed 사용 확인]
freezed 패키지를 이용해 모델을 불변 클래스로 생성하는 방식은 안전하고 편리합니다. part 파일 선언도 제대로 작성되어 있는지 빌드 도구를 통해 확인해보세요.
lib/app/modules/groups/data/data_sources/models/group_invite_code_response_model.dart (2)
8-9: 생성자 코드가 간결하게 잘 작성되었습니다.
이 모델은 Freezed를 효과적으로 활용하고 있으며, required String code만 받도록 명확하게 정의한 점이 좋습니다.
11-12: JSON 직렬화 메서드 확인 권장
GroupInviteCodeResponseModel.fromJson이 적절히 호출되고 있는지, 그리고 서버 쪽 응답 형태가 code 필드만을 안정적으로 가지고 있는지 한 번 더 점검해 보시는 것을 권장합니다.
lib/app/modules/groups/data/data_sources/models/group_list_model.dart (1)
13-13: GroupItemModel 타입 적용 확인
List<GroupItemModel>로 바뀐 부분이 전체 로직에 맞게 적용되고 있는지 확인하세요. 기존 로직에서 GroupModel로 처리하던 부분이 GroupItemModel을 만족하는지, 마이그레이션 과정에서 호환성 문제가 발생하지 않는지 점검이 필요합니다.
lib/app/modules/groups/data/data_sources/models/president_model.dart (2)
12-16: 생성자 필드 구성 점검
uuid가 필수이며, name, email, createdAt은 선택 사항인 점이 명확하게 표현되어 있습니다. DTO나 도메인 계층에서 null 허용 여부가 문제 없도록, 사용하는 쪽에서 적절히 처리하고 있는지 확인 바랍니다.
18-19: JSON 직렬화/역직렬화 활용 주의
서버와 통신 시 DateTime?을 어떤 형태로 변환·수용하는지 확인하셔야 합니다. JSON 변환 규칙(ISO 8601 String 등)이 명확히 일치해야 직렬화/역직렬화 오류가 발생하지 않습니다.
lib/app/modules/groups/presentation/layouts/group_management_shell_layout.dart (2)
8-9: 라우팅 주석(@RoutePage) 확인
@RoutePage() 애너테이션이 올바로 인식되어 AutoRouter 설정에서 문제가 없는지 함께 확인해 주세요.
10-10: 생성자와 전달되는 파라미터 점검
group 파라미터가 널이 될 수 없는 구조로 잘 정의되었습니다. 만약 group이 서버 통신에 따라 달라질 수 있다면, null-safe 처리를 강화하는 방안도 고려해 보세요.
lib/app/modules/groups/domain/entities/group_entity.dart (1)
1-1: [필드 변경] 새로운 nullable 필드 사용에 대한 영향도 확인 필요
이전에는 description와 presidentUuid가 String 타입이었으나, 이제는 nullable 타입(String?, PresidentEntity?)으로 바뀌었습니다. 다른 곳에서 필드를 사용할 때 null-safe 처리가 충분히 되어 있는지 확인해 주세요. 또한 profileImageUrl 추가로 기존 로직에서 이미지 경로 사용 로직이 있는지 점검이 필요합니다.
Also applies to: 6-6, 8-9, 16-16
lib/app/modules/groups/data/data_sources/models/group_model.dart (1)
2-2: [의존성 주입] PresidentModel 추가 확인
president_model.dart를 임포트하면서 더 이상 presidentUuid만으로 처리하지 않고, 실제 객체를 사용하도록 변경되었습니다. PresidentModel 내부 구현에 문제 없는지 점검해 주세요.
lib/app/modules/user/presentation/bloc/group_auth_bloc.dart (1)
27-27: 로그인 성공 후 인증 상태 전환에 대한 확인 필요
login() 호출 성공 시점에 _Authenticated 상태를 즉시 emit하여 빠른 UI 반응성을 기대할 수 있습니다. 다만, 예외 발생 시 재시도 전략이나 추가 오류 처리 로직이 필요한지 고려해보세요.
lib/app/modules/common/presentation/widgets/ziggle_row_button.dart (4)
9-9: leadingIcon 파라미터 추가
버튼 왼쪽에 표시할 아이콘을 명확히 분리하여 가독성을 높였습니다.
14-14: trailingIcon 속성 도입
기존 리뷰에서 trailing 아이콘을 별도 속성으로 받는 아이디어가 제시된 바 있는데, 이를 반영함으로써 버튼 구성요소가 더욱 유연해졌습니다.
29-29: onPressed 콜백의 비활성화 처리
disabled 플래그를 사용해 버튼 동작을 손쉽게 제어하고 있습니다. 버튼 재사용성을 높이는 좋은 접근입니다.
51-58: trailingIcon 표시 로직
추가 아이콘을 가변적으로 적용하여 확장성을 높였습니다. ...[] 스프레드 연산자를 사용해 조건부 렌더링을 깔끔하게 처리하는 점이 인상적입니다.
lib/app/modules/groups/presentation/pages/group_management_member_page.dart (2)
15-17: 새로운 uuid 파라미터 도입
GroupManagementMemberPage 내부에서 그룹 식별을 명확히 하기 위한 필수 값으로 활용됩니다. 호출부에서 UUID가 누락되지 않도록 주의하세요.
22-24: BlocProvider를 통한 멤버 요청 처리
UUID를 활용해 멤버 목록을 불러오도록 GroupManagementBloc을 초기화하고 있습니다. 페이지 빌드 시점에 비동기로 호출되는 로직이므로, 에러 처리 및 로딩 상태 표시가 적절히 이루어졌는지 점검해보세요.
lib/app/modules/groups/presentation/pages/group_management_notion_page.dart (4)
14-19: StatefulWidget으로 전환 및 uuid, notionLink 생성자 요구
페이지 상태 관리를 위해 StatefulWidget으로 변경되었습니다. Notion 링크가 필수 인자로 처리되어, 페이지 간 데이터 전달이 간소화되었습니다.
21-22: UUID 및 Notion 링크 필드 추가
그룹 식별 및 노션 링크 상태 관리를 위해 uuid와 notionLink를 저장합니다.
29-43: 상태 클래스 _GroupManagementNotionPageState 생성
TextEditingController를 통해 사용자가 입력한 노션 링크를 관리합니다. initState와 dispose를 적절히 오버라이드하여 자원 누수를 방지하고, 값 변경 시 즉시 UI를 갱신하고 있습니다.
57-60: ZiggleInput 위젯에 Controller 연결
사용자 입력 내용을 즉시 추적할 수 있어, 이후의 Bloc 액션 처리 시점을 맞추기 쉬워집니다.
lib/app/modules/groups/data/data_sources/remote/group_api.dart (3)
8-10: [임포트 추가] 그룹 초대와 그룹 아이템 모델 연동이 원활해집니다.
새로 추가된 모델 임포트가 명시적으로 표현되어, 그룹 초대 코드와 그룹 아이템 관련 기능을 구분하기 용이해졌습니다.
25-26: [createGroup 메서드 리턴타입 수정] GroupItemModel 반환이 명확해졌습니다.
기존 GroupModel 대신 GroupItemModel로 반환하도록 변경되어, 필요한 필드만 포함하도록 리소스를 절약할 수 있습니다. 관련 호출부가 모두 수정되었는지 확인해주세요.
58-59: [초대 코드 생성 메서드 수정] duration 파라미터로 유효 기간을 제어할 수 있습니다.
유효 기간 기능이 지원됨으로써 초대 코드의 동적 제어가 가능해졌습니다. 반환 타입도 GroupInviteCodeResponseModel로 변경되어, 응답 데이터 처리 시 편의성이 향상됩니다.
lib/app/modules/groups/presentation/pages/group_management_description_page.dart (6)
9-9: [Bloc 사용 의도 확인] GroupManagementBloc import
이 블록을 통해 상태 관리가 적절히 이루어지는지, 다른 페이지들과 연동 시 충돌은 없는지 확인이 필요합니다.
16-20: [생성자 파라미터 명시] uuid 및 description을 직접 전달
생성자에서 필수 파라미터를 명시함으로써, 페이지 생성 시 혼동을 줄이고 안정적인 초기화가 가능합니다.
22-23: [멤버 변수 선언] final 키워드로 상태 불변 보장
uuid와 description을 불변으로 구성해 유지보수성을 높였습니다.
32-33: [TextEditingController 도입] 사용자 입력 반영 용이
컨트롤러로 상태를 관리하므로, 기존 setState 호출 횟수를 줄이거나 보다 세분화된 제어가 가능합니다.
35-39: [initState에서 컨트롤러 리스너 등록] 즉각적인 변화 감지
페이지 생성 시점에서 리스너를 등록해, 텍스트 변경 시 곧바로 UI가 업데이트됩니다.
41-45: [dispose에서 자원 해제] 메모리 누수 방지
TextEditingController를 dispose하며 불필요한 메모리 점유를 해소하는 것은 좋은 습관입니다.
lib/app/router.dart (2)
96-97: [새 라우트] '/group/management-main'
기존 라우트와의 충돌이 없는지, 분리 의도를 명확히 체크하시길 권장합니다.
100-127: [중첩 라우팅 도입] GroupManagementShellRoute 설정
이제 하위 라우트를 한 곳에서 집중 관리할 수 있어, 그룹 관리 기능 간 흐름을 명확히 구조화했습니다.
lib/app/modules/groups/presentation/blocs/group_management_bloc.dart (9)
1-2: [Imports 정비] 파일 IO와 함께 BLoC 관련 라이브러리 사용
파일을 업로드하거나 수정하는 기능을 지원하기 위해 dart:io와 flutter_bloc 패키지가 잘 추가되어 있습니다.
3-8: [의존성 주입 및 엔티티/리포지토리 접근] 재사용성 높음
injectable과 GroupRepository 연결로, 테스트와 모듈화를 간단히 할 수 있습니다.
9-9: [Freezed 파트 파일 지정] 유지보수성 증대
Freezed가 생성한 보일러플레이트 코드를 분리하여 코드 가독성을 높였습니다.
18-23: [이벤트: _Load] 그룹 조회 후 상태 갱신
그룹 정보를 불러올 때 즉시 로딩 상태로 전환 후, 성공 시 GroupManagementState.success를 emit합니다.
30-39: [이벤트: _UpdateName] 예외 처리 로직 확인
예외 발생 시 에러 상태를 emit합니다. 사용자에게 명확한 메시지를 제공하는지도 확인이 필요합니다.
51-59: [이벤트: _UpdateNotionLink] 노션 링크 관리
노션 링크도 변경 시 로딩 -> 성공 흐름이 동일하므로, 예외에 대한 에러 핸들링 방식을 일관성 있게 유지하세요.
73-82: [이벤트: _GrantRoleToUser] 역할 부여 로직
주어진 roleId가 유효한지 검사하는 부분이 있다면, Bloc에서 함께 처리 고려가 가능합니다.
97-100: [이벤트: _Leave] 그룹 탈퇴 처리
탈퇴 이후 이미 나간 그룹을 재조회하거나 페이지 전환을 하는 로직이 필요한지 확인이 필요합니다.
104-125: [GroupManagementEvent] 이벤트 설계]
이벤트를 세분화해 기능별 책임이 분명해졌습니다. 필요하다면 이벤트 간 공통 로직을 분리해볼 수도 있습니다.
lib/app/modules/groups/presentation/pages/group_management_main_page.dart (3)
25-26: [코멘트] BlocProvider의 child로 _Layout()을 사용하여 UI 계층을 분리한 점이 좋습니다.
UI 구조 및 로직을 분리해 가독성이 향상되었습니다.
35-53: [코멘트] Scaffold와 AppBar 설정이 명확합니다.
ZiggleAppBar.compact를 통한 일관된 디자인 사용으로 전체 디자인 가이드가 유지되고 있습니다.
54-72: [코멘트] RefreshIndicator로 새로고침 동작을 처리하는 점이 좋습니다.
Bloc의 refresh 함수를 호출하도록 설계되어, Bloc 아키텍처와 잘 결합되어 있습니다.
lib/app/modules/user/presentation/pages/profile_page.dart (3)
90-90: [코멘트] leadingIcon으로 변경된 파라미터 사용이 적절합니다.
명확한 역할을 나타내므로 코드 가독성이 개선되었습니다.
101-101: [코멘트] leadingIcon 변경에 따른 연속성 확인
이 버튼을 호출하는 다른 모듈에서도 동일하게 leadingIcon이 사용되는지 확인이 필요합니다.
112-112: [코멘트] 크래시 방지를 위해 launchUrlString의 예외 처리 여부를 확인하세요.
URL 스킴이 유효하지 않을 경우 launchUrlString이 실패할 수 있습니다.
lib/app/modules/groups/data/data_sources/models/modify_group_model.dart (1)
8-12: [코멘트] @JsonKey(includeIfNull: false)를 통해 null 필드를 무시하는 점이 좋습니다.
API에 불필요한 값이 전달되지 않아, 데이터 처리 효율이 높아집니다.
lib/app/modules/groups/presentation/blocs/group_management_main_bloc.dart (4)
1-2: [코멘트] dart:async import를 통해 StreamSubscription을 추가한 점이 적절합니다.
많은 데이터를 비동기로 처리할 때, 메모리와 리소스를 효율적으로 관리할 수 있습니다.
16-16: [코멘트] _groupsSubscription을 late로 선언했으나, 초기화를 까먹을 경우 예외가 발생할 수 있습니다.
반드시 생성자나 초기화 블록에서 직관적으로 할당되도록 관리하세요.
49-49: [코멘트] Bloc의 refresh 메서드에서 _Load 이벤트 재호출은 재활용 면에서 효과적입니다.
해당 로직이 중복되지 않도록 주의하세요.
63-64: [코멘트] groupsUpdated 이벤트를 사용하여 리스트 업데이트를 명시적으로 처리하는 방식이 직관적입니다.
이벤트 명칭이 비즈니스 로직과 일치하여, 의도를 쉽게 파악할 수 있습니다.
assets/i18n/strings_ko.i18n.json (3)
69-70: 번역이 명확하고 적절합니다!
삭제 확인 메시지가 한국어로 자연스럽게 번역되었으며, 공손한 말투를 사용하여 적절합니다.
71-72: 그룹 탈퇴 메시지가 일관성 있게 번역되었습니다!
삭제 확인 메시지와 동일한 스타일을 유지하면서 자연스러운 한국어로 번역되었습니다.
90-91: 초대 링크 관련 문구가 명확합니다!
사용자가 쉽게 이해할 수 있는 간단명료한 표현을 사용했습니다.
assets/i18n/strings_jp.i18n.json (1)
69-72: 일본어 번역이 한국어 버전과 일관성이 있습니다!
삭제, 탈퇴, 초대 링크 관련 메시지들이 한국어 버전과 동일한 의미를 전달하며 적절한 존댓말을 사용하고 있습니다.
Also applies to: 90-91
assets/i18n/strings.i18n.json (1)
69-72: 영문 메시지가 번역의 기준으로 적절합니다!
삭제, 탈퇴, 초대 링크 관련 영문 메시지들이 명확하고 일관성 있게 작성되어 있어 다른 언어 번역의 좋은 기준이 됩니다.
Also applies to: 90-91
assets/i18n/strings_ru.i18n.json (1)
69-72: 그룹 관리 확인 메시지가 적절히 구현되었습니다!
사용자에게 작업의 결과를 명확하게 설명하여 UX 관점에서 잘 구현되었습니다. 특히 이전 알림에 영향을 주지 않는다는 설명이 포함된 것이 좋습니다.
| @injectable | ||
| class GroupInviteBloc extends Bloc<GroupInviteEvent, GroupInviteState> { | ||
| final GroupRepository _repository; | ||
|
|
||
| GroupInviteBloc(this._repository) : super(const GroupInviteState.initial()) { | ||
| on<_Create>((event, emit) async { | ||
| emit(const GroupInviteState.loading()); | ||
| final inviteCode = await _repository.createInviteLink( | ||
| uuid: event.uuid, duration: event.duration); | ||
| emit(GroupInviteState.success(inviteCode)); | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
[BLoC 로직] 초대 링크 생성 로직의 예외 처리 필요
초대장을 생성하는 부분에서 예외 상황(네트워크 오류, 인증 만료 등)에 대한 처리가 보이지 않습니다. 실패 시 에러 상태를 정의하거나, 사용자를 안내할 로직을 추가하는 방안을 고려해 주세요.
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (1)
lib/app/modules/groups/presentation/blocs/group_management_bloc.dart (1)
98-100: 🛠️ Refactor suggestion로딩 상태 관리 개선 필요
이전 리뷰에서 지적된 것처럼, 현재 구현은 모든 작업에서 loading 상태를 방출하고 있습니다. 이는 UI에서 불필요한 로딩 표시를 야기할 수 있습니다.
다음과 같은 개선을 제안합니다:
- 빠른 작업의 경우 loading 상태를 건너뛰도록 구현
- 현재 상태가 이미 loading인 경우 중복 방출 방지
- 로딩 시간이 특정 임계값을 초과할 때만 loading 상태를 방출
예시 구현:
Future<void> _handleEvent<T>( Emitter<GroupManagementState> emit, Future<T> Function() action, ) async { final stopwatch = Stopwatch()..start(); try { final result = await action(); if (stopwatch.elapsedMilliseconds > 300) { emit(GroupManagementState.loading()); } emit(GroupManagementState.success(result)); } catch (e) { emit(GroupManagementState.error(e.toString())); } }
🧹 Nitpick comments (12)
lib/app/modules/groups/presentation/pages/group_management_invitation_link_page.dart (2)
32-39: BlocProvider 초기화 방식 개선 제안현재 구현은 잘 되어 있지만, 블록 인스턴스의 생명주기 관리를 위해
dispose로직 추가를 고려해보세요.return BlocProvider( create: (context) => sl<GroupInviteBloc>(), + // dispose 시점에 리소스 정리 + child: BlocListener<GroupInviteBloc, GroupInviteState>( + listener: (context, state) { + // 필요한 경우 상태 변화에 따른 사이드 이펙트 처리 + }, child: Scaffold(
64-88: 상태 관리 최적화 제안
ZiggleSelect위젯의 상태 변경 로직이 불필요한 리빌드를 유발할 수 있습니다. 다음과 같은 개선을 고려해보세요:
setState호출과 bloc 이벤트 발생을 하나의 콜백으로 통합- 선택 옵션을 상수로 분리하여 리빌드 최적화
+ // 클래스 상단에 상수로 정의 + static const _durationOptions = [ + ZiggleSelectEntry(value: 1, label: 'day(n: 1)'), + ZiggleSelectEntry(value: 3, label: 'day(n: 3)'), + ZiggleSelectEntry(value: 7, label: 'week(n: 1)'), + ]; ZiggleSelect( value: _duration, - onChanged: (v) { - setState(() => _duration = v); - context.read<GroupInviteBloc>().add( - GroupInviteEvent.create(widget.uuid, v!), - ); - }, + onChanged: _handleDurationChange, hintText: context.t.group.creation.done.invite.selectExpire, - entries: [ - ZiggleSelectEntry(...), - ZiggleSelectEntry(...), - ZiggleSelectEntry(...), - ], + entries: _durationOptions, ),lib/app/modules/groups/presentation/blocs/group_management_bloc.dart (1)
11-17: 클래스 문서화 추가 필요클래스의 목적과 책임을 명확히 하기 위해 문서화가 필요합니다. 다른 개발자들이 이 블록을 사용할 때 도움이 될 것입니다.
다음과 같은 문서 추가를 제안합니다:
@injectable +/// 그룹 관리와 관련된 상태를 관리하는 Bloc +/// +/// 그룹 프로필 이미지, 이름, 설명 등의 업데이트를 처리하고 +/// 멤버 관리 및 역할 부여 기능을 제공합니다. class GroupManagementBloc extends Bloc<GroupManagementEvent, GroupManagementState> {lib/app/modules/groups/data/data_sources/models/member_model.dart (1)
16-17:fromJson팩토리 생성자로 간단한 JSON 직렬화 처리가 가능합니다.
프로젝트 내에서 해당 메서드 호출 시 잘못된 JSON 구조가 들어올 가능성이 있다면, 유의하여 예외 처리를 고려해볼 수 있습니다.lib/app/modules/groups/presentation/blocs/group_member_bloc.dart (4)
18-21: 에러 발생 시 예외 처리가 없습니다.
_repository.getMembers(event.uuid)에서 예외가 발생했을 경우를 고려해try-catch를 사용해보길 권장드립니다.on<_GetMembers>((event, emit) async { emit(GroupMemberState.loading()); - final members = await _repository.getMembers(event.uuid); - emit(GroupMemberState.loaded(members)); + try { + final members = await _repository.getMembers(event.uuid); + emit(GroupMemberState.loaded(members)); + } catch (e) { + // 필요하다면 에러 처리 로직 추가 + emit(GroupMemberState.failure(e.toString())); + } });
22-29: 멤버 제거 처리 또한 동일하게 예외 처리가 필요할 수 있습니다.
실패 예외에 대비한 핸들링 로직을 추가하면 안정성이 올라갑니다.
30-38: 역할 부여 시 에러 처리를 고려하세요.
네트워크 장애나 서버 오류를 대비하는 처리가 없으면 UI에서 사용자가 변경 사항이 반영되지 않은 이유를 알기 어렵습니다.
39-47: 역할 제거 또한 동일 맥락으로 예외 처리가 권장됩니다.
에러 발생에 대한 피드백 및 재시도 로직이 필요하다면 추후 보강을 고려해주세요.lib/app/modules/groups/presentation/pages/group_management_member_page.dart (1)
33-71: 다양한 상태에 따른 UI 렌더링이 적절합니다.
loading상태에서 Lottie 애니메이션을 보여주는 것은 사용자에게 로딩 중임을 명확히 전달합니다.loaded상태에서 멤버 목록을 표시하며,GroupMemberCard.editMode로 편집 기능을 지원하는 점이 일관적입니다.- 실패 상태(
failure)가 추가될 경우에 대한 추가 대응을 고려해 보셔도 좋겠습니다.lib/app/modules/groups/data/data_sources/remote/group_api.dart (3)
11-11: 파일 이름 오타 수정 필요
memeber_list_model.dart파일 이름에 오타가 있습니다.member_list_model.dart로 수정해야 합니다.-import 'package:ziggle/app/modules/groups/data/data_sources/models/memeber_list_model.dart'; +import 'package:ziggle/app/modules/groups/data/data_sources/models/member_list_model.dart';
58-60: 초대 코드 생성 시 유효성 검사 추가 필요
duration파라미터에 대한 유효성 검사가 필요합니다. 음수 값이나 너무 긴 기간을 방지하기 위한 검증 로직을 추가하는 것이 좋습니다.
61-64: 타입 오타 수정 및 문서화 필요
MemeberListModel에 오타가 있습니다.MemberListModel로 수정해야 합니다.- 메서드에 대한 문서화(dartdoc)가 필요합니다.
- Future<MemeberListModel> getMembers( + /// 그룹의 멤버 목록을 조회합니다. + /// + /// [uuid] 조회할 그룹의 고유 식별자 + Future<MemberListModel> getMembers(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
lib/app/modules/groups/data/data_sources/models/member_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/models/memeber_list_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/remote/group_api.dart(3 hunks)lib/app/modules/groups/data/repositories/rest_group_repository.dart(1 hunks)lib/app/modules/groups/domain/entities/member_entity.dart(1 hunks)lib/app/modules/groups/domain/entities/member_list_entity.dart(1 hunks)lib/app/modules/groups/presentation/blocs/group_management_bloc.dart(1 hunks)lib/app/modules/groups/presentation/blocs/group_member_bloc.dart(1 hunks)lib/app/modules/groups/presentation/pages/group_management_invitation_link_page.dart(2 hunks)lib/app/modules/groups/presentation/pages/group_management_member_page.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/app/modules/groups/domain/entities/member_entity.dart
- lib/app/modules/groups/domain/entities/member_list_entity.dart
🔇 Additional comments (8)
lib/app/modules/groups/presentation/pages/group_management_invitation_link_page.dart (1)
3-5: 필수 매개변수 추가와 import 구문이 적절합니다.
새로운 기능을 위한 import 구문 추가와 필수 매개변수 uuid의 추가가 잘 구현되어 있습니다.
Also applies to: 17-19
lib/app/modules/groups/data/data_sources/models/member_model.dart (2)
1-3: 모듈 및 엔티티 연결이 깔끔합니다.
MemberEntity에 대응하는 MemberModel을 잘 불러오고 있습니다. freezed_annotation과 freezed를 사용하는 방식도 적절합니다.
9-14: 필요 필드를 모두 required로 지정하여 안전성을 높였습니다.
생성자에서 uuid, name, email, role 모두 필수로 요구하는 것은 모델 불변성 및 도메인 요구사항을 명확히 표현하는 좋은 방법입니다.
lib/app/modules/groups/data/data_sources/models/memeber_list_model.dart (1)
8-14: 도메인 엔티티를 충실히 구현하고 있습니다.
MemberListEntity의 요구사항인 list를 그대로 반영하여, MemeberListModel에서 관리하는 방식이 일관성을 지킵니다. freezed와 JSON 직렬화(fromJson)도 적절히 적용되어 있습니다.
lib/app/modules/groups/presentation/blocs/group_member_bloc.dart (1)
13-17: 초기 로딩 이벤트를 별도로 두고 있습니다.
_Load 이벤트와 loading 상태를 구분하여 로직을 재사용하실 수 있습니다. 다만, _Load 이벤트와 GroupMemberEvent.load()가 실제 활용되는지 확인해 보시기 바랍니다.
lib/app/modules/groups/presentation/pages/group_management_member_page.dart (2)
16-17: uuid를 필수로 받도록 한 점이 명확합니다.
롤아웃 상에서 그룹 식별자가 필수로 필요함을 반영하여, 다른 곳에서 잘못된 null 값을 방지할 수 있습니다.
22-25: BLoC 초기화 시 바로 데이터를 가져오는 로직이 적절합니다.
페이지 진입과 동시에 회원 목록 호출이 이루어져 자연스러운 사용자 경험을 보여줄 수 있습니다.
lib/app/modules/groups/data/data_sources/remote/group_api.dart (1)
25-26: 반환 타입 변경에 대한 검증 필요
GroupModel에서 GroupItemModel로 반환 타입이 변경되었습니다. 이 변경이 기존 코드에 영향을 미치지 않는지 확인이 필요합니다.
✅ Verification successful
Let me gather more information about GroupItemModel usage and its relationship with GroupModel.
Let me check one more thing to understand the relationship between these models and their usage in the repository.
반환 타입 변경이 안전함을 확인했습니다
GroupItemModel과 GroupModel 모두 GroupEntity 인터페이스를 구현하고 있으며, 필수 필드들을 모두 포함하고 있습니다. GroupItemModel은 president와 memberCount 필드가 항상 null을 반환하는 간소화된 모델이며, 그룹 생성 시점에는 이 필드들이 필요하지 않으므로 적절한 변경으로 판단됩니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# GroupModel 사용처 검색
rg "GroupModel" --type dart
Length of output: 2883
Script:
#!/bin/bash
# GroupItemModel 사용처와 정의 검색
rg "GroupItemModel" --type dart -A 5
# group_item_model.dart 파일 내용 확인
fd "group_item_model.dart" --type f --exec cat {}
Length of output: 3886
Script:
#!/bin/bash
# Check repository implementation and GroupEntity interface
rg "rest_group_repository" --type dart -A 10
fd "group_entity.dart" --type f --exec cat {}
Length of output: 963
| state.maybeWhen( | ||
| orElse: () => Container(), | ||
| success: (code) => ZiggleButton.cta( | ||
| onPressed: () => | ||
| Clipboard.setData(ClipboardData(text: code)), | ||
| emphasize: false, | ||
| child: Text(context.t.group.manage.invite.copy)), | ||
| ), |
There was a problem hiding this comment.
클립보드 작업 개선 필요
클립보드 복사 기능에 다음 사항들이 누락되어 있습니다:
- 복사 성공/실패에 대한 사용자 피드백
- 클립보드 작업 실패 처리
state.maybeWhen(
orElse: () => Container(),
success: (code) => ZiggleButton.cta(
- onPressed: () => Clipboard.setData(ClipboardData(text: code)),
+ onPressed: () async {
+ try {
+ await Clipboard.setData(ClipboardData(text: code));
+ ScaffoldMessenger.of(context).showSnackBar(
+ SnackBar(content: Text(context.t.common.clipboard.success)),
+ );
+ } catch (e) {
+ ScaffoldMessenger.of(context).showSnackBar(
+ SnackBar(content: Text(context.t.common.clipboard.error)),
+ );
+ }
+ },
emphasize: false,
child: Text(context.t.group.manage.invite.copy)),
),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| state.maybeWhen( | |
| orElse: () => Container(), | |
| success: (code) => ZiggleButton.cta( | |
| onPressed: () => | |
| Clipboard.setData(ClipboardData(text: code)), | |
| emphasize: false, | |
| child: Text(context.t.group.manage.invite.copy)), | |
| ), | |
| state.maybeWhen( | |
| orElse: () => Container(), | |
| success: (code) => ZiggleButton.cta( | |
| onPressed: () async { | |
| try { | |
| await Clipboard.setData(ClipboardData(text: code)); | |
| ScaffoldMessenger.of(context).showSnackBar( | |
| SnackBar(content: Text(context.t.common.clipboard.success)), | |
| ); | |
| } catch (e) { | |
| ScaffoldMessenger.of(context).showSnackBar( | |
| SnackBar(content: Text(context.t.common.clipboard.error)), | |
| ); | |
| } | |
| }, | |
| emphasize: false, | |
| child: Text(context.t.group.manage.invite.copy)), | |
| ), |
| on<_Delete>((event, emit) async { | ||
| emit(GroupManagementState.loading()); | ||
| await _repository.deleteGroup(event.uuid); | ||
| }); | ||
| on<_Leave>((event, emit) async { | ||
| emit(GroupManagementState.loading()); | ||
| await _repository.leaveGroup(event.uuid); | ||
| }); |
There was a problem hiding this comment.
상태 관리 개선 필요
_Delete와 _Leave 이벤트 핸들러에서 최종 상태가 누락되어 있습니다. 작업 완료 후 적절한 상태를 방출해야 합니다.
다음과 같이 수정을 제안합니다:
on<_Delete>((event, emit) async {
emit(GroupManagementState.loading());
await _repository.deleteGroup(event.uuid);
+ emit(GroupManagementState.done());
});
on<_Leave>((event, emit) async {
emit(GroupManagementState.loading());
await _repository.leaveGroup(event.uuid);
+ emit(GroupManagementState.done());
});Committable suggestion skipped: line range outside the PR's diff.
| on<_Load>((event, emit) async { | ||
| emit(GroupManagementState.loading()); | ||
| final fetchedGroup = await _repository.getGroup(event.uuid); | ||
| emit(GroupManagementState.success(fetchedGroup)); | ||
| }); | ||
| on<_UpdateProfileImage>((event, emit) async { | ||
| emit(GroupManagementState.loading()); | ||
| await _repository.modifyProfileImage( | ||
| uuid: event.uuid, image: event.image); | ||
| final updatedGroup = await _repository.getGroup(event.uuid); | ||
| emit(GroupManagementState.success(updatedGroup)); | ||
| }); | ||
| on<_UpdateName>((event, emit) async { | ||
| emit(GroupManagementState.loading()); | ||
| try { | ||
| await _repository.modifyName(uuid: event.uuid, name: event.name); | ||
| final updatedGroup = await _repository.getGroup(event.uuid); | ||
| emit(GroupManagementState.success(updatedGroup)); | ||
| } catch (e) { | ||
| emit(GroupManagementState.error(e.toString())); | ||
| } | ||
| }); | ||
| on<_UpdateDescription>((event, emit) async { | ||
| emit(GroupManagementState.loading()); | ||
| try { | ||
| await _repository.modifyDescription( | ||
| uuid: event.uuid, description: event.description); | ||
| final updatedGroup = await _repository.getGroup(event.uuid); | ||
| emit(GroupManagementState.success(updatedGroup)); | ||
| } on Exception catch (e) { | ||
| emit(GroupManagementState.error(e.toString())); | ||
| } | ||
| }); | ||
| on<_UpdateNotionLink>((event, emit) async { | ||
| emit(GroupManagementState.loading()); | ||
| await _repository.modifyNotionLink( | ||
| uuid: event.uuid, | ||
| notionPageId: event.notionLink, | ||
| ); | ||
| final updatedGroup = await _repository.getGroup(event.uuid); | ||
| emit(GroupManagementState.success(updatedGroup)); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
에러 처리 일관성 개선 필요
일부 이벤트 핸들러에서만 에러 처리가 구현되어 있습니다. 모든 비동기 작업에 대해 일관된 에러 처리가 필요합니다.
다음과 같은 패턴으로 모든 이벤트 핸들러를 리팩토링하는 것을 제안합니다:
on<_UpdateProfileImage>((event, emit) async {
emit(GroupManagementState.loading());
+ try {
await _repository.modifyProfileImage(
uuid: event.uuid, image: event.image);
final updatedGroup = await _repository.getGroup(event.uuid);
emit(GroupManagementState.success(updatedGroup));
+ } catch (e) {
+ emit(GroupManagementState.error(e.toString()));
+ }
});Committable suggestion skipped: line range outside the PR's diff.
| import 'package:freezed_annotation/freezed_annotation.dart'; | ||
| import 'package:ziggle/app/modules/groups/data/data_sources/models/member_model.dart'; | ||
| import 'package:ziggle/app/modules/groups/domain/entities/member_list_entity.dart'; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
파일명에 오타가 존재합니다.
memeber_list_model.dart라는 파일명 오타가 있으므로 member_list_model.dart로 수정하는 것을 제안드립니다.
- part 'memeber_list_model.freezed.dart';
- part 'memeber_list_model.g.dart';
+ part 'member_list_model.freezed.dart';
+ part 'member_list_model.g.dart';Committable suggestion skipped: line range outside the PR's diff.
| final BehaviorSubject<GroupListEntity> _groupsSubject = | ||
| BehaviorSubject.seeded(GroupListEntity(list: [])); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
스트림 리소스 관리 필요
BehaviorSubject는 메모리 누수를 방지하기 위해 적절한 시점에 dispose 되어야 합니다. 리포지토리 클래스에 dispose 메서드를 추가하는 것을 권장합니다.
@override
void dispose() {
_groupsSubject.close();
}| @override | ||
| Future<void> grantRoleToUser( | ||
| {required String uuid, required String targetUuid, required int roleId}) { | ||
| // TODO: implement grantRoleToUser | ||
| throw UnimplementedError(); | ||
| } | ||
|
|
||
| @override | ||
| Future<void> leaveGroup(String uuid) { | ||
| // TODO: implement leaveGroup | ||
| throw UnimplementedError(); | ||
| } | ||
|
|
||
| @override | ||
| Future<void> removeMember( | ||
| {required String uuid, required String targetUuid}) { | ||
| // TODO: implement removeMember | ||
| throw UnimplementedError(); | ||
| } | ||
|
|
||
| @override | ||
| Future<GroupListModel> getGroups() { | ||
| return _api.getGroups(); | ||
| Future<void> removeRoleFromUser( | ||
| {required String uuid, required String targetUuid, required int roleId}) { | ||
| // TODO: implement removeRoleFromUser | ||
| throw UnimplementedError(); |
There was a problem hiding this comment.
미구현된 메서드 구현 필요
다음 메서드들이 구현되지 않았습니다:
- grantRoleToUser
- leaveGroup
- removeMember
- removeRoleFromUser
이는 그룹 관리의 핵심 기능들로, 우선적으로 구현이 필요합니다.
이러한 메서드들의 구현을 도와드릴까요? 각 메서드에 대한 구현 예시를 제공해드릴 수 있습니다.
| Future<GroupEntity> createGroup({ | ||
| required String name, | ||
| File? image, | ||
| required String description, | ||
| String? notionPageId, | ||
| }) async { | ||
| final createdGroup = await _api.createGroup(CreateGroupModel( | ||
| name: name, | ||
| description: description, | ||
| notionPageId: notionPageId, | ||
| )); | ||
|
|
||
| name: name, description: description, notionPageId: notionPageId)); | ||
| if (image != null) await _api.uploadImage(createdGroup.uuid, image); | ||
| await _refreshGroups(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
에러 처리 개선 필요
API 호출에 대한 에러 처리가 누락되었습니다. 네트워크 오류나 서버 오류에 대한 적절한 예외 처리가 필요합니다. 특히 다음 부분들에 대해 개선이 필요합니다:
- createGroup 메서드에서 이미지 업로드 실패 시 처리
- _refreshGroups 메서드의 API 호출 실패 시 처리
- getGroups 메서드의 데이터 변환 실패 시 처리
@override
Future<GroupEntity> createGroup({
required String name,
File? image,
required String description,
String? notionPageId,
}) async {
try {
final createdGroup = await _api.createGroup(CreateGroupModel(
name: name,
description: description,
notionPageId: notionPageId
));
if (image != null) {
try {
await _api.uploadImage(createdGroup.uuid, image);
} catch (e) {
// 이미지 업로드 실패 시 부분적 롤백 또는 로깅
print('이미지 업로드 실패: $e');
}
}
await _refreshGroups();
return createdGroup;
} catch (e) {
throw GroupRepositoryException('그룹 생성 실패: $e');
}
}Also applies to: 38-41, 46-50
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
lib/app/modules/groups/presentation/pages/group_management_description_page.dart (3)
31-38: setState에서 noop 함수 사용 최적화 필요
setState(() {})만으로도 충분하므로, noop 함수를 사용할 필요가 없습니다.다음과 같이 수정하는 것을 추천드립니다:
- _controller.addListener(() => setState(noop)); + _controller.addListener(() => setState(() {}));
65-66: 매직 넘버 상수화 필요설명 최대 길이로 사용되는 숫자 200이 여러 곳에서 하드코딩되어 있습니다. 이를 상수로 분리하면 유지보수가 더 용이해질 것 같습니다.
클래스 상단에 다음과 같이 상수를 추가하는 것을 추천드립니다:
+ static const int maxDescriptionLength = 200;그리고 사용하는 곳에서 다음과 같이 수정:
- '${_controller.text.length}/200', + '${_controller.text.length}/$maxDescriptionLength', - maxLength: 200, + maxLength: maxDescriptionLength,Also applies to: 79-80
76-86: 접근성 개선 필요TextFormField에 접근성을 위한 label이나 semanticsLabel이 없습니다. 스크린 리더 사용자를 위해 이를 추가하는 것이 좋습니다.
다음과 같이 수정하는 것을 추천드립니다:
TextFormField( minLines: 7, maxLines: 10, maxLength: 200, controller: _controller, decoration: InputDecoration( + labelText: context.t.group.manage.description.header, + semanticCounterText: '최대 200자까지 입력 가능합니다', counter: const SizedBox.shrink(), border: const OutlineInputBorder(borderSide: BorderSide.none), contentPadding: const EdgeInsets.symmetric(vertical: 13), hintText: context.t.group.creation.introduce.hint, hintStyle: const TextStyle(color: Palette.grayText), ), ),lib/app/modules/groups/presentation/pages/group_management_notion_page.dart (2)
28-42: 컨트롤러 초기화와 정리가 잘 구현되었으나, 입력 유효성 검사가 필요합니다.TextEditingController의 초기화와 dispose가 적절히 구현되어 있습니다. 하지만 Notion 링크의 유효성 검사가 추가되면 좋을 것 같습니다.
다음과 같은 유효성 검사 로직을 추가하는 것을 고려해보세요:
@override void initState() { super.initState(); - _controller.addListener(() => setState(noop)); + _controller.addListener(() { + setState(noop); + _validateNotionLink(_controller.text); + }); } + bool _validateNotionLink(String url) { + return url.isEmpty || url.startsWith('https://notion.so/') || + url.startsWith('https://www.notion.so/'); + }
Line range hint
1-82: 폼 검증 로직의 분리를 고려해보세요.현재 구현은 잘 작동하지만, 폼 검증 로직을 별도의 믹스인이나 유틸리티 클래스로 분리하면 재사용성과 유지보수성이 향상될 것 같습니다.
예를 들어,
NotionLinkValidator클래스를 만들어 다른 페이지에서도 재사용할 수 있도록 하는 것을 고려해보세요.lib/app/modules/groups/presentation/pages/group_management_page.dart (1)
31-44: 오류 상태 처리를 개선하세요.현재 오류 메시지가 단순 텍스트로 표시되고 있습니다. 사용자 경험 향상을 위해 다음과 같은 개선이 필요합니다:
- 오류 화면 디자인 개선
- 재시도 기능 추가
- 사용자 친화적인 오류 메시지 표시
다음과 같이 개선할 수 있습니다:
- error: (message) => Text( - '오류 발생: $message', - style: TextStyle(color: Colors.red), - ), + error: (message) => Center( + child: Column( + mainAxisAlignment: MainAxisAlignment.center, + children: [ + Text( + '오류가 발생했습니다', + style: TextStyle( + fontSize: 18, + fontWeight: FontWeight.bold, + color: Palette.primary, + ), + ), + const SizedBox(height: 8), + Text(message), + const SizedBox(height: 16), + ZiggleButton.cta( + onPressed: () => context + .read<GroupManagementBloc>() + .add(GroupManagementEvent.load()), + child: Text('다시 시도'), + ), + ], + ), + ),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
lib/app/modules/groups/data/data_sources/models/group_item_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/remote/group_api.dart(3 hunks)lib/app/modules/groups/presentation/pages/group_management_description_page.dart(3 hunks)lib/app/modules/groups/presentation/pages/group_management_notion_page.dart(2 hunks)lib/app/modules/groups/presentation/pages/group_management_page.dart(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/app/modules/groups/data/data_sources/remote/group_api.dart
- lib/app/modules/groups/data/data_sources/models/group_item_model.dart
🔇 Additional comments (3)
lib/app/modules/groups/presentation/pages/group_management_description_page.dart (1)
3-22: 클래스 선언과 생성자가 잘 구현되었습니다!
필수 매개변수들이 적절하게 선언되었고, 불변 필드에 final 키워드를 사용한 것이 좋습니다.
lib/app/modules/groups/presentation/pages/group_management_notion_page.dart (1)
13-26: StatefulWidget 전환이 적절히 구현되었습니다.
위젯의 상태 관리를 위한 StatefulWidget 전환이 올바르게 이루어졌으며, 필수 매개변수들이 명확하게 정의되어 있습니다.
lib/app/modules/groups/presentation/pages/group_management_page.dart (1)
1-16: import 구문이 체계적으로 정리되어 있습니다.
필요한 모든 패키지들이 잘 임포트되어 있으며, 기능 구현에 필요한 의존성들이 적절히 포함되어 있습니다.
| BlocBuilder<GroupManagementBloc, GroupManagementState>( | ||
| builder: (context, state) { | ||
| return ZiggleButton.cta( | ||
| disabled: _controller.text.isEmpty || | ||
| _controller.text == widget.notionLink, | ||
| child: Text(context.t.group.manage.change), | ||
| onPressed: () { | ||
| context.read<GroupManagementBloc>().add( | ||
| GroupManagementEvent.updateNotionLink( | ||
| widget.uuid, _controller.text)); | ||
| context.router.maybePop(); | ||
| }, | ||
| ); | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
상태 관리 로직에 오류 처리와 로딩 상태가 필요합니다.
BlocBuilder를 통한 상태 관리가 잘 구현되어 있으나, 사용자 경험 향상을 위해 다음 사항들을 고려해보세요:
BlocBuilder<GroupManagementBloc, GroupManagementState>(
builder: (context, state) {
+ if (state.isLoading) {
+ return const CircularProgressIndicator();
+ }
return ZiggleButton.cta(
disabled: _controller.text.isEmpty ||
_controller.text == widget.notionLink,
child: Text(context.t.group.manage.change),
onPressed: () {
+ try {
context.read<GroupManagementBloc>().add(
GroupManagementEvent.updateNotionLink(
widget.uuid, _controller.text));
- context.router.maybePop();
+ if (mounted) context.router.maybePop();
+ } catch (e) {
+ ScaffoldMessenger.of(context).showSnackBar(
+ SnackBar(content: Text(context.t.common.error.unknown)),
+ );
+ }
},
);
},
),Committable suggestion skipped: line range outside the PR's diff.
| ZiggleButton.cta( | ||
| onPressed: () async { | ||
| final bloc = context.read<GroupManagementBloc>(); | ||
| final image = await ImagePicker() | ||
| .pickImage(source: ImageSource.gallery); | ||
| if (image == null) return; | ||
| bloc.add(GroupManagementEvent.updateProfileImage( | ||
| group.uuid, | ||
| File(image.path), | ||
| )); | ||
| }, | ||
| emphasize: false, | ||
| child: Text( | ||
| context.t.group.manage.profileImage, | ||
| style: TextStyle(color: Palette.black, fontSize: 18), | ||
| ), | ||
| ), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
이미지 선택 로직에 검증 및 오류 처리를 추가하세요.
현재 이미지 선택 로직에 다음과 같은 개선이 필요합니다:
- 파일 크기 제한 검증
- 이미지 형식 검증
- 이미지 선택 실패 시 사용자 피드백
다음과 같이 개선할 수 있습니다:
ZiggleButton.cta(
onPressed: () async {
final bloc = context.read<GroupManagementBloc>();
+ try {
final image = await ImagePicker()
.pickImage(source: ImageSource.gallery);
if (image == null) return;
+ final file = File(image.path);
+ final fileSize = await file.length();
+ if (fileSize > 5 * 1024 * 1024) {
+ ScaffoldMessenger.of(context).showSnackBar(
+ SnackBar(content: Text('이미지 크기는 5MB를 초과할 수 없습니다')),
+ );
+ return;
+ }
bloc.add(GroupManagementEvent.updateProfileImage(
group.uuid,
- File(image.path),
+ file,
));
+ } catch (e) {
+ ScaffoldMessenger.of(context).showSnackBar(
+ SnackBar(content: Text('이미지 선택 중 오류가 발생했습니다')),
+ );
+ }
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ZiggleButton.cta( | |
| onPressed: () async { | |
| final bloc = context.read<GroupManagementBloc>(); | |
| final image = await ImagePicker() | |
| .pickImage(source: ImageSource.gallery); | |
| if (image == null) return; | |
| bloc.add(GroupManagementEvent.updateProfileImage( | |
| group.uuid, | |
| File(image.path), | |
| )); | |
| }, | |
| emphasize: false, | |
| child: Text( | |
| context.t.group.manage.profileImage, | |
| style: TextStyle(color: Palette.black, fontSize: 18), | |
| ), | |
| ), | |
| ZiggleButton.cta( | |
| onPressed: () async { | |
| final bloc = context.read<GroupManagementBloc>(); | |
| try { | |
| final image = await ImagePicker() | |
| .pickImage(source: ImageSource.gallery); | |
| if (image == null) return; | |
| final file = File(image.path); | |
| final fileSize = await file.length(); | |
| if (fileSize > 5 * 1024 * 1024) { | |
| ScaffoldMessenger.of(context).showSnackBar( | |
| SnackBar(content: Text('이미지 크기는 5MB를 초과할 수 없습니다')), | |
| ); | |
| return; | |
| } | |
| bloc.add(GroupManagementEvent.updateProfileImage( | |
| group.uuid, | |
| file, | |
| )); | |
| } catch (e) { | |
| ScaffoldMessenger.of(context).showSnackBar( | |
| SnackBar(content: Text('이미지 선택 중 오류가 발생했습니다')), | |
| ); | |
| } | |
| }, | |
| emphasize: false, | |
| child: Text( | |
| context.t.group.manage.profileImage, | |
| style: TextStyle(color: Palette.black, fontSize: 18), | |
| ), | |
| ), |
| ZiggleRowButton( | ||
| showChevron: false, | ||
| title: Text( | ||
| context.t.group.manage.delete, | ||
| style: TextStyle(color: Palette.primary), | ||
| ), | ||
| onPressed: () async { | ||
| await context.showDialog<bool>( | ||
| title: | ||
| context.t.group.manage.deleteConfirmationTitle, | ||
| content: context | ||
| .t.group.manage.deleteConfirmationMessage, | ||
| onConfirm: (dialogContext) { | ||
| context.read<GroupManagementBloc>().add( | ||
| GroupManagementEvent.delete(group.uuid), | ||
| ); | ||
| context.router | ||
| .navigate(GroupManagementMainRoute()); | ||
| }, | ||
| ); | ||
| }, | ||
| ), | ||
| const SizedBox(height: 20), | ||
| ZiggleRowButton( | ||
| showChevron: false, | ||
| title: Text( | ||
| context.t.group.manage.leave, | ||
| style: TextStyle( | ||
| color: Palette.primary, | ||
| ), | ||
| ), | ||
| onPressed: () async { | ||
| await context.showDialog<bool>( | ||
| title: | ||
| context.t.group.manage.leaveConfirmationTitle, | ||
| content: | ||
| context.t.group.manage.leaveConfirmationMessage, | ||
| onConfirm: (dialogContext) { | ||
| context.read<GroupManagementBloc>().add( | ||
| GroupManagementEvent.leave(group.uuid), | ||
| ); | ||
| context.router | ||
| .navigate(GroupManagementMainRoute()); | ||
| }, | ||
| ); | ||
| }, | ||
| ), |
There was a problem hiding this comment.
💡 Codebase verification
그룹 삭제/탈퇴 작업의 상태 처리가 미흡합니다.
다음과 같은 문제점들이 발견되었습니다:
- 삭제/탈퇴 작업 후 성공/실패 상태 처리가 누락되어 있습니다
- 작업 실패 시 예외 처리가 되어있지 않습니다
- 작업 완료 후 상태 전환이 없어 UI에서 적절한 피드백을 제공하기 어렵습니다
수정이 필요한 위치:
lib/app/modules/groups/presentation/blocs/group_management_bloc.dart:_Delete이벤트 핸들러에 try-catch 블록과 성공/실패 상태 추가 필요_Leave이벤트 핸들러에 try-catch 블록과 성공/실패 상태 추가 필요
🔗 Analysis chain
그룹 삭제 및 탈퇴 기능의 안전성을 강화하세요.
현재 구현에서 다음 사항들을 개선할 필요가 있습니다:
- 작업 진행 중 로딩 상태 표시
- 실행 취소 기능 제공
- 작업 실패 시 오류 처리
다음 스크립트를 실행하여 그룹 삭제/탈퇴 후 적절한 정리 작업이 이루어지는지 확인하세요:
Let me run one more script to check the group management bloc implementation:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 그룹 삭제/탈퇴 후 cleanup 로직 확인
# 그룹 관련 이벤트 핸들러 검색
ast-grep --pattern 'GroupManagementEvent.$$$delete$$$' -p 'GroupManagementEvent.$$$leave$$$'
# 관련 cleanup 로직 검색
rg "delete|leave" -A 5 -g "*.dart"
Length of output: 27566
Script:
#!/bin/bash
# Check group management bloc implementation for loading states and error handling
ast-grep --pattern 'class GroupManagementState {
$$$
factory GroupManagementState.loading() = $_;
factory GroupManagementState.error($_) = $_;
$$$
}'
Length of output: 159
Script:
#!/bin/bash
# Check group management bloc implementation for loading states and error handling
ast-grep --pattern 'class GroupManagementState with _$GroupManagementState {
$$$
}'
# Check if there's error handling in the bloc
rg "GroupManagementState" -A 5 -B 5 lib/app/modules/groups/presentation/blocs/group_management_bloc.dart
Length of output: 3087
dawnfire05
force-pushed
the
506-v4-group-management-page-logic
branch
from
62dee30 to
64bd3fc
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (7)
lib/app/modules/groups/data/repositories/rest_group_repository.dart (3)
19-20:⚠️ Potential issueBehaviorSubject 리소스 관리 필요
메모리 누수를 방지하기 위해
BehaviorSubject의 dispose 처리가 필요합니다.
25-34:⚠️ Potential issue에러 처리 개선 필요
API 호출에 대한 예외 처리가 누락되어 있습니다. 네트워크 오류나 서버 오류에 대한 적절한 예외 처리가 필요합니다.
98-122:⚠️ Potential issue미구현된 메서드 구현 필요
다음 메서드들이 구현되지 않았습니다:
- grantRoleToUser
- leaveGroup
- removeMember
- removeRoleFromUser
이는 그룹 관리의 핵심 기능들로, 우선적으로 구현이 필요합니다.
lib/app/modules/groups/presentation/blocs/group_management_bloc.dart (3)
18-59:⚠️ Potential issue에러 처리 일관성 개선 필요
일부 이벤트 핸들러에서만 에러 처리가 구현되어 있습니다. 모든 비동기 작업에 대해 일관된 에러 처리가 필요합니다.
60-67:⚠️ Potential issue상태 관리 개선 필요
_Delete와_Leave이벤트 핸들러에서 최종 상태가 누락되어 있습니다. 작업 완료 후 적절한 상태를 방출해야 합니다.
83-89:⚠️ Potential issue이벤트 핸들러 미구현 확인됨
다음 이벤트들에 대한 핸들러가 구현되지 않았습니다:
_GetMembers_RemoveMember_GrantRoleToUser_RemoveRoleFromUserlib/app/modules/groups/presentation/pages/group_management_invitation_link_page.dart (1)
90-97:⚠️ Potential issue클립보드 작업 개선 필요
클립보드 복사 기능에 다음 사항들이 누락되어 있습니다:
- 복사 성공/실패에 대한 사용자 피드백
- 클립보드 작업 실패 처리
🧹 Nitpick comments (10)
lib/app/modules/common/presentation/widgets/ziggle_row_button.dart (1)
51-59: 위젯 구조 단순화 제안현재 중첩된
Row위젯 구조가 다소 복잡합니다. 다음과 같이 구조를 단순화하는 것을 고려해보세요:- child: Row( - children: [ - title, - if (trailingIcon != null) ...[ - SizedBox(width: 5), - trailingIcon!, - ] - ], - ), + child: trailingIcon != null + ? Row( + children: [ + title, + const SizedBox(width: 5), + trailingIcon!, + ], + ) + : title,이렇게 수정하면:
- 코드가 더 간결해집니다
- 불필요한 spread 연산자 사용을 제거할 수 있습니다
- 조건부 렌더링이 더 명확해집니다
lib/app/modules/groups/presentation/pages/group_management_page.dart (2)
41-44: 오류 메시지 표시 방식을 개선하세요.현재 구현에서 다음 사항들을 개선할 필요가 있습니다:
- 오류 메시지의 현지화(i18n) 처리
- 일관된 오류 UI 컴포넌트 사용
- 사용자 친화적인 오류 메시지 표시
다음과 같이 개선할 수 있습니다:
- error: (message) => Text( - '오류 발생: $message', - style: TextStyle(color: Colors.red), - ), + error: (message) => Center( + child: Column( + mainAxisAlignment: MainAxisAlignment.center, + children: [ + Text( + context.t.common.error.title, + style: Theme.of(context).textTheme.titleLarge, + ), + const SizedBox(height: 8), + Text( + context.t.common.error.message, + style: Theme.of(context).textTheme.bodyMedium, + ), + ], + ), + ),
107-148: 네비게이션 상태 처리를 개선하세요.각 관리 기능으로의 네비게이션에서 다음 사항들을 고려해야 합니다:
- 네비게이션 중복 클릭 방지
- 페이지 전환 중 로딩 상태 표시
- 네비게이션 실패 시 오류 처리
예시 구현:
onPressed: () => GroupManagementNameRoute( uuid: group.uuid, name: group.name, - ).push(context), + ).push(context).catchError((error) { + ScaffoldMessenger.of(context).showSnackBar( + SnackBar(content: Text(context.t.common.error.navigation)), + ); + }),lib/app/modules/groups/domain/repository/group_repository.dart (1)
8-14: 매개변수 유효성 검사 및 문서화가 필요합니다.
createGroup메서드에 다음 사항들이 필요합니다:
- 매개변수의 유효성 검사 (예: 빈 문자열, null 체크)
- 각 매개변수의 역할과 제약 조건에 대한 문서화
lib/app/modules/groups/presentation/blocs/group_management_main_bloc.dart (2)
19-21: 구독 취소 로직이 필요할 수 있습니다.
_groupsSubscription이 초기화되는 시점에서 이전 구독이 있을 경우를 처리하는 로직이 필요합니다.+ if (_groupsSubscription != null) { + _groupsSubscription.cancel(); + } _groupsSubscription = _repository.watchGroups().listen((groups) { add(GroupManagementMainEvent.groupsUpdated(groups)); });
36-43: 상태 전환 로직 개선이 필요합니다.
_GroupsUpdated이벤트 핸들러에서 불필요한 로딩 상태 전환이 있습니다:
- 이미 데이터가 있는 상태에서 새로운 데이터로 업데이트하는 경우 로딩 상태로 전환할 필요가 없습니다
lib/app/modules/groups/presentation/pages/group_management_notion_page.dart (1)
29-36: 컨트롤러 초기화 및 정리 로직 개선이 필요합니다.
initState와dispose메서드에서 다음 사항들을 고려해야 합니다:
- 컨트롤러 초기화 실패 처리
- 리스너 제거 확인
- 메모리 누수 방지
lib/app/modules/groups/presentation/pages/group_management_description_page.dart (1)
54-107: UI 구현에 대한 개선 제안UI 구현은 전반적으로 잘 되어 있으나, 다음 사항들을 개선하면 좋을 것 같습니다:
- 설명 입력 중 저장되지 않은 변경사항이 있을 때 사용자에게 알림
- 네트워크 오류 시 사용자 피드백 추가
다음과 같이 수정을 제안합니다:
BlocBuilder<GroupManagementBloc, GroupManagementState>( builder: (context, state) { + return Column( + children: [ + if (state is _Error) + Text( + state.message, + style: TextStyle(color: Colors.red), + ), ZiggleButton.cta( onPressed: () { context.read<GroupManagementBloc>().add( GroupManagementEvent.updateDescription( widget.uuid, _controller.text)); context.router.maybePop(); }, disabled: _controller.text.isEmpty || _controller.text == widget.description, + child: state is _Loading + ? CircularProgressIndicator() + : Text(context.t.group.manage.change), ), + ], + ); }, )lib/app/modules/groups/presentation/pages/group_management_main_page.dart (1)
82-109: ListView 구현을 개선할 수 있습니다.현재 구현에서 다음과 같은 개선사항을 제안드립니다:
itemCount에서state.groups!를 직접 접근하는 것은 안전하지 않을 수 있습니다.groups.list.length를 여러 번 계산하는 것은 비효율적입니다.다음과 같이 개선해보세요:
return Expanded( child: ListView.separated( - itemCount: state.groups!.list.length + 1, + itemCount: groups.list.length + 1, itemBuilder: (context, index) { + final listLength = groups.list.length; - if (index == groups.list.length) { + if (index == listLength) { return Padding( padding: const EdgeInsets.fromLTRB(0, 15, 0, 25), child: _InquiryWidget(), ); } return GroupListItem( name: groups.list[index].name, onPressed: () { context.router.push( GroupManagementShellRoute( group: groups.list[index], ), ); }, ); }, separatorBuilder: (context, index) => SizedBox(height: 5), ), );lib/app/modules/groups/presentation/pages/group_management_name_page.dart (1)
29-43: 상태 관리가 깔끔하게 구현되었습니다.
TextEditingController의 초기화, 리스너 추가, dispose가 모두 적절하게 구현되었습니다. 이전 리뷰에서 논의된 문제들이 잘 해결되었네요.한 가지 제안:
noop대신 명시적인 함수명을 사용하면 코드의 의도가 더 명확해질 것 같습니다.- _controller.addListener(() => setState(noop)); + _controller.addListener(() => setState(() {}));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
assets/icons/lock.svgis excluded by!**/*.svg
📒 Files selected for processing (35)
assets/i18n/strings.i18n.json(2 hunks)assets/i18n/strings_jp.i18n.json(2 hunks)assets/i18n/strings_ko.i18n.json(2 hunks)assets/i18n/strings_ru.i18n.json(2 hunks)lib/app/modules/common/presentation/widgets/ziggle_row_button.dart(3 hunks)lib/app/modules/groups/data/data_sources/models/group_invite_code_response_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/models/group_item_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/models/group_list_model.dart(2 hunks)lib/app/modules/groups/data/data_sources/models/group_model.dart(2 hunks)lib/app/modules/groups/data/data_sources/models/member_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/models/memeber_list_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/models/modify_group_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/models/president_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/remote/group_api.dart(3 hunks)lib/app/modules/groups/data/repositories/rest_group_repository.dart(1 hunks)lib/app/modules/groups/domain/entities/group_entity.dart(1 hunks)lib/app/modules/groups/domain/entities/member_entity.dart(1 hunks)lib/app/modules/groups/domain/entities/member_list_entity.dart(1 hunks)lib/app/modules/groups/domain/entities/president_entity.dart(1 hunks)lib/app/modules/groups/domain/repository/group_repository.dart(1 hunks)lib/app/modules/groups/presentation/blocs/group_invite_bloc.dart(1 hunks)lib/app/modules/groups/presentation/blocs/group_management_bloc.dart(1 hunks)lib/app/modules/groups/presentation/blocs/group_management_main_bloc.dart(2 hunks)lib/app/modules/groups/presentation/blocs/group_member_bloc.dart(1 hunks)lib/app/modules/groups/presentation/layouts/group_management_shell_layout.dart(1 hunks)lib/app/modules/groups/presentation/pages/group_management_description_page.dart(3 hunks)lib/app/modules/groups/presentation/pages/group_management_invitation_link_page.dart(2 hunks)lib/app/modules/groups/presentation/pages/group_management_main_page.dart(1 hunks)lib/app/modules/groups/presentation/pages/group_management_member_page.dart(1 hunks)lib/app/modules/groups/presentation/pages/group_management_name_page.dart(2 hunks)lib/app/modules/groups/presentation/pages/group_management_notion_page.dart(2 hunks)lib/app/modules/groups/presentation/pages/group_management_page.dart(2 hunks)lib/app/modules/user/presentation/bloc/group_auth_bloc.dart(1 hunks)lib/app/modules/user/presentation/pages/profile_page.dart(3 hunks)lib/app/router.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (18)
- lib/app/modules/groups/data/data_sources/models/member_model.dart
- lib/app/modules/user/presentation/bloc/group_auth_bloc.dart
- lib/app/modules/groups/domain/entities/member_entity.dart
- lib/app/modules/groups/data/data_sources/models/group_invite_code_response_model.dart
- lib/app/modules/groups/domain/entities/president_entity.dart
- lib/app/modules/user/presentation/pages/profile_page.dart
- lib/app/modules/groups/data/data_sources/models/president_model.dart
- lib/app/modules/groups/presentation/layouts/group_management_shell_layout.dart
- lib/app/modules/groups/data/data_sources/models/group_list_model.dart
- assets/i18n/strings_ru.i18n.json
- lib/app/modules/groups/domain/entities/member_list_entity.dart
- lib/app/modules/groups/data/data_sources/models/group_item_model.dart
- lib/app/modules/groups/presentation/blocs/group_member_bloc.dart
- lib/app/modules/groups/data/data_sources/models/group_model.dart
- lib/app/modules/groups/presentation/blocs/group_invite_bloc.dart
- lib/app/modules/groups/data/data_sources/models/memeber_list_model.dart
- lib/app/modules/groups/domain/entities/group_entity.dart
- assets/i18n/strings_ko.i18n.json
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build iOS App and Test
- GitHub Check: Build Android App and Test
🔇 Additional comments (22)
lib/app/modules/groups/data/data_sources/models/modify_group_model.dart (1)
8-12: 생성자 매개변수 변경이 적절해 보입니다!명명된 매개변수(named parameters)를 사용하도록 변경한 것이 좋은 개선사항입니다. 또한
@JsonKey(includeIfNull: false)어노테이션을 추가하여 null 값을 가진 필드를 JSON에서 제외하는 것도 적절한 최적화입니다.기존 코드에서 이 모델을 사용하는 부분들을 확인하기 위해 다음 스크립트를 실행해주세요:
✅ Verification successful
생성자 사용 방식이 모두 올바르게 적용되어 있습니다!
rest_group_repository.dart의 모든 호출이 이미 명명된 매개변수를 사용하고 있으며, 변경된 생성자 패턴과 완벽하게 일치합니다.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find usages of ModifyGroupModel constructor # Expected: All usages should be updated to use named parameters rg "ModifyGroupModel\(" --type dartLength of output: 564
lib/app/modules/common/presentation/widgets/ziggle_row_button.dart (2)
9-9: 속성 이름 변경 및 새로운 속성 추가에 대한 피드백
icon을leadingIcon으로 변경한 것은 좋은 개선이지만, 이전 리뷰 코멘트에서 언급된 것처럼 자주 사용되지 않는 경우라면 위젯 파라미터로 받는 것이 더 유연할 수 있습니다.또한,
trailingIcon속성 추가로 레이아웃의 유연성이 향상되었습니다.Also applies to: 14-14, 18-18, 22-22
29-29: 비활성화 상태 처리 개선비활성화된 상태에서
onPressed를null로 설정하는 것은 Flutter의 관례를 잘 따르고 있으며, 접근성 측면에서도 적절한 구현입니다.lib/app/modules/groups/presentation/pages/group_management_page.dart (2)
89-105: 이미지 선택 로직 개선이 여전히 필요합니다.이전 리뷰에서 지적된 이미지 선택 관련 문제가 아직 해결되지 않았습니다:
- 파일 크기 제한 검증
- 이미지 형식 검증
- 오류 처리
150-196: 그룹 삭제/탈퇴 작업의 상태 처리가 여전히 미흡합니다.이전 리뷰에서 지적된 다음 문제점들이 아직 해결되지 않았습니다:
- 작업 진행 중 로딩 상태 표시
- 작업 실패 시 예외 처리
- 작업 완료 후 상태 전환
lib/app/modules/groups/domain/repository/group_repository.dart (2)
37-38: 초대 링크 생성 메서드의 매개변수 제약 조건이 필요합니다.
createInviteLink메서드의duration매개변수에 대한 제약 조건이 명시되어 있지 않습니다:
- 최소/최대 허용 기간
- 단위(초/분/시간 등)
45-54: 역할 관리 메서드들의 안전성 검증이 필요합니다.역할 관리 관련 메서드들(
grantRoleToUser,removeRoleFromUser)에 대해 다음 사항들을 검증해야 합니다:
- 유효한 roleId 값 범위
- 동일한 역할 중복 부여 방지
- 필수 역할 제거 방지
lib/app/modules/groups/presentation/pages/group_management_notion_page.dart (1)
61-74: 상태 관리 로직에 오류 처리와 로딩 상태가 필요합니다.BlocBuilder를 통한 상태 관리가 잘 구현되어 있으나, 사용자 경험 향상을 위해 다음 사항들을 고려해보세요:
lib/app/modules/groups/data/data_sources/remote/group_api.dart (1)
58-60: 초대 코드 생성 API의 매개변수 검증이 필요합니다.
createInviteCode메서드에서 다음 사항들을 검증해야 합니다:
- duration 매개변수의 유효 범위
- 응답 모델의 유효성
lib/app/modules/groups/presentation/pages/group_management_description_page.dart (2)
15-22: 생성자와 필수 매개변수가 올바르게 정의되었습니다.필수 매개변수
uuid와description이 명확하게 선언되어 있습니다.
31-44: 컨트롤러 관리가 적절히 구현되었습니다.
TextEditingController의 초기화와 dispose가 올바르게 처리되어 있습니다. 리스너를 통한 상태 업데이트도 적절합니다.lib/app/modules/groups/presentation/pages/group_management_invitation_link_page.dart (1)
17-19: 생성자와 필수 매개변수가 올바르게 정의되었습니다.필수 매개변수
uuid가 명확하게 선언되어 있습니다.lib/app/router.dart (1)
96-127: 라우팅 구조가 개선되었습니다.그룹 관리 관련 라우트들이
/group/management경로 아래에 체계적으로 구성되어 있습니다. 각 하위 라우트의 경로도 일관성 있게 정의되어 있습니다.lib/app/modules/groups/presentation/pages/group_management_main_page.dart (3)
25-28: 코드 구조가 잘 분리되어 있습니다!
_Layout클래스를 별도로 분리하여 관심사 분리가 잘 되어있고, 코드의 가독성과 유지보수성이 향상되었습니다.
36-53: 앱바 구현이 깔끔합니다!ZiggleAppBar를 사용하여 일관된 UI를 유지하고 있으며, 텍스트 스타일링도 적절히 정의되어 있습니다.
110-126: 에러 상태 UI가 잘 구현되어 있습니다!사용자 친화적인 에러 메시지와 함께 시각적 피드백을 제공하고 있으며, 텍스트 스타일링도 앱의 디자인 시스템을 잘 따르고 있습니다.
assets/i18n/strings_jp.i18n.json (2)
69-72: 일본어 번역이 자연스럽게 잘 되어있습니다!그룹 삭제와 탈퇴에 대한 확인 메시지가 일본어 사용자에게 명확하게 전달될 수 있도록 잘 번역되어 있습니다.
90-91: 초대 링크 관련 문구가 잘 번역되었습니다!"초대 링크 복사" 기능에 대한 일본어 번역이 자연스럽고 다른 UI 요소들과 일관성이 있습니다.
assets/i18n/strings.i18n.json (2)
69-72: 영문 확인 메시지가 명확하게 작성되어 있습니다!그룹 삭제와 탈퇴에 대한 영문 확인 메시지가 전문적이고 이해하기 쉽게 작성되어 있습니다.
90-91: 초대 링크 관련 영문이 명확합니다!"Copy invitation link" 문구가 사용자 행동을 명확하게 설명하고 있으며, UI/UX 모범 사례를 잘 따르고 있습니다.
lib/app/modules/groups/presentation/pages/group_management_name_page.dart (2)
3-27: StatefulWidget으로의 전환이 잘 구현되었습니다!필수 매개변수와 함께 StatefulWidget으로 적절하게 변환되었습니다.
74-91: 상태 업데이트 완료 확인이 필요합니다.현재 구현에서는 상태 업데이트 요청 직후 바로 페이지를 닫고 있습니다. 상태 업데이트가 완료되기 전에 화면이 닫힐 수 있어 사용자가 업데이트 결과를 확인하기 어려울 수 있습니다.
다음과 같이 상태 업데이트 완료를 확인한 후 화면을 닫는 것을 고려해보세요:
- context - .read<GroupManagementBloc>() - .add(GroupManagementEvent.updateName( - widget.uuid, - _controller.text, - )); - context.router.maybePop(); + context + .read<GroupManagementBloc>() + .add(GroupManagementEvent.updateName( + widget.uuid, + _controller.text, + )); + // 상태 업데이트 완료 대기 + context.watch<GroupManagementBloc>().stream.firstWhere( + (state) => state.maybeWhen( + success: () => true, + orElse: () => false, + ), + ).then((_) => context.router.maybePop());
| on<_Load>((event, emit) async { | ||
| emit(_Loading()); | ||
| try { | ||
| final groups = await _repository.getGroups(); | ||
| if (groups.list.isNotEmpty) { | ||
| emit(_Loaded(groups)); | ||
| return; | ||
| } | ||
| emit(_Error("No groups found")); | ||
| } on Exception catch (e) { | ||
| emit(_Error(e.toString())); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
에러 처리 개선이 필요합니다.
_Load 이벤트 핸들러의 에러 처리를 더 구체적으로 개선해야 합니다:
- 특정 예외 타입에 따른 처리
- 사용자 친화적인 에러 메시지
- 재시도 메커니즘
| itemBuilder: (context, index) => | ||
| GroupMemberCard.editMode( | ||
| name: members.list[index].name, | ||
| email: members.list[index].email, | ||
| role: GroupMemberRole.admin, | ||
| onBanish: () {}, | ||
| onChanged: (e) {}, | ||
| ), |
There was a problem hiding this comment.
멤버 역할 관리 로직이 불완전합니다.
GroupMemberCard.editMode에서 다음 기능들이 구현되어야 합니다:
- 실제 멤버 역할 반영
- 역할 변경 시 확인 다이얼로그
- 권한 검증
| child: BlocBuilder<GroupMemberBloc, GroupMemberState>( | ||
| builder: (context, state) { | ||
| return state.maybeWhen( | ||
| orElse: () => Container(), | ||
| initial: () => Container(), | ||
| loading: () => Center( | ||
| child: Lottie.asset(Assets.lotties.loading, | ||
| height: MediaQuery.of(context).size.width * 0.2, | ||
| width: MediaQuery.of(context).size.width * 0.2), | ||
| ), | ||
| loaded: (members) { | ||
| return Column( | ||
| children: [ | ||
| Expanded( | ||
| child: ListView.separated( | ||
| itemBuilder: (context, index) => | ||
| GroupMemberCard.editMode( | ||
| name: members.list[index].name, | ||
| email: members.list[index].email, | ||
| role: GroupMemberRole.admin, | ||
| onBanish: () {}, | ||
| onChanged: (e) {}, | ||
| ), | ||
| separatorBuilder: (context, index) => | ||
| SizedBox(height: 10), | ||
| itemCount: members.list.length, | ||
| ), | ||
| ), | ||
| SizedBox(height: 30), | ||
| ZiggleButton.cta( | ||
| child: Text(context.t.group.manage.back), | ||
| onPressed: () => context.maybePop(), | ||
| ), | ||
| ], | ||
| ); | ||
| }, | ||
| ); | ||
| }, | ||
| ), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
상태 처리 로직 개선이 필요합니다.
BlocBuilder 구현에서 다음 사항들을 개선해야 합니다:
- 에러 상태 처리
- 빈 목록 상태 처리
- 새로고침 기능
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
lib/app/modules/groups/presentation/blocs/group_management_bloc.dart (3)
18-22: 🛠️ Refactor suggestion에러 처리 일관성 개선 필요
_Load,_UpdateProfileImage,_UpdateNotionLink,_RemoveMember이벤트 핸들러에서 에러 처리가 누락되었습니다. 모든 비동기 작업에 대해 일관된 에러 처리가 필요합니다. 다음과 같이 수정하는 것을 제안합니다.on<_Load>((event, emit) async { emit(GroupManagementState.loading()); + try { final fetchedGroup = await _repository.getGroup(event.uuid); emit(GroupManagementState.success(fetchedGroup)); + } catch (e) { + emit(GroupManagementState.error(e.toString())); + } });다른 핸들러들도 동일한 방식으로 수정해주시기 바랍니다.
Also applies to: 23-29, 51-59, 60-66
67-74: 🛠️ Refactor suggestion상태 관리 개선 필요
_Delete와_Leave이벤트 핸들러에서 최종 상태를 방출하고 있지 않습니다. 작업 완료 후 적절한 상태를 방출해야 합니다.on<_Delete>((event, emit) async { emit(GroupManagementState.loading()); await _repository.deleteGroup(event.uuid); + emit(GroupManagementState.done()); }); on<_Leave>((event, emit) async { emit(GroupManagementState.loading()); await _repository.leaveGroup(event.uuid); + emit(GroupManagementState.done()); });
18-74:⚠️ Potential issue미구현된 이벤트 핸들러 확인
_GetMembers,_GrantRoleToUser,_RemoveRoleFromUser이벤트에 대한 핸들러가 구현되지 않았습니다. 해당 이벤트들을 처리하기 위한 핸들러를 구현해주시기 바랍니다.lib/app/modules/groups/presentation/pages/group_management_member_page.dart (1)
50-64:⚠️ Potential issue멤버 역할 관리 로직을 완성하세요
GroupMemberCard.editMode에서 실제 멤버의 역할을 반영하고, 역할 변경 시 확인 다이얼로그를 표시하며, 권한 검증을 추가해야 합니다. 현재는 역할 ID를 하드코딩하고 있어 역할 변경이 올바르게 동작하지 않을 수 있습니다.이전 리뷰에서 제안된 내용과 중복됩니다.
🧹 Nitpick comments (7)
lib/app/modules/groups/data/data_sources/models/role_list_model.dart (1)
9-9: 구현이 잘 되어있습니다만, 문서화를 추가하면 좋겠습니다.RoleListEntity 인터페이스 구현이 적절하게 되어있습니다. 다만, 클래스의 목적과 사용법에 대한 문서화를 추가하면 코드의 가독성이 더욱 향상될 것 같습니다.
다음과 같이 문서화를 추가하는 것을 제안드립니다:
@freezed +/// RoleListModel은 그룹 내 역할 목록을 관리하는 모델입니다. +/// +/// [roles]는 그룹 내 정의된 역할들의 목록을 포함합니다. sealed class RoleListModel with _$RoleListModel implements RoleListEntity {lib/app/modules/groups/domain/entities/authority_entity.dart (1)
1-5: 클래스 문서화 및 기능 개선이 필요합니다.이 엔티티 클래스는 그룹 권한 관리의 핵심 부분으로 보이는데, 다음과 같은 개선사항을 제안드립니다:
- 클래스의 목적과 사용법에 대한 문서화가 필요합니다.
- authorities 리스트에 대한 유효성 검사가 필요합니다.
- 권한 확인을 위한 유틸리티 메서드 추가가 필요합니다.
- 객체 비교를 위한 equals, hashCode 메서드가 필요합니다.
다음과 같이 개선하는 것을 추천드립니다:
+/// 사용자 또는 역할의 권한을 관리하는 엔티티입니다. +/// +/// [authorities]는 부여된 권한들의 목록을 포함합니다. class AuthorityEntity { final List<String> authorities; - AuthorityEntity({required this.authorities}); + AuthorityEntity({required List<String> authorities}) + : authorities = List.unmodifiable(authorities); + + /// 특정 권한이 있는지 확인합니다. + bool hasAuthority(String authority) => authorities.contains(authority); + + /// 주어진 권한들을 모두 가지고 있는지 확인합니다. + bool hasAllAuthorities(List<String> requiredAuthorities) => + requiredAuthorities.every(hasAuthority); + + @override + bool operator ==(Object other) => + identical(this, other) || + other is AuthorityEntity && + runtimeType == other.runtimeType && + authorities.length == other.authorities.length && + authorities.every((auth) => other.authorities.contains(auth)); + + @override + int get hashCode => authorities.fold(0, (h, auth) => h ^ auth.hashCode); + + @override + String toString() => 'AuthorityEntity(authorities: $authorities)'; }lib/app/modules/groups/data/data_sources/remote/group_api.dart (2)
25-26: 메서드 문서화 추가 필요API 엔드포인트의 동작을 명확히 하기 위해 메서드에 문서 주석을 추가하는 것이 좋습니다.
다음과 같은 문서 주석 추가를 제안합니다:
+ /// 새로운 그룹을 생성합니다. + /// + /// [model]에는 그룹 생성에 필요한 정보가 포함되어야 합니다. + /// 성공 시 생성된 그룹의 정보를 [GroupItemModel]로 반환합니다. @POST('') Future<GroupItemModel> createGroup(@Body() CreateGroupModel model);
61-64: 페이지네이션 구현 고려대규모 그룹의 경우 모든 멤버 목록을 한 번에 가져오면 성능 문제가 발생할 수 있습니다. 페이지네이션 구현을 고려해보세요.
예시 구현:
@GET('{uuid}/member') Future<MemberListModel> getMembers( @Path('uuid') String uuid, + @Query('page') int page, + @Query('size') int size, );lib/app/modules/groups/data/repositories/rest_group_repository.dart (1)
127-142: 역할 관리 기능 구현 시 고려사항역할 관리 메서드 구현 시 다음 사항들을 고려해야 합니다:
- 역할 변경 시 권한 검증
- 시스템 기본 역할 보호
- 순환 참조 방지
구현 시 다음과 같은 검증 로직이 필요합니다:
Future<void> createRole(String groupUuid, RoleEntity role) async { // 1. 현재 사용자의 권한 검증 // 2. 역할 이름 중복 검사 // 3. 상위 역할 순환 참조 검사 // 4. 역할 생성 및 캐시 갱신 }lib/app/modules/groups/presentation/blocs/group_member_bloc.dart (2)
33-34: 불필요한
print(roles);구문은 디버깅을 위한 것으로 보입니다. 프로덕션 코드에서는33 final roles = await _repository.getRoles(event.uuid); 34 - print(roles);
23-30: 다른 이벤트 핸들러에서도 예외 처리를 추가하세요
_RemoveMember,_GrantRoleToUser,_RemoveRoleFromUser이벤트 핸들러에서도 예외 처리를 추가하여 오류 상황을 적절히 처리해 주세요.Also applies to: 31-41, 42-50
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
lib/app/modules/groups/data/data_sources/models/member_list_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/models/role_list_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/models/role_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/remote/group_api.dart(4 hunks)lib/app/modules/groups/data/repositories/rest_group_repository.dart(1 hunks)lib/app/modules/groups/domain/entities/authority_entity.dart(1 hunks)lib/app/modules/groups/domain/entities/role_list_entity.dart(1 hunks)lib/app/modules/groups/domain/repository/group_repository.dart(1 hunks)lib/app/modules/groups/domain/repository/group_role_repository.dart(1 hunks)lib/app/modules/groups/presentation/blocs/group_management_bloc.dart(1 hunks)lib/app/modules/groups/presentation/blocs/group_member_bloc.dart(1 hunks)lib/app/modules/groups/presentation/pages/group_management_member_page.dart(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- lib/app/modules/groups/domain/repository/group_role_repository.dart
- lib/app/modules/groups/domain/entities/role_list_entity.dart
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build iOS App and Test
- GitHub Check: Build Android App and Test
🔇 Additional comments (8)
lib/app/modules/groups/data/data_sources/models/role_model.dart (1)
8-8: 구현이 잘 되어있습니다!RoleEntity 인터페이스 구현이 적절하게 되어있으며, freezed 어노테이션을 통한 코드 생성도 올바르게 설정되어 있습니다.
lib/app/modules/groups/data/data_sources/models/member_list_model.dart (1)
1-14: 올바르게 구현되었습니다코드가 정확하며 특별한 문제점이 없습니다.
lib/app/modules/groups/domain/repository/group_repository.dart (1)
3-69: 인터페이스 메서드 추가 확인새로운 메서드들이 적절하게 추가되었으며, 인터페이스가 잘 확장되었습니다.
lib/app/modules/groups/data/data_sources/remote/group_api.dart (1)
58-60: 매개변수 유효성 검사 필요
duration매개변수에 대한 유효성 검사가 필요합니다. 음수 값이나 너무 큰 값이 전달될 경우 서버에서 오류가 발생할 수 있습니다.다음 스크립트로 서버 측 유효성 검사 여부를 확인할 수 있습니다:
lib/app/modules/groups/data/repositories/rest_group_repository.dart (3)
22-23: 리소스 관리 개선 필요
BehaviorSubject는 메모리 누수를 방지하기 위해 dispose 되어야 합니다.
101-125: 미구현된 메서드 구현 필요그룹 관리의 핵심 기능인 역할 관리 메서드들이 구현되지 않았습니다.
28-39:⚠️ Potential issue에러 처리 및 롤백 로직 추가 필요
그룹 생성 중 발생할 수 있는 다양한 예외 상황(이미지 업로드 실패 등)에 대한 처리가 필요합니다.
다음과 같은 에러 처리 로직 추가를 제안합니다:
Future<GroupEntity> createGroup({ required String name, File? image, required String description, String? notionPageId, }) async { + try { final createdGroup = await _api.createGroup(CreateGroupModel( name: name, description: description, notionPageId: notionPageId)); - if (image != null) await _api.uploadImage(createdGroup.uuid, image); + if (image != null) { + try { + await _api.uploadImage(createdGroup.uuid, image); + } catch (e) { + // 이미지 업로드 실패 시 부분적 롤백 또는 로깅 + print('이미지 업로드 실패: $e'); + } + } await _refreshGroups(); return createdGroup; + } catch (e) { + throw GroupRepositoryException('그룹 생성 실패: $e'); + } }Likely invalid or redundant comment.
lib/app/modules/groups/presentation/pages/group_management_member_page.dart (1)
35-80: 에러 상태와 빈 목록에 대한 처리를 추가하세요
BlocBuilder에서 상태를 처리할 때error상태와 멤버 목록이 비어있는 경우에 대한 처리가 필요합니다. 현재는 이러한 상황에 대한 UI 처리가 없어 사용자가 혼란스러워할 수 있습니다.이전 리뷰에서 제안된 내용과 중복됩니다.
| @override | ||
| Future<void> modifyProfileImage({required String uuid, required File image}) { | ||
| return _api.uploadImage(uuid, image); | ||
| } | ||
|
|
||
| @override | ||
| Future<void> modifyName({required String uuid, required String name}) async { | ||
| await _api.modifyGroup(uuid, ModifyGroupModel(name: name)); | ||
| await _refreshGroups(); | ||
| } | ||
|
|
||
| @override | ||
| Future<void> modifyDescription( | ||
| {required String uuid, required String? description}) async { | ||
| await _api.modifyGroup(uuid, ModifyGroupModel(description: description)); | ||
| } | ||
|
|
||
| @override | ||
| Future<void> modifyNotionLink( | ||
| {required String uuid, required String? notionPageId}) async { | ||
| await _api.modifyGroup(uuid, ModifyGroupModel(notionPageId: notionPageId)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
상태 갱신 동작의 일관성 필요
일부 수정 메서드는 _refreshGroups()를 호출하지만 다른 메서드는 호출하지 않아 데이터 일관성 문제가 발생할 수 있습니다.
모든 수정 메서드에서 _refreshGroups()를 호출하도록 수정이 필요합니다:
Future<void> modifyDescription(
{required String uuid, required String? description}) async {
await _api.modifyGroup(uuid, ModifyGroupModel(description: description));
+ await _refreshGroups();
}
Future<void> modifyNotionLink(
{required String uuid, required String? notionPageId}) async {
await _api.modifyGroup(uuid, ModifyGroupModel(notionPageId: notionPageId));
+ await _refreshGroups();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @override | |
| Future<void> modifyProfileImage({required String uuid, required File image}) { | |
| return _api.uploadImage(uuid, image); | |
| } | |
| @override | |
| Future<void> modifyName({required String uuid, required String name}) async { | |
| await _api.modifyGroup(uuid, ModifyGroupModel(name: name)); | |
| await _refreshGroups(); | |
| } | |
| @override | |
| Future<void> modifyDescription( | |
| {required String uuid, required String? description}) async { | |
| await _api.modifyGroup(uuid, ModifyGroupModel(description: description)); | |
| } | |
| @override | |
| Future<void> modifyNotionLink( | |
| {required String uuid, required String? notionPageId}) async { | |
| await _api.modifyGroup(uuid, ModifyGroupModel(notionPageId: notionPageId)); | |
| } | |
| @override | |
| Future<void> modifyProfileImage({required String uuid, required File image}) { | |
| return _api.uploadImage(uuid, image); | |
| } | |
| @override | |
| Future<void> modifyName({required String uuid, required String name}) async { | |
| await _api.modifyGroup(uuid, ModifyGroupModel(name: name)); | |
| await _refreshGroups(); | |
| } | |
| @override | |
| Future<void> modifyDescription( | |
| {required String uuid, required String? description}) async { | |
| await _api.modifyGroup(uuid, ModifyGroupModel(description: description)); | |
| await _refreshGroups(); | |
| } | |
| @override | |
| Future<void> modifyNotionLink( | |
| {required String uuid, required String? notionPageId}) async { | |
| await _api.modifyGroup(uuid, ModifyGroupModel(notionPageId: notionPageId)); | |
| await _refreshGroups(); | |
| } |
| @override | ||
| Stream<GroupListEntity> watchGroups() => _groupsSubject.stream; | ||
|
|
||
| @override | ||
| Future<GroupListEntity> getGroups() async { | ||
| final GroupListModel fetched = await _api.getGroups(); | ||
| return fetched; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
캐싱 전략 최적화 필요
getGroups와 watchGroups가 서로 다른 데이터 소스를 사용하고 있어 일관성 문제가 발생할 수 있습니다.
다음과 같은 구현을 제안합니다:
@override
Stream<GroupListEntity> watchGroups() => _groupsSubject.stream;
@override
Future<GroupListEntity> getGroups() async {
- final GroupListModel fetched = await _api.getGroups();
- return fetched;
+ return _groupsSubject.value;
}Committable suggestion skipped: line range outside the PR's diff.
| emit(GroupMemberState.loading()); | ||
| final members = await _repository.getMembers(event.uuid); | ||
| emit(GroupMemberState.loaded(members)); | ||
| }); |
There was a problem hiding this comment.
예외 처리를 추가하여 오류를 처리하세요
현재 _repository.getMembers(event.uuid) 호출 시 발생할 수 있는 예외를 처리하지 않고 있습니다. 네트워크 오류 등이 발생하면 앱이 비정상 종료될 수 있으므로, try-catch 문을 사용하여 예외를 처리하고, 에러 상태를 emit하세요.
GroupMemberState에 에러 상태를 추가합니다:
66 @freezed
67 class GroupMemberState with _$GroupMemberState {
68 const factory GroupMemberState.initial() = _Initial;
69 const factory GroupMemberState.loading() = _Loading;
70 const factory GroupMemberState.loaded(MemberListEntity list) = _Loaded;
71 const factory GroupMemberState.success() = _Success;
+ const factory GroupMemberState.error(String message) = _Error;
72 }_GetMembers 이벤트 핸들러에서 예외 처리를 추가합니다:
18 on<_GetMembers>((event, emit) async {
19 emit(GroupMemberState.loading());
+ try {
20 final members = await _repository.getMembers(event.uuid);
21 emit(GroupMemberState.loaded(members));
+ } catch (e) {
+ emit(GroupMemberState.error(e.toString()));
+ }
22 });Committable suggestion skipped: line range outside the PR's diff.
dawnfire05
force-pushed
the
506-v4-group-management-page-logic
branch
from
25cf15f to
5c18738
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (2)
lib/app/modules/groups/presentation/blocs/group_member_bloc.dart (1)
18-22:⚠️ Potential issue예외 처리가 누락되었습니다
_GetMembers이벤트 핸들러에서 예외 처리가 여전히 누락되어 있습니다. 네트워크 오류 등이 발생할 경우 앱이 비정상 종료될 수 있습니다.lib/app/modules/groups/presentation/pages/group_management_member_page.dart (1)
34-80:⚠️ Potential issue상태 처리 로직이 불완전합니다
BlocBuilder 구현에서 다음 상태들의 처리가 누락되어 있습니다:
- 에러 상태 처리
- 빈 목록 상태 처리
- 새로고침 기능
🧹 Nitpick comments (13)
lib/app/modules/groups/presentation/pages/group_management_name_page.dart (1)
74-91: 상태 관리와 이벤트 처리가 잘 구현되어 있으나, 에러 처리를 추가하면 좋겠습니다.BlocBuilder를 통한 상태 관리와 버튼의 활성화 조건이 잘 구현되어 있습니다. 하지만 그룹 이름 업데이트 실패 시의 처리가 없습니다.
다음과 같이 에러 처리를 추가하는 것을 제안드립니다:
onPressed: () { + try { context .read<GroupManagementBloc>() .add(GroupManagementEvent.updateName( widget.uuid, _controller.text, )); context.router.maybePop(); + } catch (e) { + ScaffoldMessenger.of(context).showSnackBar( + SnackBar( + content: Text(context.t.common.error.unknown), + ), + ); + } },lib/app/modules/groups/presentation/pages/group_management_description_page.dart (2)
31-44: setState 로직을 개선할 수 있습니다.현재 구현에서는
noop함수를 사용하여 setState를 호출하고 있습니다. 이를 더 간단하게 만들 수 있습니다.다음과 같이 수정하는 것을 추천드립니다:
- _controller.addListener(() => setState(noop)); + _controller.addListener(() => setState(() {}));
54-106: UI 구현이 잘 되었으나, 접근성 개선이 필요합니다.스크롤 가능한 레이아웃과 상태 관리가 잘 구현되어 있습니다. 하지만 접근성을 위한 몇 가지 개선사항이 있습니다.
TextFormField에 다음과 같이 semanticsLabel을 추가하는 것을 추천드립니다:
TextFormField( minLines: 7, maxLines: 10, maxLength: 200, controller: _controller, decoration: InputDecoration( counter: const SizedBox.shrink(), border: const OutlineInputBorder(borderSide: BorderSide.none), contentPadding: const EdgeInsets.symmetric(vertical: 13), hintText: context.t.group.creation.introduce.hint, hintStyle: const TextStyle(color: Palette.grayText), ), + semanticsLabel: context.t.group.manage.description.header, ),lib/app/modules/groups/presentation/pages/group_management_notion_page.dart (1)
32-36: initState에서 setState 대신 BlocEvent를 사용하세요.컨트롤러의 리스너에서 직접 setState를 호출하는 대신, BlocEvent를 통해 상태를 관리하는 것이 좋습니다.
@override void initState() { super.initState(); - _controller.addListener(() => setState(noop)); + _controller.addListener(() { + context.read<GroupManagementBloc>().add( + GroupManagementEvent.textChanged(_controller.text) + ); + }); }lib/app/modules/groups/presentation/pages/group_management_page.dart (2)
31-44: 오류 메시지 표시 방식을 개선하세요.현재 오류 메시지가 단순 텍스트로 표시되고 있습니다. 사용자 경험 향상을 위해 다음과 같이 개선하는 것이 좋습니다:
- error: (message) => Text( - '오류 발생: $message', - style: TextStyle(color: Colors.red), - ), + error: (message) => Center( + child: Column( + mainAxisAlignment: MainAxisAlignment.center, + children: [ + Icon(Icons.error_outline, color: Palette.primary, size: 48), + const SizedBox(height: 16), + Text( + '오류가 발생했습니다', + style: TextStyle( + fontSize: 18, + fontWeight: FontWeight.bold, + color: Palette.black, + ), + ), + const SizedBox(height: 8), + Text( + message, + style: TextStyle(color: Palette.grey), + textAlign: TextAlign.center, + ), + ], + ), + ),
156-170: 비즈니스 로직과 네비게이션 로직을 분리하세요.현재 구현에서는 비즈니스 로직(그룹 삭제/탈퇴)과 네비게이션 로직이 혼재되어 있습니다. 다음과 같은 개선을 제안합니다:
- Bloc에서 상태 변경 후 네비게이션을 트리거하도록 수정
- 페이지에서는 상태 변경을 감지하여 네비게이션 수행
// bloc에서 case const GroupDeleted(): emit(GroupManagementState.success(/* ... */)); // 페이지에서 BlocListener<GroupManagementBloc, GroupManagementState>( listener: (context, state) { state.maybeWhen( orElse: () {}, success: (group) { if (group == null) { context.router.navigate(GroupManagementMainRoute()); } }, ); }, child: // ... 현재 UI )Also applies to: 181-195
lib/app/modules/groups/presentation/pages/group_management_main_page.dart (4)
44-48: 스타일 상수를 추출하는 것이 좋겠습니다.텍스트 스타일 값들을 상수로 추출하여 재사용성을 높이고 일관성을 유지하는 것이 좋습니다.
+const kNewGroupTextStyle = TextStyle( + fontSize: 16, + color: Palette.primary, + fontWeight: FontWeight.w400, +); -style: const TextStyle( - fontSize: 16, - color: Palette.primary, - fontWeight: FontWeight.w400, -), +style: kNewGroupTextStyle,
57-57: 패딩 값을 상수로 추출하는 것이 좋겠습니다.매직 넘버로 사용된 패딩 값들을 의미 있는 상수로 추출하면 유지보수가 더 쉬워질 것 같습니다.
+const kContentPadding = EdgeInsets.fromLTRB(16, 25, 16, 0); -padding: const EdgeInsets.fromLTRB(16, 25, 16, 0), +padding: kContentPadding,
77-81: 로딩 상태의 크기 제한이 필요합니다.로딩 애니메이션의 크기가 화면 크기에 따라 동적으로 계산되고 있습니다. 최소/최대 크기 제한을 추가하는 것이 좋겠습니다.
-height: MediaQuery.of(context).size.width * 0.2, -width: MediaQuery.of(context).size.width * 0.2), +height: clampDouble( + MediaQuery.of(context).size.width * 0.2, + 40, + 100, +), +width: clampDouble( + MediaQuery.of(context).size.width * 0.2, + 40, + 100, +),
110-126: 에러 메시지를 더 자세히 표시해주세요.현재는 기본 "그룹이 없습니다" 메시지만 표시되고 있습니다. 실제 에러 메시지를 함께 표시하면 디버깅에 도움이 될 것 같습니다.
Text( - context.t.group.managementMain.noGroup, + '${context.t.group.managementMain.noGroup}\n$message', textAlign: TextAlign.center, style: const TextStyle( color: Palette.grayText, fontSize: 20, fontWeight: FontWeight.w600, ), ),lib/app/modules/groups/domain/repository/group_repository.dart (1)
18-69: 그룹 관리를 위한 포괄적인 인터페이스 설계가 잘 되었습니다.새로 추가된 메서드들이 그룹 관리의 모든 핵심 기능을 잘 커버하고 있습니다:
- 그룹 CRUD 작업
- 멤버 관리
- 역할 관리
- 초대 링크 생성
각 메서드의 명명이 명확하고 책임이 잘 분리되어 있습니다.
lib/app/modules/groups/data/repositories/rest_group_repository.dart (1)
101-149: 역할 관리 메서드의 보안 검증 필요역할 관리 관련 메서드들(
grantRoleToUser,removeRoleFromUser,createRole,updateRole)에 대한 구현 시 다음 보안 검증이 필요합니다:
- 요청자의 권한 검증
- 대상 사용자의 존재 여부 확인
- 중복 역할 부여 방지
- 필수 역할 삭제 방지
lib/app/modules/groups/presentation/blocs/group_member_bloc.dart (1)
34-34: 디버그 print 문을 제거하세요프로덕션 코드에 디버그용 print 문이 남아있습니다. 로깅이 필요한 경우 적절한 로깅 시스템을 사용하세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
assets/icons/lock.svgis excluded by!**/*.svg
📒 Files selected for processing (40)
assets/i18n/strings.i18n.json(2 hunks)assets/i18n/strings_jp.i18n.json(2 hunks)assets/i18n/strings_ko.i18n.json(2 hunks)assets/i18n/strings_ru.i18n.json(2 hunks)lib/app/modules/common/presentation/widgets/ziggle_row_button.dart(3 hunks)lib/app/modules/groups/data/data_sources/models/group_invite_code_response_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/models/group_item_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/models/group_list_model.dart(2 hunks)lib/app/modules/groups/data/data_sources/models/group_model.dart(2 hunks)lib/app/modules/groups/data/data_sources/models/member_list_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/models/member_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/models/modify_group_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/models/president_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/models/role_list_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/models/role_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/remote/group_api.dart(4 hunks)lib/app/modules/groups/data/repositories/rest_group_repository.dart(1 hunks)lib/app/modules/groups/domain/entities/authority_entity.dart(1 hunks)lib/app/modules/groups/domain/entities/group_entity.dart(1 hunks)lib/app/modules/groups/domain/entities/member_entity.dart(1 hunks)lib/app/modules/groups/domain/entities/member_list_entity.dart(1 hunks)lib/app/modules/groups/domain/entities/president_entity.dart(1 hunks)lib/app/modules/groups/domain/entities/role_list_entity.dart(1 hunks)lib/app/modules/groups/domain/repository/group_repository.dart(1 hunks)lib/app/modules/groups/domain/repository/group_role_repository.dart(1 hunks)lib/app/modules/groups/presentation/blocs/group_invite_bloc.dart(1 hunks)lib/app/modules/groups/presentation/blocs/group_management_bloc.dart(1 hunks)lib/app/modules/groups/presentation/blocs/group_management_main_bloc.dart(2 hunks)lib/app/modules/groups/presentation/blocs/group_member_bloc.dart(1 hunks)lib/app/modules/groups/presentation/layouts/group_management_shell_layout.dart(1 hunks)lib/app/modules/groups/presentation/pages/group_management_description_page.dart(3 hunks)lib/app/modules/groups/presentation/pages/group_management_invitation_link_page.dart(2 hunks)lib/app/modules/groups/presentation/pages/group_management_main_page.dart(1 hunks)lib/app/modules/groups/presentation/pages/group_management_member_page.dart(1 hunks)lib/app/modules/groups/presentation/pages/group_management_name_page.dart(2 hunks)lib/app/modules/groups/presentation/pages/group_management_notion_page.dart(2 hunks)lib/app/modules/groups/presentation/pages/group_management_page.dart(2 hunks)lib/app/modules/user/presentation/bloc/group_auth_bloc.dart(1 hunks)lib/app/modules/user/presentation/pages/profile_page.dart(3 hunks)lib/app/router.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (26)
- lib/app/modules/groups/domain/repository/group_role_repository.dart
- lib/app/modules/groups/domain/entities/authority_entity.dart
- lib/app/modules/groups/data/data_sources/models/group_invite_code_response_model.dart
- lib/app/modules/groups/domain/entities/president_entity.dart
- lib/app/modules/groups/data/data_sources/models/member_model.dart
- lib/app/modules/groups/domain/entities/member_entity.dart
- lib/app/modules/groups/data/data_sources/models/role_model.dart
- lib/app/modules/user/presentation/bloc/group_auth_bloc.dart
- lib/app/modules/user/presentation/pages/profile_page.dart
- lib/app/modules/groups/domain/entities/role_list_entity.dart
- lib/app/modules/groups/data/data_sources/models/group_list_model.dart
- lib/app/modules/groups/data/data_sources/models/president_model.dart
- lib/app/modules/groups/domain/entities/member_list_entity.dart
- lib/app/modules/common/presentation/widgets/ziggle_row_button.dart
- lib/app/modules/groups/data/data_sources/models/group_model.dart
- lib/app/modules/groups/data/data_sources/models/member_list_model.dart
- lib/app/modules/groups/presentation/layouts/group_management_shell_layout.dart
- lib/app/modules/groups/presentation/blocs/group_management_main_bloc.dart
- lib/app/modules/groups/data/data_sources/models/role_list_model.dart
- assets/i18n/strings_ru.i18n.json
- lib/app/modules/groups/data/data_sources/models/group_item_model.dart
- assets/i18n/strings_ko.i18n.json
- assets/i18n/strings.i18n.json
- lib/app/modules/groups/presentation/blocs/group_management_bloc.dart
- lib/app/modules/groups/domain/entities/group_entity.dart
- lib/app/modules/groups/presentation/blocs/group_invite_bloc.dart
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build Android App and Test
- GitHub Check: Build iOS App and Test
🔇 Additional comments (21)
lib/app/modules/groups/presentation/pages/group_management_name_page.dart (2)
14-27: StatefulWidget으로의 변경이 적절합니다.TextEditingController를 관리하기 위해 StatefulWidget으로 변경한 것이 올바른 접근입니다. 필수 매개변수들도 잘 정의되어 있습니다.
30-43: 컨트롤러 관리가 잘 구현되어 있습니다.
- TextEditingController가 widget.name으로 적절히 초기화되었습니다.
- setState 콜백이 noop 함수를 사용하여 깔끔하게 구현되었습니다.
- dispose에서 컨트롤러가 정상적으로 정리됩니다.
lib/app/modules/groups/presentation/pages/group_management_description_page.dart (1)
3-4: 생성자와 필수 매개변수가 적절히 구현되었습니다!필수 매개변수
uuid와description이 명확하게 정의되어 있으며, 관련 import들도 적절히 추가되었습니다.Also applies to: 15-22
lib/app/modules/groups/presentation/pages/group_management_notion_page.dart (1)
61-74: 상태 관리 로직에 오류 처리와 로딩 상태가 필요합니다.BlocBuilder를 통한 상태 관리가 잘 구현되어 있으나, 사용자 경험 향상을 위해 다음 사항들을 고려해보세요:
BlocBuilder<GroupManagementBloc, GroupManagementState>( builder: (context, state) { + if (state.isLoading) { + return const CircularProgressIndicator(); + } return ZiggleButton.cta( disabled: _controller.text.isEmpty || _controller.text == widget.notionLink, child: Text(context.t.group.manage.change), onPressed: () { + try { context.read<GroupManagementBloc>().add( GroupManagementEvent.updateNotionLink( widget.uuid, _controller.text)); - context.router.maybePop(); + if (mounted) context.router.maybePop(); + } catch (e) { + ScaffoldMessenger.of(context).showSnackBar( + SnackBar(content: Text(context.t.common.error.unknown)), + ); + } }, ); }, ),lib/app/router.dart (1)
96-127: 라우팅 구조가 잘 구성되어 있습니다.그룹 관리 관련 라우트들이 체계적으로 구성되어 있으며, 중첩 라우팅을 통해 관련 기능들이 논리적으로 그룹화되어 있습니다.
assets/i18n/strings_jp.i18n.json (2)
69-72: 그룹 관리 관련 확인 메시지가 적절히 추가되었습니다.사용자에게 중요한 작업에 대한 확인 메시지가 명확하게 작성되어 있습니다.
90-91: 초대 링크 관련 텍스트가 일관성 있게 추가되었습니다.초대 링크 복사 기능에 대한 텍스트가 다른 언어 파일들과 일관되게 추가되었습니다.
lib/app/modules/groups/presentation/pages/group_management_page.dart (3)
1-17: 필요한 모든 의존성이 잘 구성되어 있습니다!각 import가 명확한 목적을 가지고 있으며, 불필요한 import가 없습니다.
89-105: 이미지 선택 로직에 검증 및 오류 처리가 필요합니다.이전 리뷰에서 지적된 이미지 선택 관련 개선사항이 아직 반영되지 않았습니다.
150-196: 그룹 삭제/탈퇴 작업의 상태 처리가 미흡합니다.이전 리뷰에서 지적된 삭제/탈퇴 작업의 상태 처리 관련 개선사항이 아직 반영되지 않았습니다.
lib/app/modules/groups/presentation/pages/group_management_main_page.dart (2)
16-28: 구조가 잘 분리되어 있습니다!상태 관리와 레이아웃 로직이 깔끔하게 분리되어 있어 코드의 가독성과 유지보수성이 향상되었습니다.
38-38: PageSource 값을 검토해주세요.
PageSource.unknown은 적절하지 않아 보입니다. 정확한 출처 페이지를 지정해주세요.lib/app/modules/groups/data/data_sources/models/modify_group_model.dart (1)
8-12: 좋은 리팩토링입니다!명명된 매개변수(named parameters)를 사용하고
@JsonKey(includeIfNull: false)어노테이션을 추가한 것은 다음과 같은 이점이 있습니다:
- 호출 시점에서 매개변수의 의미가 더 명확해집니다
- null 값을 가진 필드가 JSON에서 제외되어 불필요한 데이터 전송을 방지합니다
lib/app/modules/groups/domain/repository/group_repository.dart (1)
11-16: 도메인 엔티티로의 전환이 잘 이루어졌습니다.
GroupModel에서GroupEntity로의 반환 타입 변경은 도메인 주도 설계(DDD) 원칙에 더 잘 부합합니다.lib/app/modules/groups/data/data_sources/remote/group_api.dart (2)
57-59: 초대 코드 생성 API가 개선되었습니다.
duration매개변수 추가와GroupInviteCodeResponseModel반환 타입으로의 변경은 좋은 개선입니다. 하지만duration의 단위(일/시간/분 등)가 명시되어 있지 않습니다.API 문서나 구현을 확인하여
duration의 정확한 단위를 확인해주세요.
61-64: 멤버 조회 API가 추가되었습니다.새로운
getMembers엔드포인트가 잘 구현되었습니다.lib/app/modules/groups/presentation/pages/group_management_invitation_link_page.dart (2)
90-97: 클립보드 작업 개선이 필요합니다.클립보드 복사 기능에 다음 사항들이 누락되어 있습니다:
- 복사 성공/실패에 대한 사용자 피드백
- 클립보드 작업 실패 처리
32-34: Bloc 패턴이 잘 적용되었습니다.
BlocProvider를 사용한 상태 관리 구현이 깔끔합니다. 의존성 주입도sl<GroupInviteBloc>()을 통해 잘 처리되었습니다.lib/app/modules/groups/data/repositories/rest_group_repository.dart (2)
22-23: 스트림 리소스 관리 개선 필요메모리 누수를 방지하기 위해
BehaviorSubject의 dispose 처리가 필요합니다.
71-81: 상태 갱신 동작의 일관성 개선 필요
modifyDescription과modifyNotionLink메서드에서_refreshGroups()를 호출하지 않아 데이터 일관성이 깨질 수 있습니다.lib/app/modules/groups/presentation/blocs/group_member_bloc.dart (1)
66-72:⚠️ Potential issue상태 정의에 오류 상태가 누락되었습니다
GroupMemberState에 오류 상태가 정의되어 있지 않습니다. 에러 처리를 위해 error 상태를 추가하세요.다음과 같이 error 상태를 추가하세요:
@freezed class GroupMemberState with _$GroupMemberState { const factory GroupMemberState.initial() = _Initial; const factory GroupMemberState.loading() = _Loading; const factory GroupMemberState.loaded(MemberListEntity list) = _Loaded; const factory GroupMemberState.success() = _Success; + const factory GroupMemberState.error(String message) = _Error; }Likely invalid or redundant comment.
| if (state.groups != null) | ||
| Expanded( | ||
| loaded: (groups) { | ||
| return Expanded( | ||
| child: ListView.separated( | ||
| itemCount: state.groups!.list.length + 1, |
There was a problem hiding this comment.
null safety 처리가 필요합니다.
state.groups!에서 강제 언래핑을 사용하고 있습니다. null 체크를 추가하거나 더 안전한 방법으로 처리해주세요.
-itemCount: state.groups!.list.length + 1,
+itemCount: (state.groups?.list.length ?? 0) + 1,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| itemCount: state.groups!.list.length + 1, | |
| itemCount: (state.groups?.list.length ?? 0) + 1, |
| @override | ||
| Future<MemberListEntity> getMembers(String uuid) async { | ||
| return _api.getMembers(uuid); | ||
| } | ||
|
|
||
| @override | ||
| Future<void> grantRoleToUser( | ||
| {required String uuid, required String targetUuid, required int roleId}) { | ||
| // TODO: implement grantRoleToUser | ||
| throw UnimplementedError(); | ||
| } | ||
|
|
||
| @override | ||
| Future<void> leaveGroup(String uuid) { | ||
| // TODO: implement leaveGroup | ||
| throw UnimplementedError(); | ||
| } | ||
|
|
||
| @override | ||
| Future<void> removeMember( | ||
| {required String uuid, required String targetUuid}) async { | ||
| await _api.banishUser(uuid, targetUuid); | ||
| } | ||
|
|
||
| @override | ||
| Future<void> removeRoleFromUser( | ||
| {required String uuid, required String targetUuid, required int roleId}) { | ||
| // TODO: implement removeRoleFromUser | ||
| throw UnimplementedError(); | ||
| } | ||
|
|
||
| @override | ||
| Future<void> createRole(String groupUuid, RoleEntity role) { | ||
| // TODO: implement createRole | ||
| throw UnimplementedError(); | ||
| } | ||
|
|
||
| @override | ||
| Future<void> deleteRole(String groupUuid, int roleId) { | ||
| // TODO: implement deleteRole | ||
| throw UnimplementedError(); | ||
| } | ||
|
|
||
| @override | ||
| Future<RoleListEntity> getRoles(String groupUuid) { | ||
| return _api.getRoles(groupUuid); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
역할 관리 관련 예외 처리 구체화 필요
역할 관리 관련 메서드들의 예외 상황에 대한 구체적인 처리가 필요합니다:
class RoleManagementException implements Exception {
final String message;
final String code;
RoleManagementException(this.message, this.code);
static const String INSUFFICIENT_PERMISSIONS = 'INSUFFICIENT_PERMISSIONS';
static const String USER_NOT_FOUND = 'USER_NOT_FOUND';
static const String ROLE_NOT_FOUND = 'ROLE_NOT_FOUND';
static const String DUPLICATE_ROLE = 'DUPLICATE_ROLE';
}| Future<GroupEntity> createGroup({ | ||
| required String name, | ||
| File? image, | ||
| required String description, | ||
| String? notionPageId, | ||
| }) async { | ||
| final createdGroup = await _api.createGroup(CreateGroupModel( | ||
| name: name, | ||
| description: description, | ||
| notionPageId: notionPageId, | ||
| )); | ||
|
|
||
| name: name, description: description, notionPageId: notionPageId)); | ||
| if (image != null) await _api.uploadImage(createdGroup.uuid, image); | ||
| await _refreshGroups(); | ||
| return createdGroup; | ||
| } |
There was a problem hiding this comment.
반환 타입 일관성 확보 필요
createGroup 메서드가 GroupEntity를 반환하도록 선언되어 있으나, API 응답을 직접 반환하고 있습니다. 적절한 타입 변환이 필요합니다.
Future<GroupEntity> createGroup({
required String name,
File? image,
required String description,
String? notionPageId,
}) async {
final createdGroup = await _api.createGroup(CreateGroupModel(
name: name, description: description, notionPageId: notionPageId));
if (image != null) await _api.uploadImage(createdGroup.uuid, image);
await _refreshGroups();
- return createdGroup;
+ return createdGroup.toEntity();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Future<GroupEntity> createGroup({ | |
| required String name, | |
| File? image, | |
| required String description, | |
| String? notionPageId, | |
| }) async { | |
| final createdGroup = await _api.createGroup(CreateGroupModel( | |
| name: name, | |
| description: description, | |
| notionPageId: notionPageId, | |
| )); | |
| name: name, description: description, notionPageId: notionPageId)); | |
| if (image != null) await _api.uploadImage(createdGroup.uuid, image); | |
| await _refreshGroups(); | |
| return createdGroup; | |
| } | |
| Future<GroupEntity> createGroup({ | |
| required String name, | |
| File? image, | |
| required String description, | |
| String? notionPageId, | |
| }) async { | |
| final createdGroup = await _api.createGroup(CreateGroupModel( | |
| name: name, description: description, notionPageId: notionPageId)); | |
| if (image != null) await _api.uploadImage(createdGroup.uuid, image); | |
| await _refreshGroups(); | |
| return createdGroup.toEntity(); | |
| } |
| on<_RemoveMember>((event, emit) async { | ||
| emit(GroupMemberState.loading()); | ||
| await _repository.removeMember( | ||
| uuid: event.uuid, | ||
| targetUuid: event.targetUuid, | ||
| ); | ||
| emit(GroupMemberState.success()); | ||
| }); |
There was a problem hiding this comment.
멤버 제거 작업에 예외 처리가 필요합니다
_RemoveMember 이벤트 핸들러에서 예외 처리가 누락되어 있습니다. 네트워크 오류나 권한 문제가 발생할 수 있으므로 try-catch 블록을 추가하여 적절한 에러 상태를 emit 해야 합니다.
| on<_GrantRoleToUser>((event, emit) async { | ||
| emit(GroupMemberState.loading()); | ||
| final roles = await _repository.getRoles(event.uuid); | ||
| print(roles); | ||
| await _repository.grantRoleToUser( | ||
| uuid: event.uuid, | ||
| targetUuid: event.targetUuid, | ||
| roleId: event.roleId, | ||
| ); | ||
| emit(GroupMemberState.success()); | ||
| }); |
There was a problem hiding this comment.
역할 부여 작업에 예외 처리가 필요합니다
_GrantRoleToUser 이벤트 핸들러에서 다음 사항들이 누락되어 있습니다:
- 네트워크 오류 처리
- 권한 검증
- 역할 존재 여부 확인
적절한 예외 처리와 검증 로직을 추가하세요.
| on<_RemoveRoleFromUser>((event, emit) async { | ||
| emit(GroupMemberState.loading()); | ||
| await _repository.removeRoleFromUser( | ||
| uuid: event.uuid, | ||
| targetUuid: event.targetUuid, | ||
| roleId: event.roleId, | ||
| ); | ||
| emit(GroupMemberState.success()); | ||
| }); |
There was a problem hiding this comment.
역할 제거 작업에 예외 처리가 필요합니다
_RemoveRoleFromUser 이벤트 핸들러에서 예외 처리와 권한 검증이 누락되어 있습니다. 다음 사항들을 고려하여 구현하세요:
- 네트워크 오류 처리
- 권한 검증
- 역할 존재 여부 확인
| GroupMemberCard.editMode( | ||
| name: members.list[index].name, | ||
| email: members.list[index].email, | ||
| role: GroupMemberRole.admin, |
There was a problem hiding this comment.
역할이 하드코딩되어 있습니다
GroupMemberRole.admin이 하드코딩되어 있어 실제 멤버의 역할이 반영되지 않습니다. members.list[index].role을 사용하도록 수정하세요.
| onChanged: (e) { | ||
| context.read<GroupMemberBloc>().add( | ||
| GroupMemberEvent.grantRoleToUser( | ||
| uuid, members.list[index].uuid, 3)); | ||
| }, |
There was a problem hiding this comment.
역할 변경 로직이 불완전합니다
역할 변경 처리에서 다음 사항들이 누락되어 있습니다:
- 역할 ID가 하드코딩되어 있습니다 (3)
- 변경 전 확인 다이얼로그가 없습니다
- 권한 검증이 누락되어 있습니다
사용자 경험과 보안을 위해 위 사항들을 구현하세요.
dawnfire05
force-pushed
the
506-v4-group-management-page-logic
branch
from
5c18738 to
d5931d7
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (2)
lib/app/modules/groups/data/data_sources/remote/group_api.dart (2)
76-80: 매개변수 타입 불일치를 수정해야 합니다.
grantUserRole메서드에서roleId가 String 타입으로 선언되어 있으나, 도메인 계층에서는 int 타입으로 사용됩니다. 타입 일관성을 위해 수정이 필요합니다.Future<void> grantUserRole( @Path('uuid') String uuid, @Path('targetUuid') String targetUuid, - @Query('roleId') String roleId, + @Query('roleId') int roleId, );
83-87: 매개변수 타입 불일치를 수정해야 합니다.
deleteUserRole메서드에서도 동일한 타입 불일치가 있습니다.Future<void> deleteUserRole( @Path('uuid') String uuid, @Path('targetUuid') String targetUuid, - @Query('roleId') String roleId, + @Query('roleId') int roleId, );
♻️ Duplicate comments (11)
lib/app/modules/groups/presentation/pages/group_management_main_page.dart (1)
85-85:⚠️ Potential issuenull safety 처리가 필요합니다.
state.groups!에서 강제 언래핑을 사용하고 있습니다. null 체크를 추가하거나 더 안전한 방법으로 처리해주세요.-itemCount: state.groups!.list.length + 1, +itemCount: (state.groups?.list.length ?? 0) + 1,lib/app/modules/groups/data/repositories/rest_group_repository.dart (1)
41-44:⚠️ Potential issue에러 처리가 필요합니다.
_refreshGroups메서드에서 API 호출 실패에 대한 에러 처리가 누락되었습니다.Future<void> _refreshGroups() async { + try { final newList = await _api.getGroups(); _groupsSubject.add(newList); + } catch (e) { + _groupsSubject.addError('그룹 목록 갱신 실패: $e'); + } }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.Future<void> _refreshGroups() async { try { final newList = await _api.getGroups(); _groupsSubject.add(newList); } catch (e) { _groupsSubject.addError('그룹 목록 갱신 실패: $e'); } }lib/app/modules/groups/presentation/blocs/group_member_bloc.dart (5)
18-22:⚠️ Potential issue예외 처리 구현이 필요합니다
네트워크 오류나 기타 예외 상황에 대한 처리가 누락되어 있습니다. 앱의 안정성을 위해 try-catch 블록을 추가하고 적절한 에러 상태를 emit 하세요.
on<_GetMembers>((event, emit) async { emit(GroupMemberState.loading()); + try { final members = await _repository.getMembers(event.uuid); emit(GroupMemberState.loaded(members)); + } catch (e) { + emit(GroupMemberState.error(e.toString())); + } });
23-30:⚠️ Potential issue멤버 제거 작업의 안전성 개선이 필요합니다
다음 사항들이 누락되어 있습니다:
- 예외 처리
- 사용자 확인 다이얼로그
- 권한 검증
on<_RemoveMember>((event, emit) async { emit(GroupMemberState.loading()); + try { await _repository.removeMember( uuid: event.uuid, targetUuid: event.targetUuid, ); emit(GroupMemberState.success()); + } catch (e) { + emit(GroupMemberState.error(e.toString())); + } });
31-41:⚠️ Potential issue역할 부여 로직의 안전성 개선이 필요합니다
다음 보안 및 안정성 관련 사항들이 누락되어 있습니다:
- 예외 처리
- 권한 검증
- 역할 유효성 검증
- 사용자 확인 절차
on<_GrantRoleToUser>((event, emit) async { emit(GroupMemberState.loading()); + try { final roles = await _repository.getRoles(event.uuid); - print(roles); + if (!roles.contains(event.roleId)) { + emit(GroupMemberState.error('유효하지 않은 역할입니다')); + return; + } await _repository.grantRoleToUser( uuid: event.uuid, targetUuid: event.targetUuid, roleId: event.roleId, ); emit(GroupMemberState.success()); + } catch (e) { + emit(GroupMemberState.error(e.toString())); + } });
42-50:⚠️ Potential issue역할 제거 로직의 안전성 개선이 필요합니다
다음 보안 및 안정성 관련 사항들이 누락되어 있습니다:
- 예외 처리
- 권한 검증
- 역할 유효성 검증
- 사용자 확인 절차
66-72:⚠️ Potential issue에러 상태 추가가 필요합니다
예외 처리를 위한 에러 상태가 누락되어 있습니다. 다음과 같이 에러 상태를 추가하세요:
@freezed class GroupMemberState with _$GroupMemberState { const factory GroupMemberState.initial() = _Initial; const factory GroupMemberState.loading() = _Loading; const factory GroupMemberState.loaded(MemberListEntity list) = _Loaded; const factory GroupMemberState.success() = _Success; + const factory GroupMemberState.error(String message) = _Error; }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.@freezed class GroupMemberState with _$GroupMemberState { const factory GroupMemberState.initial() = _Initial; const factory GroupMemberState.loading() = _Loading; const factory GroupMemberState.loaded(MemberListEntity list) = _Loaded; const factory GroupMemberState.success() = _Success; const factory GroupMemberState.error(String message) = _Error; }lib/app/modules/groups/presentation/pages/group_management_member_page.dart (2)
53-53:⚠️ Potential issue하드코딩된 역할 값을 실제 멤버 역할로 대체하세요
GroupMemberRole.admin이 하드코딩되어 있어 실제 멤버의 역할이 반영되지 않습니다. 다음과 같이 수정하세요:- role: GroupMemberRole.admin, + role: members.list[index].role,
59-63:⚠️ Potential issue역할 변경 로직의 개선이 필요합니다
다음 사항들이 누락되어 있습니다:
- 역할 ID가 하드코딩되어 있습니다
- 변경 전 사용자 확인 절차가 없습니다
- 권한 검증이 없습니다
사용자 경험과 보안을 위해 다음과 같이 개선하세요:
onChanged: (e) async { + final confirmed = await showDialog<bool>( + context: context, + builder: (context) => AlertDialog( + title: Text('역할 변경'), + content: Text('정말로 이 사용자의 역할을 변경하시겠습니까?'), + actions: [ + TextButton( + onPressed: () => Navigator.pop(context, false), + child: Text('취소'), + ), + TextButton( + onPressed: () => Navigator.pop(context, true), + child: Text('확인'), + ), + ], + ), + ); + if (confirmed == true) { context.read<GroupMemberBloc>().add( GroupMemberEvent.grantRoleToUser( - uuid, members.list[index].uuid, 3)); + uuid, members.list[index].uuid, e.roleId)); + } },lib/app/modules/groups/presentation/pages/group_management_invitation_link_page.dart (1)
90-97:⚠️ Potential issue클립보드 작업 개선 필요
클립보드 복사 기능에 다음 사항들이 누락되어 있습니다:
- 복사 성공/실패에 대한 사용자 피드백
- 클립보드 작업 실패 처리
state.maybeWhen( orElse: () => Container(), success: (code) => ZiggleButton.cta( - onPressed: () => Clipboard.setData(ClipboardData(text: code)), + onPressed: () async { + try { + await Clipboard.setData(ClipboardData(text: code)); + ScaffoldMessenger.of(context).showSnackBar( + SnackBar(content: Text(context.t.common.clipboard.success)), + ); + } catch (e) { + ScaffoldMessenger.of(context).showSnackBar( + SnackBar(content: Text(context.t.common.clipboard.error)), + ); + } + }, emphasize: false, child: Text(context.t.group.manage.invite.copy)), ),lib/app/modules/groups/presentation/blocs/group_management_bloc.dart (1)
18-74:⚠️ Potential issue이벤트 핸들러 일관성 개선 필요
- 에러 처리가 일부 이벤트 핸들러에만 구현되어 있습니다.
- _Delete와 _Leave 이벤트에서 최종 상태 방출이 누락되었습니다.
다음과 같이 수정을 제안합니다:
on<_Delete>((event, emit) async { emit(GroupManagementState.loading()); - await _repository.deleteGroup(event.uuid); + try { + await _repository.deleteGroup(event.uuid); + emit(GroupManagementState.done()); + } catch (e) { + emit(GroupManagementState.error(e.toString())); + } }); on<_Leave>((event, emit) async { emit(GroupManagementState.loading()); - await _repository.leaveGroup(event.uuid); + try { + await _repository.leaveGroup(event.uuid); + emit(GroupManagementState.done()); + } catch (e) { + emit(GroupManagementState.error(e.toString())); + } });
🧹 Nitpick comments (9)
lib/app/modules/groups/presentation/pages/group_management_description_page.dart (2)
31-44: 상태 관리 구현을 개선할 수 있습니다.
noop함수 대신 상태 변경의 목적을 더 명확하게 표현하는 것이 좋습니다.다음과 같이 변경해보세요:
- _controller.addListener(() => setState(noop)); + _controller.addListener(() => setState(() { + // 텍스트 변경 시 UI 업데이트 + }));
54-88: UI 상수값을 분리하면 좋겠습니다.매직 넘버와 스타일 값들을 상수로 분리하면 유지보수가 더 쉬워질 것 같습니다.
다음과 같이 클래스 상단에 상수를 정의하는 것을 추천드립니다:
class _GroupManagementDescriptionPageState extends State<GroupManagementDescriptionPage> { static const double _horizontalPadding = 18.0; static const double _verticalPadding = 20.0; static const int _maxLength = 200; static const int _minLines = 7; static const int _maxLines = 10; // ... }lib/app/modules/groups/presentation/pages/group_management_page.dart (1)
41-44: 에러 상태 UI를 개선하세요.현재 에러 메시지가 단순 텍스트로만 표시되고 있습니다. 사용자 경험을 개선하기 위해 다음을 고려해보세요:
- 에러 아이콘 추가
- 재시도 버튼 구현
- 스타일링된 에러 컨테이너 사용
- error: (message) => Text( - '오류 발생: $message', - style: TextStyle(color: Colors.red), - ), + error: (message) => Center( + child: Column( + mainAxisAlignment: MainAxisAlignment.center, + children: [ + Icon(Icons.error_outline, color: Palette.primary, size: 48), + const SizedBox(height: 16), + Text( + '오류 발생: $message', + style: TextStyle(color: Palette.primary), + ), + const SizedBox(height: 16), + ZiggleButton.cta( + onPressed: () => context + .read<GroupManagementBloc>() + .add(GroupManagementEvent.load()), + child: Text('다시 시도'), + ), + ], + ), + ),lib/app/modules/groups/presentation/pages/group_management_main_page.dart (3)
17-28: 코드 구조가 개선되었습니다.레이아웃 로직을 별도의
_Layout클래스로 분리한 것은 좋은 접근입니다. 하지만 클래스의 목적과 책임을 명확히 하기 위해 문서화를 추가하면 좋겠습니다.다음과 같이 클래스 문서화를 추가하는 것을 제안합니다:
@RoutePage() +/// 그룹 관리 메인 페이지 +/// +/// BlocProvider를 통해 GroupManagementMainBloc을 제공하고 +/// _Layout 위젯을 통해 UI를 구성합니다. class GroupManagementMainPage extends StatelessWidget {
30-32: 위젯 문서화가 필요합니다.private 클래스이지만 복잡한 레이아웃 로직을 포함하고 있으므로, 문서화를 통해 클래스의 역할과 책임을 명확히 하면 좋겠습니다.
+/// 그룹 관리 페이지의 레이아웃을 담당하는 위젯 +/// +/// 그룹 목록 표시, 새 그룹 생성 버튼, 로딩 상태 등을 처리합니다. class _Layout extends StatelessWidget {
73-129: 상태 처리 로직을 개선할 수 있습니다.BlocBuilder 내부의 상태 처리 로직이 복잡합니다. 각 상태별 위젯을 별도의 컴포넌트로 분리하면 코드의 가독성과 유지보수성이 향상될 것 같습니다.
다음과 같이 상태별 위젯을 분리하는 것을 제안합니다:
class _LoadingWidget extends StatelessWidget { @override Widget build(BuildContext context) { return Center( child: Lottie.asset( Assets.lotties.loading, height: MediaQuery.of(context).size.width * 0.2, width: MediaQuery.of(context).size.width * 0.2, ), ); } } class _LoadedWidget extends StatelessWidget { final Groups groups; const _LoadedWidget({required this.groups}); @override Widget build(BuildContext context) { return Expanded( child: ListView.separated( itemCount: groups.list.length + 1, itemBuilder: (context, index) { if (index == groups.list.length) { return const Padding( padding: EdgeInsets.fromLTRB(0, 15, 0, 25), child: _InquiryWidget(), ); } return GroupListItem( name: groups.list[index].name, onPressed: () { context.router.push( GroupManagementShellRoute( group: groups.list[index], ), ); }, ); }, separatorBuilder: (context, index) => const SizedBox(height: 5), ), ); } } class _ErrorWidget extends StatelessWidget { final String message; const _ErrorWidget({required this.message}); @override Widget build(BuildContext context) { return Column( children: [ Assets.images.bonfire.svg(), const SizedBox(height: 20), Text( context.t.group.managementMain.noGroup, textAlign: TextAlign.center, style: const TextStyle( color: Palette.grayText, fontSize: 20, fontWeight: FontWeight.w600, ), ), const SizedBox(height: 20), const _InquiryWidget(), ], ); } }lib/app/modules/groups/domain/repository/group_repository.dart (1)
11-70: 인터페이스 문서화 개선이 필요합니다.각 메서드의 기능, 매개변수, 반환 값, 발생 가능한 예외 등에 대한 문서화가 누락되어 있습니다. 인터페이스의 사용성과 유지보수성을 높이기 위해 문서화를 추가하는 것이 좋습니다.
예시:
/// 새로운 그룹을 생성합니다. /// /// [name] 그룹의 이름 /// [image] 그룹의 프로필 이미지 (선택사항) /// [description] 그룹의 설명 /// [notionPageId] 연결된 Notion 페이지 ID (선택사항) /// /// 성공 시 생성된 [GroupEntity]를 반환합니다. /// 실패 시 [GroupRepositoryException]을 발생시킵니다. Future<GroupEntity> createGroup({ required String name, File? image, required String description, String? notionPageId, });lib/app/modules/groups/presentation/blocs/group_member_bloc.dart (1)
34-34: 디버그 print 문을 제거하세요프로덕션 코드에 디버그 print 문이 남아있습니다. 제거하거나 적절한 로깅 시스템으로 대체하세요.
lib/app/modules/groups/presentation/pages/group_management_name_page.dart (1)
69-72: 입력 유효성 검사 추가 고려텍스트 입력에 대한 유효성 검사(최대 길이, 특수문자 제한 등)를 추가하는 것이 좋을 것 같습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
assets/icons/lock.svgis excluded by!**/*.svg
📒 Files selected for processing (40)
assets/i18n/strings.i18n.json(2 hunks)assets/i18n/strings_jp.i18n.json(2 hunks)assets/i18n/strings_ko.i18n.json(2 hunks)assets/i18n/strings_ru.i18n.json(2 hunks)lib/app/modules/common/presentation/widgets/ziggle_row_button.dart(3 hunks)lib/app/modules/groups/data/data_sources/models/group_invite_code_response_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/models/group_item_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/models/group_list_model.dart(2 hunks)lib/app/modules/groups/data/data_sources/models/group_model.dart(2 hunks)lib/app/modules/groups/data/data_sources/models/member_list_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/models/member_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/models/modify_group_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/models/president_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/models/role_list_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/models/role_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/remote/group_api.dart(4 hunks)lib/app/modules/groups/data/repositories/rest_group_repository.dart(1 hunks)lib/app/modules/groups/domain/entities/authority_entity.dart(1 hunks)lib/app/modules/groups/domain/entities/group_entity.dart(1 hunks)lib/app/modules/groups/domain/entities/member_entity.dart(1 hunks)lib/app/modules/groups/domain/entities/member_list_entity.dart(1 hunks)lib/app/modules/groups/domain/entities/president_entity.dart(1 hunks)lib/app/modules/groups/domain/entities/role_list_entity.dart(1 hunks)lib/app/modules/groups/domain/repository/group_repository.dart(1 hunks)lib/app/modules/groups/domain/repository/group_role_repository.dart(1 hunks)lib/app/modules/groups/presentation/blocs/group_invite_bloc.dart(1 hunks)lib/app/modules/groups/presentation/blocs/group_management_bloc.dart(1 hunks)lib/app/modules/groups/presentation/blocs/group_management_main_bloc.dart(2 hunks)lib/app/modules/groups/presentation/blocs/group_member_bloc.dart(1 hunks)lib/app/modules/groups/presentation/layouts/group_management_shell_layout.dart(1 hunks)lib/app/modules/groups/presentation/pages/group_management_description_page.dart(3 hunks)lib/app/modules/groups/presentation/pages/group_management_invitation_link_page.dart(2 hunks)lib/app/modules/groups/presentation/pages/group_management_main_page.dart(1 hunks)lib/app/modules/groups/presentation/pages/group_management_member_page.dart(1 hunks)lib/app/modules/groups/presentation/pages/group_management_name_page.dart(2 hunks)lib/app/modules/groups/presentation/pages/group_management_notion_page.dart(2 hunks)lib/app/modules/groups/presentation/pages/group_management_page.dart(2 hunks)lib/app/modules/user/presentation/bloc/group_auth_bloc.dart(1 hunks)lib/app/modules/user/presentation/pages/profile_page.dart(3 hunks)lib/app/router.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (28)
- lib/app/modules/groups/domain/repository/group_role_repository.dart
- lib/app/modules/groups/domain/entities/member_entity.dart
- lib/app/modules/user/presentation/bloc/group_auth_bloc.dart
- lib/app/modules/groups/data/data_sources/models/role_list_model.dart
- lib/app/modules/groups/domain/entities/authority_entity.dart
- lib/app/modules/groups/domain/entities/member_list_entity.dart
- lib/app/modules/groups/data/data_sources/models/group_list_model.dart
- lib/app/modules/groups/domain/entities/role_list_entity.dart
- lib/app/modules/groups/data/data_sources/models/member_model.dart
- lib/app/modules/groups/data/data_sources/models/role_model.dart
- lib/app/modules/groups/data/data_sources/models/group_invite_code_response_model.dart
- lib/app/modules/groups/domain/entities/president_entity.dart
- lib/app/modules/user/presentation/pages/profile_page.dart
- lib/app/modules/groups/data/data_sources/models/member_list_model.dart
- lib/app/modules/groups/presentation/blocs/group_management_main_bloc.dart
- lib/app/modules/groups/data/data_sources/models/group_item_model.dart
- lib/app/modules/groups/presentation/layouts/group_management_shell_layout.dart
- lib/app/modules/common/presentation/widgets/ziggle_row_button.dart
- assets/i18n/strings_ru.i18n.json
- assets/i18n/strings_jp.i18n.json
- lib/app/modules/groups/presentation/blocs/group_invite_bloc.dart
- lib/app/modules/groups/domain/entities/group_entity.dart
- lib/app/modules/groups/presentation/pages/group_management_notion_page.dart
- lib/app/modules/groups/data/data_sources/models/group_model.dart
- assets/i18n/strings.i18n.json
- lib/app/router.dart
- assets/i18n/strings_ko.i18n.json
- lib/app/modules/groups/data/data_sources/models/president_model.dart
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build iOS App and Test
- GitHub Check: Build Android App and Test
🔇 Additional comments (15)
lib/app/modules/groups/presentation/pages/group_management_description_page.dart (2)
15-22: 생성자와 필드 선언이 잘 구현되었습니다!불변 필드를
final로 선언하고, 필수 매개변수에required키워드를 사용한 것이 좋습니다.
91-105: 이벤트 처리에 오류 처리를 추가하면 좋겠습니다.설명 업데이트 실패 시의 오류 처리가 없습니다. 사용자에게 적절한 피드백을 제공하는 것이 좋습니다.
다음 사항들을 확인해주세요:
- 설명 업데이트 실패 시 오류 처리
- 네트워크 연결 상태 확인
- 사용자에게 적절한 피드백 제공
lib/app/modules/groups/presentation/pages/group_management_page.dart (3)
1-17: LGTM! 필요한 모든 패키지가 잘 임포트되어 있습니다.필요한 기능을 위한 적절한 패키지들이 잘 구성되어 있습니다.
89-105: 이미지 선택 로직에 검증 및 오류 처리를 추가하세요.이전 리뷰에서 지적된 이미지 선택 로직의 개선사항이 아직 반영되지 않았습니다.
150-196: 그룹 삭제/탈퇴 작업의 상태 처리가 미흡합니다.이전 리뷰에서 지적된 그룹 삭제/탈퇴 작업의 상태 처리 개선사항이 아직 반영되지 않았습니다.
lib/app/modules/groups/presentation/pages/group_management_main_page.dart (1)
138-165: 구현이 적절합니다.문의 위젯이 잘 구현되어 있습니다. 레이아웃과 스타일링이 일관성 있게 적용되어 있습니다.
lib/app/modules/groups/data/data_sources/models/modify_group_model.dart (1)
8-12: LGTM! 모델 구현이 개선되었습니다.명명된 매개변수와 null 값 제외 처리를 통해 더 유연하고 효율적인 구현이 되었습니다.
lib/app/modules/groups/data/data_sources/remote/group_api.dart (1)
25-27: API 응답 타입이 개선되었습니다.
createGroup과createInviteCode메서드의 반환 타입이 더 구체적인 모델 타입으로 변경되어 타입 안전성이 향상되었습니다.Also applies to: 57-60
lib/app/modules/groups/data/repositories/rest_group_repository.dart (2)
22-23: 스트림 리소스 관리가 필요합니다.
BehaviorSubject는 메모리 누수를 방지하기 위해 dispose 되어야 합니다.
71-81: 상태 갱신의 일관성이 필요합니다.
modifyDescription과modifyNotionLink메서드에서_refreshGroups()가 호출되지 않아 데이터 일관성 문제가 발생할 수 있습니다.lib/app/modules/groups/presentation/pages/group_management_invitation_link_page.dart (2)
17-19: 필수 매개변수 추가가 적절히 구현되었습니다.
uuid매개변수가required키워드와 함께 올바르게 추가되었으며,final키워드를 사용하여 불변성이 보장되었습니다.
32-34: Bloc 생명주기 관리 확인 필요
BlocProvider가 올바르게 구현되었지만, Bloc의 적절한 dispose 처리가 되고 있는지 확인이 필요합니다.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check if GroupInviteBloc implements proper disposal ast-grep --pattern 'class GroupInviteBloc extends Bloc { $$$ @override Future<void> close() { $$$ } $$$ }'Length of output: 122
🏁 Script executed:
#!/bin/bash # Step 1: Locate the file where GroupInviteBloc is defined rg -l "class GroupInviteBloc"Length of output: 96
🏁 Script executed:
#!/bin/bash # Step 2: Verify that GroupInviteBloc properly implements the close method and calls super.close() echo ">> Checking for the overridden close method in GroupInviteBloc:" rg -n "Future<void> close\(" lib/app/modules/groups/presentation/blocs/group_invite_bloc.dart echo ">> Checking if super.close() is called within GroupInviteBloc:" rg -n "super\.close\(" lib/app/modules/groups/presentation/blocs/group_invite_bloc.dartLength of output: 457
🏁 Script executed:
#!/bin/bash # 추가 확인: GroupInviteBloc에 StreamSubscription 사용 여부 확인 rg -n "StreamSubscription" lib/app/modules/groups/presentation/blocs/group_invite_bloc.dartLength of output: 91
Bloc 생명주기 관리 재검증 결과
- GroupInviteBloc 파일(lib/app/modules/groups/presentation/blocs/group_invite_bloc.dart) 내에서 close() 메소드 오버라이딩(및 super.close() 호출)이나 추가적인 StreamSubscription과 같은 리소스 관리를 나타내는 코드가 확인되지 않았습니다.
- Bloc 내에서 별도의 리소스 관리가 이루어지지 않는 경우, BlocProvider가 제공하는 기본 dispose 로직으로도 충분하므로 별도의 close() 오버라이딩 없이 자동으로 관리됩니다.
따라서 bloc dispose 처리에 대한 우려는 해소되었으며, 현재 구현 방식으로도 문제가 없어 보입니다.
lib/app/modules/groups/presentation/pages/group_management_name_page.dart (2)
14-27: StatefulWidget 전환이 적절히 구현되었습니다.TextEditingController를 관리하기 위한 StatefulWidget 전환이 잘 이루어졌습니다.
29-43: 상태 관리가 올바르게 구현되었습니다.
- TextEditingController가 적절히 초기화되었습니다.
- initState에서 리스너 추가가 잘 되어있습니다.
- dispose에서 컨트롤러 정리가 잘 이루어졌습니다.
lib/app/modules/groups/presentation/blocs/group_management_bloc.dart (1)
101-111: 상태 정의가 잘 구현되었습니다.freezed를 사용한 상태 정의가 완벽하게 구현되었으며, 필요한 모든 상태가 포함되어 있습니다.
| @override | ||
| Future<void> grantRoleToUser( | ||
| {required String uuid, required String targetUuid, required int roleId}) { | ||
| // TODO: implement grantRoleToUser | ||
| throw UnimplementedError(); | ||
| } |
There was a problem hiding this comment.
미구현된 메서드들의 구현이 필요합니다.
다음 메서드들이 아직 구현되지 않았습니다:
- grantRoleToUser
- leaveGroup
- removeRoleFromUser
- createRole
- deleteRole
- updateRole
이는 그룹 관리의 핵심 기능들입니다.
이러한 메서드들의 구현을 도와드릴까요?
Also applies to: 108-112, 120-125, 127-131, 133-137, 144-149
| ZiggleSelect( | ||
| value: _duration, | ||
| onChanged: (v) { | ||
| setState(() => _duration = v); | ||
| context.read<GroupInviteBloc>().add( | ||
| GroupInviteEvent.create(widget.uuid, v!), | ||
| ); | ||
| }, | ||
| hintText: | ||
| context.t.group.creation.done.invite.selectExpire, | ||
| entries: [ | ||
| ZiggleSelectEntry( | ||
| value: 1, | ||
| label: context.t.common.duration.day(n: 1), | ||
| ), | ||
| ZiggleSelectEntry( | ||
| value: 3, | ||
| label: context.t.common.duration.day(n: 3), | ||
| ), | ||
| ZiggleSelectEntry( | ||
| value: 7, | ||
| label: context.t.common.duration.week(n: 1), | ||
| ), | ||
| ], | ||
| ), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
상태 변경 에러 처리 개선 필요
기간 선택 시 상태 변경과 이벤트 발생이 구현되어 있지만, 에러 처리가 누락되어 있습니다. 다음과 같은 개선이 필요합니다:
- 상태 변경 실패 시 에러 처리
- 이벤트 발생 실패 시 사용자 피드백
onChanged: (v) {
+ try {
setState(() => _duration = v);
context.read<GroupInviteBloc>().add(
GroupInviteEvent.create(widget.uuid, v!),
);
+ } catch (e) {
+ ScaffoldMessenger.of(context).showSnackBar(
+ SnackBar(content: Text(context.t.common.error)),
+ );
+ }
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ZiggleSelect( | |
| value: _duration, | |
| onChanged: (v) { | |
| setState(() => _duration = v); | |
| context.read<GroupInviteBloc>().add( | |
| GroupInviteEvent.create(widget.uuid, v!), | |
| ); | |
| }, | |
| hintText: | |
| context.t.group.creation.done.invite.selectExpire, | |
| entries: [ | |
| ZiggleSelectEntry( | |
| value: 1, | |
| label: context.t.common.duration.day(n: 1), | |
| ), | |
| ZiggleSelectEntry( | |
| value: 3, | |
| label: context.t.common.duration.day(n: 3), | |
| ), | |
| ZiggleSelectEntry( | |
| value: 7, | |
| label: context.t.common.duration.week(n: 1), | |
| ), | |
| ], | |
| ), | |
| ZiggleSelect( | |
| value: _duration, | |
| onChanged: (v) { | |
| try { | |
| setState(() => _duration = v); | |
| context.read<GroupInviteBloc>().add( | |
| GroupInviteEvent.create(widget.uuid, v!), | |
| ); | |
| } catch (e) { | |
| ScaffoldMessenger.of(context).showSnackBar( | |
| SnackBar(content: Text(context.t.common.error)), | |
| ); | |
| } | |
| }, | |
| hintText: | |
| context.t.group.creation.done.invite.selectExpire, | |
| entries: [ | |
| ZiggleSelectEntry( | |
| value: 1, | |
| label: context.t.common.duration.day(n: 1), | |
| ), | |
| ZiggleSelectEntry( | |
| value: 3, | |
| label: context.t.common.duration.day(n: 3), | |
| ), | |
| ZiggleSelectEntry( | |
| value: 7, | |
| label: context.t.common.duration.week(n: 1), | |
| ), | |
| ], | |
| ), |
| BlocBuilder<GroupManagementBloc, GroupManagementState>( | ||
| builder: (context, state) { | ||
| return ZiggleButton.cta( | ||
| disabled: _controller.text.isEmpty || | ||
| _controller.text == widget.name, | ||
| onPressed: () { | ||
| context | ||
| .read<GroupManagementBloc>() | ||
| .add(GroupManagementEvent.updateName( | ||
| widget.uuid, | ||
| _controller.text, | ||
| )); | ||
| context.router.maybePop(); | ||
| }, | ||
| child: Text(context.t.group.manage.change), | ||
| ); | ||
| }, | ||
| ), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
로딩 상태 처리 추가 필요
상태 업데이트 중에 사용자 입력을 방지하고 로딩 표시기를 보여주는 것이 좋을 것 같습니다.
다음과 같이 수정을 제안합니다:
BlocBuilder<GroupManagementBloc, GroupManagementState>(
builder: (context, state) {
return ZiggleButton.cta(
- disabled: _controller.text.isEmpty ||
- _controller.text == widget.name,
+ disabled: state.maybeWhen(
+ loading: () => true,
+ orElse: () => _controller.text.isEmpty ||
+ _controller.text == widget.name,
+ ),
onPressed: () {
context
.read<GroupManagementBloc>()
.add(GroupManagementEvent.updateName(
widget.uuid,
_controller.text,
));
- context.router.maybePop();
+ state.maybeWhen(
+ success: (_) => context.router.maybePop(),
+ orElse: () => null,
+ );
},
- child: Text(context.t.group.manage.change),
+ child: state.maybeWhen(
+ loading: () => CircularProgressIndicator(),
+ orElse: () => Text(context.t.group.manage.change),
+ ),
);
},
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| BlocBuilder<GroupManagementBloc, GroupManagementState>( | |
| builder: (context, state) { | |
| return ZiggleButton.cta( | |
| disabled: _controller.text.isEmpty || | |
| _controller.text == widget.name, | |
| onPressed: () { | |
| context | |
| .read<GroupManagementBloc>() | |
| .add(GroupManagementEvent.updateName( | |
| widget.uuid, | |
| _controller.text, | |
| )); | |
| context.router.maybePop(); | |
| }, | |
| child: Text(context.t.group.manage.change), | |
| ); | |
| }, | |
| ), | |
| BlocBuilder<GroupManagementBloc, GroupManagementState>( | |
| builder: (context, state) { | |
| return ZiggleButton.cta( | |
| disabled: state.maybeWhen( | |
| loading: () => true, | |
| orElse: () => _controller.text.isEmpty || | |
| _controller.text == widget.name, | |
| ), | |
| onPressed: () { | |
| context | |
| .read<GroupManagementBloc>() | |
| .add(GroupManagementEvent.updateName( | |
| widget.uuid, | |
| _controller.text, | |
| )); | |
| state.maybeWhen( | |
| success: (_) => context.router.maybePop(), | |
| orElse: () => null, | |
| ); | |
| }, | |
| child: state.maybeWhen( | |
| loading: () => CircularProgressIndicator(), | |
| orElse: () => Text(context.t.group.manage.change), | |
| ), | |
| ); | |
| }, | |
| ), |
| const factory GroupManagementEvent.getMembers(String uuid) = _GetMembers; | ||
| const factory GroupManagementEvent.removeMember( | ||
| String uuid, String targetUuid) = _RemoveMember; | ||
| const factory GroupManagementEvent.grantRoleToUser( | ||
| String uuid, String targetUuid, int roleId) = _GrantRoleToUser; | ||
| const factory GroupManagementEvent.removeRoleFromUser( | ||
| String uuid, String targetUuid, int roleId) = _RemoveRoleFromUser; |
There was a problem hiding this comment.
미구현된 이벤트 핸들러 구현 필요
다음 이벤트들에 대한 핸들러가 구현되지 않았습니다:
- _GetMembers
- _GrantRoleToUser
- _RemoveRoleFromUser
이는 그룹 멤버 관리 기능이 완전히 구현되지 않았음을 의미합니다.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
lib/app/modules/groups/presentation/pages/group_management_notion_page.dart (1)
157-162: 오류 처리가 누락되었습니다.그룹 관리 블록 이벤트 처리 시 오류 처리가 없습니다.
다음과 같이 오류 처리를 추가하세요:
onPressed: () { + try { context.read<GroupManagementBloc>().add( GroupManagementEvent.updateNotionLink( widget.uuid, _controller.text)); - context.router.maybePop(); + if (mounted) context.router.maybePop(); + } catch (e) { + ScaffoldMessenger.of(context).showSnackBar( + SnackBar(content: Text(context.t.common.error.unknown)), + ); + } },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.onPressed: () { try { context.read<GroupManagementBloc>().add( GroupManagementEvent.updateNotionLink( widget.uuid, _controller.text)); if (mounted) context.router.maybePop(); } catch (e) { ScaffoldMessenger.of(context).showSnackBar( SnackBar(content: Text(context.t.common.error.unknown)), ); } },
🧹 Nitpick comments (2)
lib/app/modules/groups/presentation/pages/group_management_notion_page.dart (1)
51-56: 디바운스 타임이 너무 깁니다.현재 1초의 디바운스 시간은 사용자 입력에 대한 반응이 느리게 느껴질 수 있습니다.
디바운스 시간을 줄이는 것을 고려하세요:
- _subject.debounceTime(const Duration(seconds: 1)).listen((notionLink) { + _subject.debounceTime(const Duration(milliseconds: 500)).listen((notionLink) {assets/i18n/strings_jp.i18n.json (1)
86-87: 로딩 메시지가 일관성이 없습니다.
loading메시지가 다른 로딩 메시지와 스타일이 다릅니다.다음과 같이 메시지 스타일을 통일하세요:
- "loading": "ノーションを読み込んでいます", + "loading": "ノーションを読み込み中...",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
assets/i18n/strings.i18n.json(2 hunks)assets/i18n/strings_jp.i18n.json(2 hunks)assets/i18n/strings_ko.i18n.json(2 hunks)assets/i18n/strings_ru.i18n.json(2 hunks)lib/app/modules/groups/presentation/blocs/notion_bloc.dart(2 hunks)lib/app/modules/groups/presentation/pages/group_management_notion_page.dart(1 hunks)lib/app/modules/notices/presentation/pages/feed_page.dart(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- assets/i18n/strings.i18n.json
- assets/i18n/strings_ko.i18n.json
- assets/i18n/strings_ru.i18n.json
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build Android App and Test
- GitHub Check: Build iOS App and Test
| RegExp regex = RegExp(r'([a-f0-9]{32})'); | ||
| Match? notionId = regex.firstMatch(notionLink); | ||
| if (notionId != null) { | ||
| final data = await _repository.getGroups(notionId.group(0)!); | ||
| emit(NotionState.done(data)); | ||
| } else { | ||
| emit(NotionState.error(t.group.manage.notionLink.error)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
정규식 패턴이 너무 단순합니다.
현재 정규식은 32자리 16진수만 확인하고 있습니다. Notion 링크의 다양한 형식을 고려하여 패턴을 개선해야 합니다.
다음과 같이 정규식을 개선하세요:
- RegExp regex = RegExp(r'([a-f0-9]{32})');
+ RegExp regex = RegExp(r'(?:notion\.so/)?(?:[^/]+/)?([a-f0-9]{32})');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| RegExp regex = RegExp(r'([a-f0-9]{32})'); | |
| Match? notionId = regex.firstMatch(notionLink); | |
| if (notionId != null) { | |
| final data = await _repository.getGroups(notionId.group(0)!); | |
| emit(NotionState.done(data)); | |
| } else { | |
| emit(NotionState.error(t.group.manage.notionLink.error)); | |
| } | |
| RegExp regex = RegExp(r'(?:notion\.so/)?(?:[^/]+/)?([a-f0-9]{32})'); | |
| Match? notionId = regex.firstMatch(notionLink); | |
| if (notionId != null) { | |
| final data = await _repository.getGroups(notionId.group(0)!); | |
| emit(NotionState.done(data)); | |
| } else { | |
| emit(NotionState.error(t.group.manage.notionLink.error)); | |
| } |
| onTapSearch: () { | ||
| AnalyticsRepository.click( | ||
| const AnalyticsEvent.search(PageSource.feed)); | ||
| const SearchRoute().push(context); | ||
| GroupManagementMainRoute().push(context); | ||
| // AnalyticsRepository.click( | ||
| // const AnalyticsEvent.search(PageSource.feed)); | ||
| // const SearchRoute().push(context); | ||
| }, |
There was a problem hiding this comment.
검색 기능이 그룹 관리로 대체되었습니다.
검색 기능을 제거하고 그룹 관리로 대체하는 것은 사용자 경험을 저하시킬 수 있습니다. 두 기능을 모두 제공하는 것이 좋습니다.
검색 기능을 복원하고 그룹 관리를 별도의 버튼으로 추가하는 것을 권장합니다.
| onTapWrite: () { | ||
| AnalyticsRepository.click( | ||
| const AnalyticsEvent.write(PageSource.feed)); | ||
| if (UserBloc.userOrNull(context) == null) { | ||
| return context.showToast( | ||
| context.t.user.login.description, | ||
| ); | ||
| } | ||
| const NoticeWriteBodyRoute().push(context); | ||
| context.read<GroupAuthBloc>().add(GroupAuthEvent.login()); | ||
| // AnalyticsRepository.click( | ||
| // const AnalyticsEvent.write(PageSource.feed)); | ||
| // if (UserBloc.userOrNull(context) == null) { | ||
| // return context.showToast( | ||
| // context.t.user.login.description, | ||
| // ); | ||
| // } | ||
| // const NoticeWriteBodyRoute().push(context); | ||
| }, |
There was a problem hiding this comment.
분석 추적이 제거되었습니다.
사용자 행동 분석을 위한 중요한 분석 추적 코드가 제거되었습니다.
분석 추적을 복원하되, 그룹 관리 관련 이벤트도 추가하세요:
+ AnalyticsRepository.click(
+ const AnalyticsEvent.groupManagement(PageSource.feed));
context.read<GroupAuthBloc>().add(GroupAuthEvent.login());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onTapWrite: () { | |
| AnalyticsRepository.click( | |
| const AnalyticsEvent.write(PageSource.feed)); | |
| if (UserBloc.userOrNull(context) == null) { | |
| return context.showToast( | |
| context.t.user.login.description, | |
| ); | |
| } | |
| const NoticeWriteBodyRoute().push(context); | |
| context.read<GroupAuthBloc>().add(GroupAuthEvent.login()); | |
| // AnalyticsRepository.click( | |
| // const AnalyticsEvent.write(PageSource.feed)); | |
| // if (UserBloc.userOrNull(context) == null) { | |
| // return context.showToast( | |
| // context.t.user.login.description, | |
| // ); | |
| // } | |
| // const NoticeWriteBodyRoute().push(context); | |
| }, | |
| onTapWrite: () { | |
| AnalyticsRepository.click( | |
| const AnalyticsEvent.groupManagement(PageSource.feed)); | |
| context.read<GroupAuthBloc>().add(GroupAuthEvent.login()); | |
| // AnalyticsRepository.click( | |
| // const AnalyticsEvent.write(PageSource.feed)); | |
| // if (UserBloc.userOrNull(context) == null) { | |
| // return context.showToast( | |
| // context.t.user.login.description, | |
| // ); | |
| // } | |
| // const NoticeWriteBodyRoute().push(context); | |
| }, |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
lib/app/modules/groups/data/repositories/rest_group_repository.dart (4)
18-23: 의존성 주입 및 상태 관리 방식이 개선되었습니다.
@Injectable에서@Singleton으로 변경되어 인스턴스 생명주기가 더 명확해졌습니다. 하지만BehaviorSubject의 리소스 관리가 필요합니다.다음과 같이
dispose메서드를 추가해주세요:@override void dispose() { _groupsSubject.close(); }
28-39: 에러 처리가 누락되었습니다.그룹 생성 및 데이터 갱신 과정에서 발생할 수 있는 예외 상황에 대한 처리가 필요합니다.
다음과 같이 에러 처리를 추가해주세요:
Future<GroupEntity> createGroup({ required String name, File? image, required String description, String? notionPageId, }) async { try { final createdGroup = await _api.createGroup(CreateGroupModel( name: name, description: description, notionPageId: notionPageId )); if (image != null) { try { await _api.uploadImage(createdGroup.uuid, image); } catch (e) { print('이미지 업로드 실패: $e'); } } await _refreshGroups(); return createdGroup; } catch (e) { throw GroupRepositoryException('그룹 생성 실패: $e'); } } Future<void> _refreshGroups() async { try { final newList = await _api.getGroups(); _groupsSubject.add(newList); } catch (e) { throw GroupRepositoryException('그룹 목록 갱신 실패: $e'); } }Also applies to: 41-44
108-111: 미구현된 메서드들의 구현이 필요합니다.다음 메서드들이 아직 구현되지 않았습니다:
- leaveGroup
- removeRoleFromUser
- createRole
- deleteRole
- updateRole
구현이 필요한 메서드들의 예시 코드를 제공해드릴까요?
Also applies to: 120-124, 127-130, 133-136, 144-148
71-81: 상태 갱신의 일관성이 필요합니다.
modifyDescription과modifyNotionLink메서드에서_refreshGroups()가 호출되지 않아 데이터 일관성 문제가 발생할 수 있습니다.다음과 같이 수정해주세요:
Future<void> modifyDescription( {required String uuid, required String? description}) async { await _api.modifyGroup(uuid, ModifyGroupModel(description: description)); + await _refreshGroups(); } Future<void> modifyNotionLink( {required String uuid, required String? notionPageId}) async { await _api.modifyGroup(uuid, ModifyGroupModel(notionPageId: notionPageId)); + await _refreshGroups(); }
🧹 Nitpick comments (3)
lib/app/modules/groups/data/enums/group_member_role.dart (2)
5-26: 각 역할에 대한 문서화가 필요합니다.각 역할(
admin,manager,user)의 권한과 책임에 대한 설명을 추가하면 코드의 가독성과 유지보수성이 향상될 것입니다.예시:
/// 그룹 멤버의 역할을 정의하는 열거형 /// /// [admin] - 모든 권한을 가진 최고 관리자 /// [manager] - 일부 관리 권한을 가진 중간 관리자 /// [user] - 기본 사용자 권한 @JsonEnum(alwaysCreate: true) enum GroupMemberRole { ... }
16-25: 역할 값을 상수로 정의하는 것이 좋습니다.
toInt()메서드의 매직 넘버를 상수로 정의하면 코드의 유지보수성이 향상될 것입니다.예시:
enum GroupMemberRole { @JsonValue('admin') admin, @JsonValue('manager') manager, @JsonValue('user') user; static const adminValue = 1; static const managerValue = 2; static const userValue = 3; int toInt() { switch (this) { case GroupMemberRole.admin: return adminValue; case GroupMemberRole.manager: return managerValue; case GroupMemberRole.user: return userValue; } } }lib/app/modules/groups/presentation/pages/group_management_member_page.dart (1)
17-19: 문서화가 필요합니다.클래스와 필드에 대한 문서 주석을 추가하여 코드의 가독성과 유지보수성을 향상시키세요.
다음과 같이 문서 주석을 추가하세요:
+/// 그룹 멤버 관리 페이지 class GroupManagementMemberPage extends StatelessWidget { const GroupManagementMemberPage({super.key, required this.uuid}); + /// 그룹의 고유 식별자 final String uuid;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
lib/app/modules/groups/data/data_sources/models/member_list_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/models/member_model.dart(1 hunks)lib/app/modules/groups/data/data_sources/remote/group_api.dart(5 hunks)lib/app/modules/groups/data/enums/group_member_role.dart(1 hunks)lib/app/modules/groups/data/repositories/rest_group_repository.dart(1 hunks)lib/app/modules/groups/domain/entities/member_entity.dart(1 hunks)lib/app/modules/groups/presentation/blocs/group_member_bloc.dart(1 hunks)lib/app/modules/groups/presentation/pages/group_management_member_page.dart(1 hunks)lib/app/modules/groups/presentation/widgets/group_member_card.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- lib/app/modules/groups/data/data_sources/models/member_model.dart
- lib/app/modules/groups/domain/entities/member_entity.dart
- lib/app/modules/groups/presentation/blocs/group_member_bloc.dart
- lib/app/modules/groups/data/data_sources/models/member_list_model.dart
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build Android App and Test
- GitHub Check: Build iOS App and Test
🔇 Additional comments (10)
lib/app/modules/groups/data/enums/group_member_role.dart (1)
28-39: LGTM! 지역화가 잘 구현되어 있습니다.
toLocalizedString메서드가 각 역할에 대한 지역화된 문자열을 적절히 반환하고 있습니다.lib/app/modules/groups/presentation/widgets/group_member_card.dart (1)
4-4: LGTM! 역할 열거형이 적절히 임포트되었습니다.
GroupMemberRole열거형을 별도의 파일로 분리하고 올바르게 임포트하여 코드의 구조가 개선되었습니다.lib/app/modules/groups/data/data_sources/remote/group_api.dart (6)
8-12: 새로운 모델 클래스들이 추가되었습니다.새로운 기능을 지원하기 위해 필요한 모델 클래스들이 적절하게 추가되었습니다.
25-26: 그룹 생성 메서드의 반환 타입이 변경되었습니다.
GroupModel에서GroupItemModel로 반환 타입이 변경되었습니다. 이는 도메인 계층의GroupEntity와 더 잘 정렬됩니다.
58-59: 초대 코드 생성 메서드가 개선되었습니다.
duration매개변수가 추가되어 초대 코드의 유효 기간을 지정할 수 있게 되었습니다. 반환 타입도 더 명확한GroupInviteCodeResponseModel로 변경되었습니다.
61-64: 멤버 목록 조회 기능이 추가되었습니다.그룹의 멤버 목록을 조회하는 새로운 엔드포인트가 추가되었습니다.
79-79: 역할 부여 메서드의 매개변수 타입이 개선되었습니다.
roleId의 타입이String에서int로 변경되었습니다. 이는 데이터베이스의 ID 타입과 더 잘 정렬됩니다.
102-102: 메서드 이름이 더 명확하게 변경되었습니다.
getRole에서getRoles로 메서드 이름이 변경되어 복수의 역할을 반환한다는 의미가 더 명확해졌습니다.lib/app/modules/groups/presentation/pages/group_management_member_page.dart (2)
34-43: 상태 처리 로직이 불완전합니다.BlocBuilder 구현에서 다음 사항들이 누락되어 있습니다:
- 에러 상태 처리
- 빈 목록 상태 처리
- 새로고침 기능
다음과 같이 개선하세요:
return state.maybeWhen( orElse: () => Container(), initial: () => Container(), + error: (message) => Center( + child: Column( + mainAxisAlignment: MainAxisAlignment.center, + children: [ + Text(message), + ElevatedButton( + onPressed: () => context.read<GroupMemberBloc>() + ..add(GroupMemberEvent.getMembers(uuid)), + child: Text('다시 시도'), + ), + ], + ), + ), loading: () => Center( child: Lottie.asset(Assets.lotties.loading,
50-63: 역할 관리 로직이 불완전합니다.
GroupMemberCard.editMode구현에서 다음 사항들이 개선되어야 합니다:
- 역할 변경 시 확인 다이얼로그 추가
- 권한 검증 로직 추가
다음과 같이 개선하세요:
onChanged: (e) { + showDialog( + context: context, + builder: (context) => AlertDialog( + title: Text('역할 변경'), + content: Text('멤버의 역할을 변경하시겠습니까?'), + actions: [ + TextButton( + onPressed: () => Navigator.pop(context), + child: Text('취소'), + ), + TextButton( + onPressed: () { + if (context.read<GroupMemberBloc>().hasPermission()) { context.read<GroupMemberBloc>().add( GroupMemberEvent.grantRoleToUser( uuid, members.list[index].uuid, e!)); + Navigator.pop(context); + } else { + ScaffoldMessenger.of(context).showSnackBar( + SnackBar(content: Text('권한이 없습니다')), + ); + } + }, + child: Text('확인'), + ), + ], + ), + ); },
| onBanish: () { | ||
| context.read<GroupManagementBloc>().add( | ||
| GroupManagementEvent.removeMember( | ||
| uuid, members.list[index].uuid)); | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
멤버 제거 시 확인 절차가 필요합니다.
멤버 제거 기능에 확인 다이얼로그를 추가하여 실수로 인한 제거를 방지하세요.
다음과 같이 개선하세요:
onBanish: () {
+ showDialog(
+ context: context,
+ builder: (context) => AlertDialog(
+ title: Text('멤버 제거'),
+ content: Text('정말로 이 멤버를 제거하시겠습니까?'),
+ actions: [
+ TextButton(
+ onPressed: () => Navigator.pop(context),
+ child: Text('취소'),
+ ),
+ TextButton(
+ onPressed: () {
context.read<GroupManagementBloc>().add(
GroupManagementEvent.removeMember(
uuid, members.list[index].uuid));
+ Navigator.pop(context);
+ },
+ child: Text('확인'),
+ ),
+ ],
+ ),
+ );
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onBanish: () { | |
| context.read<GroupManagementBloc>().add( | |
| GroupManagementEvent.removeMember( | |
| uuid, members.list[index].uuid)); | |
| }, | |
| onBanish: () { | |
| showDialog( | |
| context: context, | |
| builder: (context) => AlertDialog( | |
| title: Text('멤버 제거'), | |
| content: Text('정말로 이 멤버를 제거하시겠습니까?'), | |
| actions: [ | |
| TextButton( | |
| onPressed: () => Navigator.pop(context), | |
| child: Text('취소'), | |
| ), | |
| TextButton( | |
| onPressed: () { | |
| context.read<GroupManagementBloc>().add( | |
| GroupManagementEvent.removeMember( | |
| uuid, members.list[index].uuid)); | |
| Navigator.pop(context); | |
| }, | |
| child: Text('확인'), | |
| ), | |
| ], | |
| ), | |
| ); | |
| }, |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
lib/app/modules/groups/data/enums/group_member_role.dart (2)
16-26: 문서화와 상수 정의를 추가하는 것이 좋겠습니다.메서드의 목적과 각 숫자값의 의미를 명확히 하기 위해 다음과 같은 개선을 제안합니다.
다음과 같이 수정해보세요:
+ /// 역할을 정수값으로 변환합니다. + /// admin: 1, manager: 2, member: 3 + static const int adminValue = 1; + static const int managerValue = 2; + static const int memberValue = 3; + int toInt() { switch (this) { case GroupMemberRole.admin: - return 1; + return adminValue; case GroupMemberRole.manager: - return 2; + return managerValue; case GroupMemberRole.member: - return 3; + return memberValue; } }
28-39: 컨텍스트 null 안전성 검사를 추가하면 좋겠습니다.BuildContext가 null이 아님을 보장하기 위한 검사를 추가하는 것이 안전합니다.
다음과 같이 수정해보세요:
String toLocalizedString(BuildContext context) { + if (context == null) { + throw ArgumentError('BuildContext는 null이 될 수 없습니다.'); + } switch (this) { case GroupMemberRole.admin: return context.t.group.memberCard.role.admin;lib/app/modules/groups/presentation/pages/group_management_member_page.dart (1)
18-20: 생성자에 문서화가 필요합니다.필수 매개변수인
uuid에 대한 설명을 추가하여 코드의 가독성과 유지보수성을 향상시키세요.다음과 같이 문서화를 추가하세요:
+ /// 그룹 관리 멤버 페이지 + /// + /// [uuid] - 관리할 그룹의 고유 식별자 const GroupManagementMemberPage({super.key, required this.uuid});
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
assets/i18n/strings.i18n.json(3 hunks)assets/i18n/strings_jp.i18n.json(3 hunks)assets/i18n/strings_ko.i18n.json(3 hunks)assets/i18n/strings_ru.i18n.json(3 hunks)lib/app/modules/groups/data/enums/group_member_role.dart(1 hunks)lib/app/modules/groups/data/repositories/rest_group_repository.dart(1 hunks)lib/app/modules/groups/presentation/pages/group_management_member_page.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- assets/i18n/strings.i18n.json
- assets/i18n/strings_jp.i18n.json
- assets/i18n/strings_ko.i18n.json
- assets/i18n/strings_ru.i18n.json
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build Android App and Test
- GitHub Check: Build iOS App and Test
🔇 Additional comments (8)
lib/app/modules/groups/data/enums/group_member_role.dart (1)
1-14: LGTM! JSON 직렬화가 잘 구현되어 있습니다.역할 계층과 JSON 직렬화 구현이 명확하고 적절합니다.
lib/app/modules/groups/data/repositories/rest_group_repository.dart (5)
22-23: 리소스 관리 개선 필요
BehaviorSubject의 리소스 누수를 방지하기 위해 dispose 메서드 구현이 필요합니다.@override void dispose() { _groupsSubject.close(); }
28-39: 반환 타입과 에러 처리 개선 필요
- API 응답을 Entity로 변환하지 않고 직접 반환하고 있습니다.
- 네트워크 오류나 이미지 업로드 실패에 대한 예외 처리가 필요합니다.
46-53: 데이터 일관성 개선 필요
getGroups와watchGroups가 서로 다른 데이터 소스를 사용하여 일관성 문제가 발생할 수 있습니다.
71-81: 상태 갱신 일관성 개선 필요
modifyDescription과modifyNotionLink메서드에서_refreshGroups()를 호출하지 않아 데이터 일관성이 깨질 수 있습니다.
108-149: 미구현 메서드 구현 필요다음 메서드들의 구현이 필요합니다:
- leaveGroup
- removeRoleFromUser
- createRole
- deleteRole
- updateRole
구체적인 구현 예시가 필요하시다면 알려주세요. 각 메서드에 대한 상세 구현을 제안해드릴 수 있습니다.
lib/app/modules/groups/presentation/pages/group_management_member_page.dart (2)
24-26: 상태 처리 로직 개선이 필요합니다.BlocBuilder 구현에서 다음 사항들을 개선해야 합니다:
- 에러 상태 처리
- 빈 목록 상태 처리
- 새로고침 기능
69-76: 역할 변경 로직이 불완전합니다.역할 변경 처리에서 다음 사항들이 누락되어 있습니다:
- 역할 변경 전 확인 다이얼로그가 없습니다
- 권한 검증이 누락되어 있습니다
- 실패 처리가 없습니다
| } | ||
|
|
||
| @override | ||
| Future<GroupEntity> getGroup(String uuid) { | ||
| return _api.getGroup(uuid); | ||
| } | ||
|
|
||
| @override | ||
| Future<void> modifyProfileImage({required String uuid, required File image}) { | ||
| return _api.uploadImage(uuid, image); | ||
| } | ||
|
|
||
| @override | ||
| Future<void> modifyName({required String uuid, required String name}) async { | ||
| await _api.modifyGroup(uuid, ModifyGroupModel(name: name)); | ||
| await _refreshGroups(); | ||
| } | ||
|
|
||
| @override | ||
| Future<void> modifyDescription( | ||
| {required String uuid, required String? description}) async { | ||
| await _api.modifyGroup(uuid, ModifyGroupModel(description: description)); | ||
| } | ||
|
|
||
| @override | ||
| Future<void> modifyNotionLink( | ||
| {required String uuid, required String? notionPageId}) async { | ||
| await _api.modifyGroup(uuid, ModifyGroupModel(notionPageId: notionPageId)); | ||
| } | ||
|
|
||
| @override | ||
| Future<void> deleteGroup(String uuid) async { | ||
| await _api.deleteGroup(uuid); | ||
| await _refreshGroups(); | ||
| } | ||
|
|
||
| @override | ||
| Future<String> createInviteLink( | ||
| {required String uuid, required int duration}) async { | ||
| final response = await _api.createInviteCode(uuid, duration); | ||
| return response.code; | ||
| } | ||
|
|
||
| @override | ||
| Future<MemberListEntity> getMembers(String uuid) async { | ||
| return _api.getMembers(uuid); | ||
| } | ||
|
|
||
| @override | ||
| Future<void> grantRoleToUser( | ||
| {required String uuid, required String targetUuid, required int roleId}) { | ||
| return _api.grantUserRole(uuid, targetUuid, roleId); | ||
| } | ||
|
|
||
| @override | ||
| Future<void> leaveGroup(String uuid) { | ||
| // TODO: implement leaveGroup | ||
| throw UnimplementedError(); | ||
| } | ||
|
|
||
| @override | ||
| Future<void> removeMember( | ||
| {required String uuid, required String targetUuid}) async { | ||
| await _api.banishUser(uuid, targetUuid); | ||
| await _refreshGroups(); | ||
| } | ||
|
|
||
| @override | ||
| Future<void> removeRoleFromUser( | ||
| {required String uuid, required String targetUuid, required int roleId}) { | ||
| // TODO: implement removeRoleFromUser | ||
| throw UnimplementedError(); | ||
| } | ||
|
|
||
| @override | ||
| Future<void> createRole(String groupUuid, RoleEntity role) { | ||
| // TODO: implement createRole | ||
| throw UnimplementedError(); | ||
| } | ||
|
|
||
| @override | ||
| Future<void> deleteRole(String groupUuid, int roleId) { | ||
| // TODO: implement deleteRole | ||
| throw UnimplementedError(); | ||
| } | ||
|
|
||
| @override | ||
| Future<RoleListEntity> getRoles(String groupUuid) { | ||
| return _api.getRoles(groupUuid); | ||
| } | ||
|
|
||
| @override | ||
| Future<GroupListModel> getGroups() { | ||
| return _api.getGroups(); | ||
| Future<void> updateRole( | ||
| String groupUuid, int roleId, AuthorityEntity authority) { | ||
| // TODO: implement updateRole | ||
| throw UnimplementedError(); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
전역 에러 처리 전략 필요
리포지토리 전반에 걸쳐 일관된 에러 처리 전략이 필요합니다:
- 비즈니스 예외 정의
- 네트워크 에러 처리
- 데이터 변환 실패 처리
class GroupRepositoryException implements Exception {
final String message;
final String code;
const GroupRepositoryException(this.message, {this.code = 'UNKNOWN'});
static const String NETWORK_ERROR = 'NETWORK_ERROR';
static const String INVALID_DATA = 'INVALID_DATA';
static const String OPERATION_FAILED = 'OPERATION_FAILED';
}| return state.maybeWhen( | ||
| orElse: () => Container(), | ||
| initial: () => Container(), | ||
| loading: () => Center( | ||
| child: Lottie.asset(Assets.lotties.loading, | ||
| height: MediaQuery.of(context).size.width * 0.2, | ||
| width: MediaQuery.of(context).size.width * 0.2), | ||
| ), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
상태별 UI 처리가 미흡합니다.
orElse와 initial 상태에서 빈 컨테이너를 반환하는 것은 사용자 경험을 저하시킵니다. 각 상태에 맞는 적절한 UI를 제공하세요:
- 초기 상태: 안내 메시지
- 오류 상태: 오류 메시지와 재시도 버튼
- 빈 목록: 안내 메시지
| onBanish: () { | ||
| context.showDialog<bool>( | ||
| title: 'title', | ||
| content: 'content', | ||
| onConfirm: (_) { | ||
| context.read<GroupManagementBloc>().add( | ||
| GroupManagementEvent.removeMember( | ||
| uuid, members.list[index].uuid)); | ||
| context | ||
| .read<GroupMemberBloc>() | ||
| .add(GroupMemberEvent.getMembers(uuid)); | ||
| }, | ||
| ); | ||
| }, |
There was a problem hiding this comment.
멤버 제거 다이얼로그의 텍스트가 하드코딩되어 있습니다.
다이얼로그의 제목과 내용이 'title'과 'content'로 하드코딩되어 있습니다. 다국어 지원을 위해 context.t를 사용하세요.
다음과 같이 수정하세요:
- title: 'title',
- content: 'content',
+ title: context.t.group.manage.member.remove.title,
+ content: context.t.group.manage.member.remove.content,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onBanish: () { | |
| context.showDialog<bool>( | |
| title: 'title', | |
| content: 'content', | |
| onConfirm: (_) { | |
| context.read<GroupManagementBloc>().add( | |
| GroupManagementEvent.removeMember( | |
| uuid, members.list[index].uuid)); | |
| context | |
| .read<GroupMemberBloc>() | |
| .add(GroupMemberEvent.getMembers(uuid)); | |
| }, | |
| ); | |
| }, | |
| onBanish: () { | |
| context.showDialog<bool>( | |
| - title: 'title', | |
| - content: 'content', | |
| + title: context.t.group.manage.member.remove.title, | |
| + content: context.t.group.manage.member.remove.content, | |
| onConfirm: (_) { | |
| context.read<GroupManagementBloc>().add( | |
| GroupManagementEvent.removeMember( | |
| uuid, members.list[index].uuid)); | |
| context | |
| .read<GroupMemberBloc>() | |
| .add(GroupMemberEvent.getMembers(uuid)); | |
| }, | |
| ); | |
| }, |
Summary by CodeRabbit
새로운 기능
버그 수정
개선 사항