Conversation
Summary of ChangesHello @hodoon, 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! 이 PR은 CODEF 연동을 통해 가져온 사용자의 은행 거래 및 카드 승인 내역을 가계부 테이블에 자동으로 저장하는 기능을 구현합니다. 기존 자산 동기화 서비스에 가계부 동기화 로직을 통합하여, 새로운 거래 내역이 발생할 때마다 가계부가 최신 상태로 유지되도록 자동화 프로세스를 구축했습니다. 이를 통해 사용자는 수동으로 가계부를 업데이트할 필요 없이, CODEF 연동만으로 자산과 가계부를 효율적으로 관리할 수 있게 됩니다. 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
|
There was a problem hiding this comment.
Code Review
이번 PR은 CODEF를 통해 가져온 거래 내역을 가계부에 자동으로 동기화하는 리팩토링을 진행했네요. AssetFetchService와 AssetSyncService에 LedgerSyncService를 통합하여 새로운 거래 내역이 있을 때 가계부 동기화가 트리거되도록 수정한 점이 좋습니다. 다만, AssetSyncService 내에서 가계부 동기화를 호출하는 로직이 중복되고, 동기화 기간을 나타내는 숫자가 하드코딩되어 있어 이 부분을 상수로 추출하고 중복 로직을 메서드로 분리하면 코드의 유지보수성과 가독성이 더 향상될 것 같습니다. 관련하여 몇 가지 리뷰 의견을 남겼습니다.
src/main/java/org/umc/valuedi/domain/asset/service/command/AssetSyncService.java
Outdated
Show resolved
Hide resolved
src/main/java/org/umc/valuedi/domain/asset/service/command/AssetSyncService.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
이전 거래내역 동기화 API의 중복 제거 로직은 카드 승인내역을 기준으로 은행 거래내역을 필터링하는 방식이었는데, 금융사 연동 과정에서 바로 LedgerEntry까지 저장하도록 변경되면서 기존 중복 판단 로직이 잘 동작하지 않는 것 같습니다.
은행 데이터를 먼저 연동하는 경우, 비교 대상인 카드 데이터가 아직 DB에 없어 isDuplicateOfCardApproval 검사가 항상 통과하고 은행 기반 LedgerEntry가 그대로 저장됩니다. 이후 카드 데이터를 연동하더라도 이미 저장된 은행 내역은 기존 데이터로 남아 중복 제거가 되지 않는 것 같습니다.
카드 연동 시점에 기존 은행 기반 LedgerEntry 중 중복되는 내역을 찾아 삭제하거나 숨김 처리하고 카드 내역을 저장하는 방식도 한 번 검토해보시면 좋을 것 같습니다!
seamooll
left a comment
There was a problem hiding this comment.
그리고 커밋 메시지 띄어쓰기 컨벤션(feat: 커밋 메시지)도 함께 맞춰주시면 좋을 것 같습니다!
seamooll
left a comment
There was a problem hiding this comment.
ledger_entry 테이블에 canonical_key 컬럼이 NOT NULL로 추가되었는데 LedgerEntryRepositoryImpl의 bulkInsert 메서드 INSERT문에서 canonical_key가 포함되어 있지 않아 에러가 발생하는 것 같습니다. canonical_key 추가해주세요!
🔗 Related Issue
📝 Summary
CODEF에서 사용자의 거래내역과 카드내역을 연동해서 저장할 때 가계부 테이블에도 자동으로 저장되도록 코드를 수정
🔄 Changes
📸 API Test Results (Swagger)
✅ Checklist