Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,14 @@ ResponseEntity<ClubMemberSheetSyncResponse> migrateSheet(
@Operation(
summary = "시트 불러오기 전 부원 목록을 미리본다",
description = """
스프레드시트 URL을 읽어 등록 예정인 부원 목록을 JSON으로 반환합니다.
PUT /clubs/{clubId}/sheet 로 AI 분석 및 등록이 완료된 스프레드시트를 읽어
등록 예정인 부원 목록을 JSON으로 반환합니다.
이 API는 데이터를 저장하지 않고 미리보기 용도로만 사용합니다.
"""
)
@PostMapping("/{clubId}/sheet/import/preview")
ResponseEntity<SheetImportPreviewResponse> previewPreMembers(
@PathVariable(name = "clubId") Integer clubId,
@Valid @RequestBody SheetImportRequest request,
@UserId Integer requesterId
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,10 @@ public ResponseEntity<ClubMemberSheetSyncResponse> migrateSheet(
@Override
public ResponseEntity<SheetImportPreviewResponse> previewPreMembers(
@PathVariable(name = "clubId") Integer clubId,
@Valid @RequestBody SheetImportRequest request,
@UserId Integer requesterId
) {
SheetImportPreviewResponse response = sheetImportService.previewPreMembersFromSheet(
clubId, requesterId, request.spreadsheetUrl()
clubId, requesterId
);
return ResponseEntity.ok(response);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,25 @@ public interface ClubRepository extends Repository<Club, Integer> {
""")
Optional<Club> findById(@Param(value = "id") Integer id);

@Query(value = """
SELECT c
FROM Club c
LEFT JOIN FETCH c.university
LEFT JOIN FETCH c.clubRecruitment cr
WHERE c.id = :id
""")
Optional<Club> findByIdWithUniversity(@Param(value = "id") Integer id);

default Club getById(Integer id) {
return findById(id).orElseThrow(() ->
CustomException.of(ApiResponseCode.NOT_FOUND_CLUB));
}

default Club getByIdWithUniversity(Integer id) {
return findByIdWithUniversity(id).orElseThrow(() ->
CustomException.of(ApiResponseCode.NOT_FOUND_CLUB));
}

List<Club> findAll();

Club save(Club club);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
import org.springframework.transaction.annotation.Transactional;
import org.springframework.transaction.support.TransactionTemplate;

import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.api.services.sheets.v4.Sheets;
import com.google.api.services.sheets.v4.model.ValueRange;

Expand Down Expand Up @@ -53,28 +55,55 @@ public class SheetImportService {
private final ChatRoomMembershipService chatRoomMembershipService;
private final ClubPermissionValidator clubPermissionValidator;
private final PlatformTransactionManager transactionManager;
private final ObjectMapper objectMapper;

public SheetImportPreviewResponse previewPreMembersFromSheet(
Integer clubId,
Integer requesterId,
String spreadsheetUrl
Integer requesterId
) {
clubPermissionValidator.validateManagerAccess(clubId, requesterId);
Club club = resolveClubWithRegisteredSheet(clubId);
String spreadsheetId = club.getGoogleSheetId();
SheetColumnMapping mapping = resolveRegisteredMemberListMapping(club);
SheetImportSource source = loadSheetImportSource(spreadsheetId, mapping);
SheetImportPlan plan = buildImportPlan(
clubId,
club,
source.members(),
source.warnings()
);
return SheetImportPreviewResponse.of(plan.previewMembers(), plan.warnings());
}

Comment on lines +65 to 77
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

previewPreMembersFromSheet()가 @Transactional/executeReadOnlyTransaction 없이 Club 엔티티를 조회한 뒤 buildImportPlan()에서 club.getUniversity().getId()를 접근합니다. Club.university는 LAZY라서(그리고 ClubRepository.getById는 university를 fetch join 하지 않음) 트랜잭션 밖에서는 LazyInitializationException이 발생할 수 있습니다. 이 메서드 전체를 @transactional(readOnly = true)로 감싸거나, 최소한 club 조회 및 buildImportPlan 구간을 executeReadOnlyTransaction으로 감싸도록 수정해주세요(구글 시트 호출은 트랜잭션 밖에 두는 편이 안전합니다).

Suggested change
Club club = resolveClubWithRegisteredSheet(clubId);
String spreadsheetId = club.getGoogleSheetId();
SheetColumnMapping mapping = resolveRegisteredMemberListMapping(club);
SheetImportSource source = loadSheetImportSource(spreadsheetId, mapping);
SheetImportPlan plan = buildImportPlan(
clubId,
club,
source.members(),
source.warnings()
);
return SheetImportPreviewResponse.of(plan.previewMembers(), plan.warnings());
}
RegisteredSheetContext context = executeReadOnlyTransaction(() -> {
Club club = resolveClubWithRegisteredSheet(clubId);
return new RegisteredSheetContext(
club.getGoogleSheetId(),
resolveRegisteredMemberListMapping(club)
);
});
SheetImportSource source = loadSheetImportSource(context.spreadsheetId(), context.mapping());
SheetImportPlan plan = executeReadOnlyTransaction(() -> {
Club club = resolveClubWithRegisteredSheet(clubId);
return buildImportPlan(
clubId,
club,
source.members(),
source.warnings()
);
});
return SheetImportPreviewResponse.of(plan.previewMembers(), plan.warnings());
}
private <T> T executeReadOnlyTransaction(Supplier<T> supplier) {
TransactionTemplate transactionTemplate = new TransactionTemplate(transactionManager);
transactionTemplate.setReadOnly(true);
return transactionTemplate.execute(status -> supplier.get());
}
private static final class RegisteredSheetContext {
private final String spreadsheetId;
private final SheetColumnMapping mapping;
private RegisteredSheetContext(String spreadsheetId, SheetColumnMapping mapping) {
this.spreadsheetId = spreadsheetId;
this.mapping = mapping;
}
private String spreadsheetId() {
return spreadsheetId;
}
private SheetColumnMapping mapping() {
return mapping;
}
}

Copilot uses AI. Check for mistakes.
String spreadsheetId = SpreadsheetUrlParser.extractId(spreadsheetUrl);
SheetHeaderMapper.SheetAnalysisResult analysis =
sheetHeaderMapper.analyzeAllSheets(spreadsheetId);
SheetImportSource source = loadSheetImportSource(spreadsheetId, analysis.memberListMapping());
return executeReadOnlyTransaction(() -> {
Club club = clubRepository.getById(clubId);
SheetImportPlan plan = buildImportPlan(
clubId,
club,
source.members(),
source.warnings()
);
return SheetImportPreviewResponse.of(plan.previewMembers(), plan.warnings());
});
private Club resolveClubWithRegisteredSheet(Integer clubId) {
Club club = clubRepository.getByIdWithUniversity(clubId);
String spreadsheetId = club.getGoogleSheetId();
if (spreadsheetId == null || spreadsheetId.isBlank()) {
throw CustomException.of(ApiResponseCode.CLUB_SHEET_ANALYSIS_REQUIRED);
}
return club;
}

private SheetColumnMapping resolveRegisteredMemberListMapping(Club club) {
String mappingJson = club.getSheetColumnMapping();
if (mappingJson == null || mappingJson.isBlank()) {
throw CustomException.of(ApiResponseCode.CLUB_SHEET_ANALYSIS_REQUIRED);
}

try {
Map<String, Object> raw = objectMapper.readValue(mappingJson, new TypeReference<>() {});
int dataStartRow = raw.containsKey("dataStartRow")
? ((Number)raw.get("dataStartRow")).intValue() : 2;
Map<String, Integer> fieldMap = new HashMap<>();
Comment on lines +93 to +97
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

resolveRegisteredMemberListMapping()의 JSON 파싱 로직이 SheetSyncExecutor.resolveRawMapping()와 거의 동일하게 중복되어 있습니다. 두 곳이 서로 다른 방식으로 진화하면(기본값/검증 규칙 등) 동작이 쉽게 어긋날 수 있으니, 공용 유틸(예: SheetColumnMapping의 static parse/fromJson)로 추출하거나 한쪽 로직을 재사용하도록 정리하는 편이 유지보수에 유리합니다.

Copilot uses AI. Check for mistakes.
raw.forEach((key, value) -> {
if (!"dataStartRow".equals(key) && value instanceof Number number) {
fieldMap.put(key, number.intValue());
}
});
return new SheetColumnMapping(fieldMap, dataStartRow);
} catch (Exception e) {
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

예외를 잡아 CLUB_SHEET_ANALYSIS_REQUIRED로 변환하는 것은 의도일 수 있지만, 현재 catch 블록에서 원인 예외를 완전히 삼켜 운영/디버깅 시 원인 파악이 어렵습니다. 최소한 log.warn으로 cause를 남기거나(민감정보가 없다면 mappingJson 길이/일부 정보 포함) detail에 원인 요약을 넣는 방식으로 추적 가능성을 확보해주세요.

Suggested change
} catch (Exception e) {
} catch (Exception e) {
log.warn(
"Failed to parse sheet column mapping. mappingJsonLength={}, causeType={}",
mappingJson.length(),
e.getClass().getSimpleName(),
e
);

Copilot uses AI. Check for mistakes.
throw CustomException.of(ApiResponseCode.CLUB_SHEET_ANALYSIS_REQUIRED);
}
Comment on lines +104 to +106
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

[LEVEL: low] 넓은 범위의 예외 처리가 디버깅을 어렵게 할 수 있습니다.

문제: catch (Exception e)JsonProcessingException, ClassCastException 외에 예상치 못한 프로그래밍 오류(NullPointerException 등)도 동일하게 처리합니다. 영향: 실제 버그가 CLUB_SHEET_ANALYSIS_REQUIRED 응답으로 은폐될 수 있습니다. 제안: 특정 예외 타입만 처리하거나, 최소한 로그를 남겨 디버깅을 돕도록 개선하세요.

🔧 개선 예시
         } catch (Exception e) {
+            log.warn("Failed to parse sheet column mapping for club. json={}", mappingJson, e);
             throw CustomException.of(ApiResponseCode.CLUB_SHEET_ANALYSIS_REQUIRED);
         }
🤖 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/service/SheetImportService.java`
around lines 104 - 106, Replace the broad catch (Exception e) in
SheetImportService with targeted handling: catch the expected parsing/format
exceptions (e.g., JsonProcessingException, ClassCastException) and throw
CustomException.of(ApiResponseCode.CLUB_SHEET_ANALYSIS_REQUIRED) for those
cases, and for any other unexpected Throwable log the error using the class
logger and rethrow or wrap it (do not map all unexpected errors to
CLUB_SHEET_ANALYSIS_REQUIRED); reference the existing CustomException.of and
ApiResponseCode.CLUB_SHEET_ANALYSIS_REQUIRED so the specific-case behavior is
preserved while ensuring unexpected errors are logged and propagated for proper
debugging.

Comment on lines +93 to +106
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

resolveRegisteredMemberListMapping()에서 sheetColumnMapping JSON 파싱 실패(예: 잘못된 JSON/예상치 못한 타입) 시 바로 CLUB_SHEET_ANALYSIS_REQUIRED로 변환되는데, CustomException은 GlobalExceptionHandler에서 별도 로깅이 없어 원인(및 clubId)을 추적하기 어렵습니다. catch 블록에서 최소한 clubId와 함께 log.warn으로 예외 원인을 남기고, 필요하면 원본 예외를 포함해 로깅해 주세요.

Copilot uses AI. Check for mistakes.
}

@Transactional
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/gg/agit/konect/global/code/ApiResponseCode.java
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ public enum ApiResponseCode {
INVALID_NOTIFICATION_TOKEN(HttpStatus.BAD_REQUEST, "푸시 알림 토큰이 유효하지 않습니다."),
FEE_PAYMENT_IMAGE_REQUIRED(HttpStatus.BAD_REQUEST, "회비 납부가 필요한 동아리입니다. 납부 증빙 사진을 첨부해주세요."),
AMBIGUOUS_USER_MATCH(HttpStatus.BAD_REQUEST, "동일한 정보로 식별되는 사용자가 2명 이상입니다. 관리자에게 문의해주세요."),
CLUB_SHEET_ANALYSIS_REQUIRED(HttpStatus.BAD_REQUEST,
"구글 시트 파일에서 동아리 부원을 가져오기 전에 먼저 AI 분석 및 등록이 완료되어야 합니다."),

// 401 Unauthorized
INVALID_SESSION(HttpStatus.UNAUTHORIZED, "올바르지 않은 인증 정보 입니다."),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package gg.agit.konect.domain.club.service;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyList;
import static org.mockito.ArgumentMatchers.anySet;
Expand All @@ -15,9 +16,10 @@
import org.junit.jupiter.api.Test;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.Spy;
import org.springframework.transaction.PlatformTransactionManager;
import org.springframework.transaction.support.SimpleTransactionStatus;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.api.services.sheets.v4.Sheets;
import com.google.api.services.sheets.v4.model.ValueRange;

Expand All @@ -33,6 +35,8 @@
import gg.agit.konect.domain.club.repository.ClubRepository;
import gg.agit.konect.domain.user.model.User;
import gg.agit.konect.domain.user.repository.UserRepository;
import gg.agit.konect.global.code.ApiResponseCode;
import gg.agit.konect.global.exception.CustomException;
import gg.agit.konect.support.ServiceTestSupport;
import gg.agit.konect.support.fixture.ClubFixture;
import gg.agit.konect.support.fixture.UniversityFixture;
Expand All @@ -43,8 +47,6 @@ class SheetImportServiceTest extends ServiceTestSupport {
private static final Integer CLUB_ID = 1;
private static final Integer REQUESTER_ID = 2;
private static final String SPREADSHEET_ID = "sheet-id";
private static final String SPREADSHEET_URL =
"https://docs.google.com/spreadsheets/d/" + SPREADSHEET_ID + "/edit";

@Mock
private Sheets googleSheetsService;
Expand Down Expand Up @@ -82,23 +84,26 @@ class SheetImportServiceTest extends ServiceTestSupport {
@Mock
private PlatformTransactionManager transactionManager;

@Spy
private ObjectMapper objectMapper = new ObjectMapper();

@InjectMocks
private SheetImportService sheetImportService;

@Test
void previewPreMembersFromSheetReturnsDirectAndPreMembers() throws IOException {
Club club = ClubFixture.create(UniversityFixture.create());
club.updateGoogleSheetId(SPREADSHEET_ID);
club.updateSheetColumnMapping(objectMapper.writeValueAsString(
SheetColumnMapping.defaultMapping().toMap()
));
User directUser = UserFixture.createUser(club.getUniversity(), "Alex Kim", "2021232948");

given(clubRepository.getById(CLUB_ID)).willReturn(club);
given(sheetHeaderMapper.analyzeAllSheets(SPREADSHEET_ID)).willReturn(
new SheetHeaderMapper.SheetAnalysisResult(SheetColumnMapping.defaultMapping(), null, null)
);
given(clubRepository.getByIdWithUniversity(CLUB_ID)).willReturn(club);
given(clubMemberRepository.findStudentNumbersByClubId(CLUB_ID)).willReturn(Set.of());
given(clubPreMemberRepository.findStudentNumberAndNameByClubId(CLUB_ID))
.willReturn(List.<ClubPreMemberRepository.PreMemberKey>of());
given(clubMemberRepository.findUserIdsByClubId(CLUB_ID)).willReturn(List.of());
given(transactionManager.getTransaction(any())).willReturn(new SimpleTransactionStatus());
given(userRepository.findAllByUniversityIdAndStudentNumberIn(
eq(club.getUniversity().getId()),
anySet()
Expand All @@ -115,8 +120,7 @@ void previewPreMembersFromSheetReturnsDirectAndPreMembers() throws IOException {

SheetImportPreviewResponse response = sheetImportService.previewPreMembersFromSheet(
CLUB_ID,
REQUESTER_ID,
SPREADSHEET_URL
REQUESTER_ID
);

assertThat(response.previewCount()).isEqualTo(2);
Expand All @@ -133,6 +137,35 @@ void previewPreMembersFromSheetReturnsDirectAndPreMembers() throws IOException {
.containsExactly(true, true);
}

@Test
void previewPreMembersFromSheetThrowsWhenSheetIsNotRegistered() {
Club club = ClubFixture.create(UniversityFixture.create());

given(clubRepository.getByIdWithUniversity(CLUB_ID)).willReturn(club);

assertThatThrownBy(() -> sheetImportService.previewPreMembersFromSheet(CLUB_ID, REQUESTER_ID))
.isInstanceOf(CustomException.class)
.extracting(exception -> ((CustomException)exception).getErrorCode())
.isEqualTo(ApiResponseCode.CLUB_SHEET_ANALYSIS_REQUIRED);

verifyNoInteractions(googleSheetsService, sheetHeaderMapper);
}

@Test
void previewPreMembersFromSheetThrowsWhenSheetMappingIsMissing() {
Club club = ClubFixture.create(UniversityFixture.create());
club.updateGoogleSheetId(SPREADSHEET_ID);

given(clubRepository.getByIdWithUniversity(CLUB_ID)).willReturn(club);

assertThatThrownBy(() -> sheetImportService.previewPreMembersFromSheet(CLUB_ID, REQUESTER_ID))
.isInstanceOf(CustomException.class)
Comment on lines +140 to +147
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

새로 추가된 테스트에서는 previewPreMembersFromSheet()가 내부적으로 TransactionTemplate을 실행하므로 transactionManager.getTransaction(...)가 반드시 TransactionStatus를 반환해야 합니다. 현재 이 테스트에서는 transactionManager 스텁이 없어 Mockito 기본값(null)로 인해 NPE로 테스트가 실패할 수 있으니, 다른 테스트처럼 given(transactionManager.getTransaction(any())).willReturn(new SimpleTransactionStatus()) 등을 추가해 주세요.

Copilot uses AI. Check for mistakes.
.extracting(exception -> ((CustomException)exception).getErrorCode())
.isEqualTo(ApiResponseCode.CLUB_SHEET_ANALYSIS_REQUIRED);

verifyNoInteractions(googleSheetsService, sheetHeaderMapper);
}

@Test
void confirmImportPreMembersImportsOnlyEnabledMembers() {
Club club = ClubFixture.create(UniversityFixture.create());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,10 @@ void previewPreMembersSuccess() throws Exception {

given(sheetImportService.previewPreMembersFromSheet(
eq(CLUB_ID),
eq(REQUESTER_ID),
eq(SPREADSHEET_URL)
eq(REQUESTER_ID)
)).willReturn(response);

SheetImportRequest request = new SheetImportRequest(SPREADSHEET_URL);

performPost("/clubs/" + CLUB_ID + "/sheet/import/preview", request)
performPost("/clubs/" + CLUB_ID + "/sheet/import/preview")
.andExpect(status().isOk())
.andExpect(jsonPath("$.previewCount").value(2))
.andExpect(jsonPath("$.autoRegisteredCount").value(1))
Expand All @@ -96,19 +93,32 @@ void previewPreMembersSuccess() throws Exception {
void previewPreMembersForbiddenGoogleSheetAccess() throws Exception {
given(sheetImportService.previewPreMembersFromSheet(
eq(CLUB_ID),
eq(REQUESTER_ID),
eq(SPREADSHEET_URL)
eq(REQUESTER_ID)
)).willThrow(CustomException.of(ApiResponseCode.FORBIDDEN_GOOGLE_SHEET_ACCESS));

SheetImportRequest request = new SheetImportRequest(SPREADSHEET_URL);

performPost("/clubs/" + CLUB_ID + "/sheet/import/preview", request)
performPost("/clubs/" + CLUB_ID + "/sheet/import/preview")
.andExpect(status().isForbidden())
.andExpect(jsonPath("$.code")
.value(ApiResponseCode.FORBIDDEN_GOOGLE_SHEET_ACCESS.name()))
.andExpect(jsonPath("$.message")
.value(ApiResponseCode.FORBIDDEN_GOOGLE_SHEET_ACCESS.getMessage()));
}

@Test
@DisplayName("returns 400 when sheet analysis and registration are not completed")
void previewPreMembersRequiresRegisteredSheet() throws Exception {
given(sheetImportService.previewPreMembersFromSheet(
eq(CLUB_ID),
eq(REQUESTER_ID)
)).willThrow(CustomException.of(ApiResponseCode.CLUB_SHEET_ANALYSIS_REQUIRED));

performPost("/clubs/" + CLUB_ID + "/sheet/import/preview")
.andExpect(status().isBadRequest())
.andExpect(jsonPath("$.code")
.value(ApiResponseCode.CLUB_SHEET_ANALYSIS_REQUIRED.name()))
.andExpect(jsonPath("$.message")
.value(ApiResponseCode.CLUB_SHEET_ANALYSIS_REQUIRED.getMessage()));
}
}

@Nested
Expand Down
Loading