Conversation
## 🔗 Related Issue <!-- 이슈 번호를 작성하여 종료시켜주세요 --> - Closes #64 ## 📝 Summary <!-- 작업한 기능을 설명해주세요 --> - 은행별 목표 조회를 별도 API로 분리하지 않고, 계좌 조회 API에 연결된 목표 정보를 함께 제공하도록 수정하였습니다. - JPQL 기반 조회 로직을 QueryDSL로 리팩토링하였습니다. ## 🔄 Changes <!-- 구체적으로 어떤 파일/로직이 변경되었는지 체크해주세요 --> - [X] API 변경 (추가/수정) - [ ] 데이터 및 도메인 변경 (DB, 비즈니스 로직) - [ ] 설정 또는 인프라 관련 변경 - [X] 리팩토링 ## 💬 Questions & Review Points <!-- 리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요 --> ## 📸 API Test Results (Swagger) <!-- API 테스트 스크린샷 첨부 --> <img width="1632" height="1302" alt="image" src="https://github.com/user-attachments/assets/8847e665-2aae-4421-ae8b-c64e9af2f942" /> <img width="1672" height="866" alt="image" src="https://github.com/user-attachments/assets/d4e1f256-a2d4-454d-97c4-a15c2757de34" /> ## ✅ Checklist - [x] API 테스트 완료 - [x] 테스트 결과 사진 첨부 - [x] 빌드 성공 확인 (./gradlew build)
## 🔗 Related Issue <!-- 이슈 번호를 작성하여 종료시켜주세요 --> - Closes #68 ## 📝 Summary <!-- 작업한 기능을 설명해주세요 --> 은행 금융사 연동 삭제 시 계좌와 연결된 목표도 삭제되도록 보완했습니다. ## 🔄 Changes <!-- 구체적으로 어떤 파일/로직이 변경되었는지 체크해주세요 --> - [X] API 변경 (추가/수정) - [ ] 데이터 및 도메인 변경 (DB, 비즈니스 로직) - [ ] 설정 또는 인프라 관련 변경 - [ ] 리팩토링 ## 💬 Questions & Review Points <!-- 리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요 --> ## 📸 API Test Results (Swagger) <!-- API 테스트 스크린샷 첨부 --> ## ✅ Checklist - [x] API 테스트 완료 - [ ] 테스트 결과 사진 첨부 - [x] 빌드 성공 확인 (./gradlew build)
## 🔗 Related Issue <!-- 이슈 번호를 작성하여 종료시켜주세요 --> - Closes #56 ## 📝 Summary <!-- 작업한 기능을 설명해주세요 --> ### Goal 생성 시 계좌 연동 및 계좌 변경 API 추가 - 목표(Goal) 생성 시 계좌(account) 필수 연동 구조로 변경 - Goal ↔ BankAccount 1:1 관계 적용 - 현재 계좌들 중 연결 된 목표가 없는 계좌 조회하는 API 추가 - 목표에 연결된 계좌를 PUT 방식으로 설정/교체할 수 있는 API 추가 - Goal 상세/생성 응답에 연결된 계좌 정보 포함 ### Primary 목표 조회 API 추가 - 홈화면에서 진행 중(ACTIVE) 목표를 최신순으로 전체 조회하는 /api/goals/primary API 추가 ## 🔄 Changes <!-- 구체적으로 어떤 파일/로직이 변경되었는지 체크해주세요 --> - [x] API 변경 (추가/수정) - [ ] 데이터 및 도메인 변경 (DB, 비즈니스 로직) - [ ] 설정 또는 인프라 관련 변경 - [ ] 리팩토링 ## 💬 Questions & Review Points 일단 아직 거래 내역 불러오는 api 제외 계좌 연동 관련 api 모두 작성했습니다. 계좌는 목표 생성 시에 필수로 id를 입력하게 변경했고, 계좌 연동이 해지될 때 다른 계좌로 계좌만 재연결 해야하는걸 대비 해 /api/goals/{goalId}/linked-accounts 를 put으로 구현했습니다! ## 📸 API Test Results (Swagger) ### 목표 생성 시 선택한 계좌가 이미 목표랑 연동 된 계좌일 때 <img width="966" height="877" alt="image" src="https://github.com/user-attachments/assets/cd993671-d3b6-4398-8236-9be16fad1681" /> ### 목표 생성 시 <img width="1185" height="681" alt="image" src="https://github.com/user-attachments/assets/cd43cc96-effe-4385-baf8-6c0ffadf611e" /> ### /api/goals/accounts -> 목표에 연결되지 않은 계좌 목록 조회 <img width="1003" height="585" alt="image" src="https://github.com/user-attachments/assets/c90a0a4d-5fb8-45b9-a450-417f3fcb618b" /> ### /api/goals/{goalId}/ledgers -> 목표 별 거래내역 조회 <img width="2125" height="894" alt="image" src="https://github.com/user-attachments/assets/30cddf60-64be-45e8-bba3-9c755619b261" /> ### /api/goals/primary -> 홈화면 목표 목록 조회 <img width="991" height="726" alt="image" src="https://github.com/user-attachments/assets/a1b960ab-3fec-4283-a297-0da9b7df1107" /> ## ✅ Checklist - [x] API 테스트 완료 - [x] 테스트 결과 사진 첨부 - [x] 빌드 성공 확인 (./gradlew build) --------- Co-authored-by: dohee <152317074+seamooll@users.noreply.github.com>
## 🔗 Related Issue <!-- 이슈 번호를 작성하여 종료시켜주세요 --> - closes #54 ## 📝 Summary <!-- 작업한 기능을 설명해주세요 --> 사용자의 금융 자산(은행 내역, 카드 내역)을 한 번에 가져오는 전체 동기화 API를 구현하였습니다. - 전체 자산 동기화 API 구현 및 성능 개선 - 은행/카드 거래 내역을 외부 API로 조회 후 DB에 저장하는 전체 동기화 로직 구현 - JPA saveAll() 대신 JdbcTemplate 기반 Bulk Insert 방식 적용 (대량 Insert 시 발생하던 성능 저하 문제 개선) - 트러블슈팅 및 안정화 ## 🔄 Changes <!-- 구체적으로 어떤 파일/로직이 변경되었는지 체크해주세요 --> - [X] API 변경 (추가/수정) - [ ] 데이터 및 도메인 변경 (DB, 비즈니스 로직) - [ ] 설정 또는 인프라 관련 변경 - [X] 리팩토링 ## 💬 Questions & Review Points <!-- 리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요 --> ## 📸 API Test Results (Swagger) <!-- API 테스트 스크린샷 첨부 --> <img width="1949" height="1216" alt="image" src="https://github.com/user-attachments/assets/890e548e-8e34-429c-9ce6-0c0c970b14d0" /> ## ✅ Checklist - [X] API 테스트 완료 - [X] 테스트 결과 사진 첨부 - [X] 빌드 성공 확인 (./gradlew build)
Summary of ChangesHello @seamooll, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 Pull Request는 사용자의 자산 관리 경험을 개선하기 위해 API를 추가하고 기존 API를 개선하는 데 중점을 둡니다. 주요 변경 사항으로는 자산 동기화 API 추가, 은행별 계좌 및 목표 목록 조회 API 변경, 코드 리팩토링, API 문서 업데이트 등이 있습니다. 이러한 변경 사항을 통해 사용자는 자신의 금융 자산을 최신 상태로 유지하고, 목표 달성 현황을 더 쉽게 파악하며, 개발자는 API를 더 효율적으로 사용할 수 있습니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이번 PR은 자산 동기화 기능의 비동기 처리 도입, 목표 관리 기능의 계좌 연동 추가, 그리고 가계부 동기화 로직의 성능 개선 등 대규모 변경사항을 포함하고 있습니다. 전반적으로 코드 구조가 크게 개선되었으며, 특히 비동기 처리와 벌크 연산을 통한 성능 최적화가 인상적입니다. 몇 가지 잠재적인 문제점과 개선 사항을 발견하여 리뷰 코멘트를 남겼습니다. 주로 비동기 컨텍스트에서의 엔티티 상태 관리, 리스트 조회 시 예외 처리 정책, 데이터베이스 삽입 작업의 안정성, 그리고 Null 처리 관련 내용입니다. 확인 부탁드립니다.
Comment on lines
+134
to
+140
| try { | ||
|
|
||
| connection.getCardList().clear(); | ||
| connection.getCardList().addAll(cards); | ||
| } catch (Exception e) { | ||
| log.warn("Connection 객체의 카드 리스트 갱신 중 오류 (무시하고 진행): {}", e.getMessage()); | ||
| } |
There was a problem hiding this comment.
이벤트 리스너가 AFTER_COMMIT 단계에서 실행되므로 connection 객체는 detached 상태일 가능성이 높습니다. 이 상태에서 lazy-loaded 컬렉션인 getCardList()를 수정하려고 하면 LazyInitializationException이 발생할 수 있습니다. 현재 try-catch 블록이 이 예외를 무시하고 넘어가도록 되어 있어, 후속 로직인 codefAssetService.getCardApprovals(connection) 호출이 의도대로 동작하지 않을 위험이 있습니다.
codefAssetService에 List<Card>를 직접 전달하는 오버로딩된 메소드를 사용하여 이 문제를 해결하는 것이 좋습니다.
// connection.getCardList()를 직접 수정하는 대신, 조회한 cards 리스트를 명시적으로 전달합니다.
LocalDate now = LocalDate.now();
List<CardApproval> approvals = codefAssetService.getCardApprovals(connection, cards, now.minusMonths(3), now);| throw new GoalException(GoalErrorCode.ACCOUNT_ALREADY_LINKED_TO_GOAL); | ||
| } | ||
|
|
||
| Long startAmount = account.getBalanceAmount(); |
There was a problem hiding this comment.
account.getBalanceAmount()가 null을 반환할 경우, Goal 엔티티의 startAmount 필드는 nullable=false이므로 데이터베이스 제약 조건 위반이나 NullPointerException이 발생할 수 있습니다. getBalanceAmount()가 null일 경우를 대비하여 기본값(예: 0L)을 설정하는 것이 안전합니다.
Suggested change
| Long startAmount = account.getBalanceAmount(); | |
| Long startAmount = account.getBalanceAmount() != null ? account.getBalanceAmount() : 0L; |
Comment on lines
+80
to
+82
| if (!account.getIsActive()) { | ||
| throw new GoalException(GoalErrorCode.GOAL_ACCOUNT_INACTIVE); | ||
| } |
There was a problem hiding this comment.
스트림의 map 내부에서 계좌가 비활성 상태일 때 예외를 던지면, 목록의 항목 중 하나만 비활성 계좌에 연결되어 있어도 전체 목표 목록 조회 API가 실패하게 됩니다. 이는 사용자 경험에 좋지 않은 영향을 줄 수 있습니다.
목록 조회 시에는 예외를 발생시키는 대신, 비활성 계좌에 연결된 목표를 필터링하여 제외하거나 DTO에 특정 상태를 표시하여 반환하는 것이 더 나은 접근 방식입니다.
Suggested change
| if (!account.getIsActive()) { | |
| throw new GoalException(GoalErrorCode.GOAL_ACCOUNT_INACTIVE); | |
| } | |
| if (account == null || !account.getIsActive()) { | |
| // 비활성 계좌에 연결된 목표는 건너뛰거나, 별도의 처리를 할 수 있습니다. | |
| // 여기서는 null을 반환하여 추후 filter에서 제거하도록 합니다. | |
| return null; | |
| } |
Comment on lines
+24
to
+25
| String sql = "INSERT INTO ledger_entry (member_id, category_id, bank_transaction_id, card_approval_id, transaction_at, transaction_type, title, memo, is_user_modified, created_at, updated_at) " + | ||
| "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, NOW(), NOW())"; |
There was a problem hiding this comment.
ledger_entry 테이블에는 bank_transaction_id와 card_approval_id에 대한 고유 제약 조건이 있습니다. INSERT INTO를 사용하면 동시성 문제나 중복 데이터로 인해 DataIntegrityViolationException이 발생할 수 있습니다. INSERT IGNORE INTO를 사용하여 중복 삽입을 안전하게 무시하도록 변경하면 벌크 삽입 작업의 안정성을 높일 수 있습니다.
Suggested change
| String sql = "INSERT INTO ledger_entry (member_id, category_id, bank_transaction_id, card_approval_id, transaction_at, transaction_type, title, memo, is_user_modified, created_at, updated_at) " + | |
| "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, NOW(), NOW())"; | |
| String sql = "INSERT IGNORE INTO ledger_entry (member_id, category_id, bank_transaction_id, card_approval_id, transaction_at, transaction_type, title, memo, is_user_modified, created_at, updated_at) " + | |
| "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, NOW(), NOW())"; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 Summary
[API 1차 완료] develop 브랜치의 최신 변경사항을 main 브랜치에 반영합니다.
🔄 Changes