Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a Club Settings API (interface, controller, service) to manage recruitment, application, and fee toggles; new DTOs for settings; replaces feeDeadline with boolean flags on Club; updates fee DTOs and usages; and adds a DB migration to add toggle columns. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as "ClubSettingsController"
participant Service as "ClubSettingsService"
participant Repo as "ClubRepository/DB"
Client->>Controller: GET /clubs/{clubId}/settings (userId)
Controller->>Service: getSettings(clubId, userId)
Service->>Repo: fetch club by clubId
Repo-->>Service: Club entity
Service-->>Controller: ClubSettingsResponse
Controller-->>Client: 200 OK (response)
Client->>Controller: PATCH /clubs/{clubId}/settings (body, userId)
Controller->>Service: updateSettings(clubId, userId, request)
Service->>Repo: fetch club by clubId
Repo-->>Service: Club entity
Service->>Service: validate & update Club.updateSettings(...)
Service->>Repo: save updated Club
Repo-->>Service: persisted Club
Service-->>Controller: updated ClubSettingsResponse
Controller-->>Client: 200 OK (response)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
동아리 관리 화면(지원서/회비/모집공고)에서 필요한 “설정 토글 + 요약 정보”를 한 번에 조회/수정할 수 있도록 백엔드 API/도메인/마이그레이션을 추가·정리한 PR입니다.
Changes:
- 동아리 설정 조회/수정 API(
/clubs/{clubId}/settings) 및 응답/요청 DTO 추가 - Club 도메인에 설정 토글 필드(모집공고/지원서/회비) 반영 및 회비 설정 API 스펙 일부 변경
- club 테이블에 설정 토글 컬럼 추가 마이그레이션(V29) 추가
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/resources/db/migration/V29__add_club_settings_toggle_columns.sql | club 설정 토글 컬럼(모집공고/지원서) 추가 |
| src/main/java/gg/agit/konect/domain/club/service/ClubSettingsService.java | 설정 조회/수정 서비스 및 요약 정보 조립 로직 추가 |
| src/main/java/gg/agit/konect/domain/club/service/ClubApplicationService.java | 회비 정보 교체 호출 시 불필요 파라미터 제거 |
| src/main/java/gg/agit/konect/domain/club/model/Club.java | 설정 토글 필드 추가 및 회비 정보 처리 로직 조정 |
| src/main/java/gg/agit/konect/domain/club/dto/ClubSettingsUpdateRequest.java | 설정 PATCH 요청 DTO 추가 |
| src/main/java/gg/agit/konect/domain/club/dto/ClubSettingsResponse.java | 설정 화면용 종합 응답 DTO 추가 |
| src/main/java/gg/agit/konect/domain/club/dto/ClubFeeInfoResponse.java | 회비 정보 응답 스펙에서 필드 제거 |
| src/main/java/gg/agit/konect/domain/club/dto/ClubFeeInfoReplaceRequest.java | 회비 정보 교체 요청 스펙에서 필드 제거 |
| src/main/java/gg/agit/konect/domain/club/controller/ClubSettingsController.java | 설정 조회/수정 컨트롤러 구현 추가 |
| src/main/java/gg/agit/konect/domain/club/controller/ClubSettingsApi.java | 설정 조회/수정 API 인터페이스/스웨거 정의 추가 |
| int questionCount = clubApplyQuestionRepository.findAllByClubIdOrderByIdAsc(clubId).size(); | ||
| return new ApplicationSummary(questionCount); |
There was a problem hiding this comment.
문항 개수 계산을 위해 findAllByClubIdOrderByIdAsc(...).size()로 전체 레코드를 조회하고 있어, 문항이 많아질수록 불필요한 I/O가 발생합니다. countByClubId 같은 count 쿼리 메서드를 추가해서 개수만 조회하도록 변경하는 게 좋습니다.
| int questionCount = clubApplyQuestionRepository.findAllByClubIdOrderByIdAsc(clubId).size(); | |
| return new ApplicationSummary(questionCount); | |
| long questionCount = clubApplyQuestionRepository.countByClubId(clubId); | |
| return new ApplicationSummary((int) questionCount); |
| String accountNumber, | ||
|
|
||
| @Schema(description = "예금주", example = "BCSD", requiredMode = REQUIRED) | ||
| String accountHolder, | ||
|
|
||
| @Schema(description = "회비 납부 필요 여부", example = "true", requiredMode = REQUIRED) | ||
| Boolean isFeeRequired | ||
| String accountHolder |
There was a problem hiding this comment.
ClubFeeInfoResponse에서 isFeeRequired 필드가 제거되어 기존 API 응답 스펙을 사용하는 클라이언트가 깨질 수 있습니다. 의도된 변경이라면 API 문서/클라이언트 동시 배포가 필요하고, 호환성이 필요하면 필드를 유지한 채 deprecated 처리 또는 버전 분리(새 응답 DTO)로 전환하는 방안을 검토해주세요.
|
|
||
| @Size(max = 100, message = "예금주는 최대 100자 입니다.") | ||
| @Schema(description = "예금주", example = "BCSD", requiredMode = NOT_REQUIRED) | ||
| String accountHolder, | ||
|
|
||
| @NotNull(message = "회비 납부 필요 여부는 필수로 입력해야 합니다.") | ||
| @Schema(description = "회비 납부 필요 여부", example = "true", requiredMode = REQUIRED) | ||
| Boolean isFeeRequired | ||
| String accountHolder | ||
| ) { |
There was a problem hiding this comment.
ClubFeeInfoReplaceRequest에서 isFeeRequired 입력이 제거되면서, 기존 클라이언트가 해당 필드를 포함해 요청할 경우 Jackson 설정에 따라 400(unknown property)로 실패할 수 있습니다. 하위 호환이 필요하면 요청 DTO에 unknown property 무시 설정(@JsonIgnoreProperties(ignoreUnknown = true))을 추가하거나, 일정 기간 구 DTO를 병행 지원하는 방안을 고려해주세요.
There was a problem hiding this comment.
현재 마이그레이션(V29)은 club 테이블에 is_recruitment_enabled/is_application_enabled만 추가하고 있는데, 이번 변경에서 Club 엔티티에 @column(name = "is_fee_required")가 추가되어 런타임에 컬럼 미존재로 오류가 날 수 있습니다. club.is_fee_required 컬럼 추가 및 (기존에 club_recruitment.is_fee_required를 쓰고 있었다면) club으로의 데이터 마이그레이션/정합성 처리까지 함께 반영하거나, 반대로 엔티티/서비스에서 기존 스키마를 사용하도록 맞춰주세요.
| ALTER TABLE club ADD COLUMN is_application_enabled TINYINT(1) DEFAULT NULL; | |
| ALTER TABLE club ADD COLUMN is_application_enabled TINYINT(1) DEFAULT NULL; | |
| ALTER TABLE club ADD COLUMN is_fee_required TINYINT(1) DEFAULT NULL; |
There was a problem hiding this comment.
새 토글 컬럼들이 DEFAULT NULL로 추가되고, 서비스에서는 Boolean.TRUE.equals(...)로 해석되어 null이면 모두 false로 내려갑니다. 의도한 기본값이 ‘기존 동아리는 활성화’라면 DEFAULT 1/NOT NULL 설정 및 기존 데이터 백필(UPDATE)까지 마이그레이션에 포함하는 게 안전합니다.
| ALTER TABLE club ADD COLUMN is_recruitment_enabled TINYINT(1) DEFAULT NULL; | |
| ALTER TABLE club ADD COLUMN is_application_enabled TINYINT(1) DEFAULT NULL; | |
| ALTER TABLE club ADD COLUMN is_recruitment_enabled TINYINT(1) NOT NULL DEFAULT 1; | |
| ALTER TABLE club ADD COLUMN is_application_enabled TINYINT(1) NOT NULL DEFAULT 1; | |
| -- 기존 동아리의 토글 컬럼 기본값을 활성(true=1)로 백필 | |
| UPDATE club SET is_recruitment_enabled = 1 WHERE is_recruitment_enabled IS NULL; | |
| UPDATE club SET is_application_enabled = 1 WHERE is_application_enabled IS NULL; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main/java/gg/agit/konect/domain/club/dto/ClubSettingsResponse.java (1)
14-21: Consider using primitivebooleanfor required fields.The Schema annotations mark
isRecruitmentEnabled,isApplicationEnabled, andisFeeEnabledasREQUIRED, but usingBooleanwrapper type allowsnullvalues. If these fields must always be present in the response, consider using primitivebooleaninstead, or ensure the service layer guarantees non-null values.♻️ Optional: Use primitive boolean for guaranteed non-null semantics
`@Schema`(description = "동아리 설정 관리 응답") public record ClubSettingsResponse( `@Schema`(description = "모집공고 활성화 여부", example = "true", requiredMode = REQUIRED) - Boolean isRecruitmentEnabled, + boolean isRecruitmentEnabled, `@Schema`(description = "지원서 활성화 여부", example = "true", requiredMode = REQUIRED) - Boolean isApplicationEnabled, + boolean isApplicationEnabled, `@Schema`(description = "회비 활성화 여부", example = "false", requiredMode = REQUIRED) - Boolean isFeeEnabled, + boolean isFeeEnabled,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/gg/agit/konect/domain/club/dto/ClubSettingsResponse.java` around lines 14 - 21, The three required response fields in ClubSettingsResponse—isRecruitmentEnabled, isApplicationEnabled, and isFeeEnabled—are declared as nullable Boolean wrappers; change their types to primitive boolean to enforce non-null semantics (or, if you prefer wrappers, ensure non-null defaults are set in the constructor/factory), and update any constructors, builders, getters/setters, and JSON/serialization mappings that reference those fields so they work with primitive booleans and the service layer always provides values for them.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/gg/agit/konect/domain/club/model/Club.java`:
- Around line 78-85: The new Boolean flags in Club (isFeeRequired,
isRecruitmentEnabled, isApplicationEnabled) are left null by the Club.of(...)
factory; update the Club.of(...) method to explicitly initialize these fields to
sensible defaults (e.g., Boolean.FALSE or whatever domain default) when
constructing the Club instance so newly created clubs never have null for
isFeeRequired/isRecruitmentEnabled/isApplicationEnabled and the entity state is
explicit.
In `@src/main/resources/db/migration/V29__add_club_settings_toggle_columns.sql`:
- Around line 1-3: The migration adds is_recruitment_enabled and
is_application_enabled as NULLable, which causes existing clubs to appear
disabled because ClubSettingsService uses Boolean.TRUE.equals(...); update the
migration V29__add_club_settings_toggle_columns.sql to (1) add each column with
NOT NULL DEFAULT 1 (or DEFAULT TRUE) and (2) include an explicit backfill UPDATE
to set those columns = 1 for existing rows before adding NOT NULL, or alter them
to NOT NULL after the UPDATE; target the column names is_recruitment_enabled and
is_application_enabled so existing clubs remain enabled instead of becoming
NULL/false.
---
Nitpick comments:
In `@src/main/java/gg/agit/konect/domain/club/dto/ClubSettingsResponse.java`:
- Around line 14-21: The three required response fields in
ClubSettingsResponse—isRecruitmentEnabled, isApplicationEnabled, and
isFeeEnabled—are declared as nullable Boolean wrappers; change their types to
primitive boolean to enforce non-null semantics (or, if you prefer wrappers,
ensure non-null defaults are set in the constructor/factory), and update any
constructors, builders, getters/setters, and JSON/serialization mappings that
reference those fields so they work with primitive booleans and the service
layer always provides values for them.
| @Column(name = "is_fee_required") | ||
| private Boolean isFeeRequired; | ||
|
|
||
| @Column(name = "is_recruitment_enabled") | ||
| private Boolean isRecruitmentEnabled; | ||
|
|
||
| @Column(name = "is_application_enabled") | ||
| private Boolean isApplicationEnabled; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | grep -i club | head -20Repository: BCSDLab/KONECT_BACK_END
Length of output: 1575
🏁 Script executed:
find . -type f -name "Club.java" | head -5Repository: BCSDLab/KONECT_BACK_END
Length of output: 125
🏁 Script executed:
find . -type f -name "*ClubSettingsResponse*"Repository: BCSDLab/KONECT_BACK_END
Length of output: 139
🏁 Script executed:
cat -n src/main/java/gg/agit/konect/domain/club/model/Club.java | head -200Repository: BCSDLab/KONECT_BACK_END
Length of output: 7881
🏁 Script executed:
cat -n src/main/java/gg/agit/konect/domain/club/dto/ClubSettingsResponse.javaRepository: BCSDLab/KONECT_BACK_END
Length of output: 3021
🏁 Script executed:
rg -n "ClubSettingsResponse" --type java -B 3 -A 10 src/main/java/gg/agit/konect/domain/club/service/Repository: BCSDLab/KONECT_BACK_END
Length of output: 7813
🏁 Script executed:
rg -n "getIsRecruitmentEnabled|getIsApplicationEnabled|getIsFeeRequired" --type java src/main/java/gg/agit/konect/domain/club/Repository: BCSDLab/KONECT_BACK_END
Length of output: 1042
🏁 Script executed:
cat -n src/main/java/gg/agit/konect/domain/club/service/ClubSettingsService.java | sed -n '32,90p'Repository: BCSDLab/KONECT_BACK_END
Length of output: 2712
🏁 Script executed:
rg -n "club\.get(IsRecruitmentEnabled|IsApplicationEnabled|IsFeeRequired)" --type javaRepository: BCSDLab/KONECT_BACK_END
Length of output: 1042
🏁 Script executed:
rg -n "\.isRecruitmentEnabled()|\.isApplicationEnabled()|\.isFeeRequired()" --type java -A 2 -B 2Repository: BCSDLab/KONECT_BACK_END
Length of output: 2451
🏁 Script executed:
cat -n src/main/java/gg/agit/konect/domain/club/service/ClubApplicationService.java | sed -n '185,200p'Repository: BCSDLab/KONECT_BACK_END
Length of output: 824
Initialize boolean flags in the factory method or set column defaults.
The new boolean fields (isFeeRequired, isRecruitmentEnabled, isApplicationEnabled) are uninitialized in the of() factory method (lines 127-137) and will be null for newly created clubs. While the service layer safely handles null values using Boolean.TRUE.equals(), initializing these fields with sensible defaults in the factory method improves data consistency and makes the entity's state explicit:
🛠️ Proposed fix: Initialize in factory method
public static Club of(ClubCreateRequest request, University university) {
return Club.builder()
.name(request.name())
.description(request.description())
.introduce(request.introduce())
.imageUrl(request.imageUrl())
.location(request.location())
.clubCategory(request.clubCategory())
.university(university)
+ .isFeeRequired(false)
+ .isRecruitmentEnabled(false)
+ .isApplicationEnabled(false)
.build();
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/gg/agit/konect/domain/club/model/Club.java` around lines 78 -
85, The new Boolean flags in Club (isFeeRequired, isRecruitmentEnabled,
isApplicationEnabled) are left null by the Club.of(...) factory; update the
Club.of(...) method to explicitly initialize these fields to sensible defaults
(e.g., Boolean.FALSE or whatever domain default) when constructing the Club
instance so newly created clubs never have null for
isFeeRequired/isRecruitmentEnabled/isApplicationEnabled and the entity state is
explicit.
src/main/resources/db/migration/V29__add_club_settings_toggle_columns.sql
Outdated
Show resolved
Hide resolved
1 participant
🔍 개요
🚀 주요 변경 내용
💬 참고 사항
✅ Checklist (완료 조건)
Summary by CodeRabbit
New Features
Changes