-
Notifications
You must be signed in to change notification settings - Fork 1
MOSU-377 refactor: 신청 상세 내역 delete -> update로 수정 #378
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughSwitched related exam_application cleanup from hard delete to soft delete by updating a deleted flag via SQL. Parameter binding and guard for null/empty inputs remain. The subsequent deletion of application records is unchanged. No public API signatures were modified. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant S as Service
participant R as ApplicationJpaRepositoryImpl
participant DB as Database
S->>R: deleteApplications(applicationIds)
alt applicationIds empty/null
R-->>S: no-op
else proceed
R->>DB: UPDATE exam_application SET deleted = true WHERE application_id IN (:ids)
DB-->>R: rows updated
R->>DB: DELETE FROM application WHERE id IN (:ids)
DB-->>R: rows deleted
R-->>S: completion
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @chominju02, 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은 가상 계좌 입금 지연 시 신청 상세 내역이 의도치 않게 삭제되는 문제를 해결하기 위한 것입니다. 데이터의 실제 삭제 대신 논리적 삭제를 적용하여 데이터 무결성을 유지하고, 배치 처리 과정에서 발생할 수 있는 데이터 손실을 방지합니다.
Highlights
- 신청 상세 내역 처리 방식 변경: 가상 계좌 입금 지연으로 인한 신청 상세 내역 삭제 문제를 해결하기 위해, 실제 삭제 대신 deleted 컬럼을 true로 업데이트하도록 변경되었습니다.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
이 PR은 배치 처리 시 신청 내역이 삭제되는 문제를 해결하기 위해 물리적 삭제(hard delete)를 논리적 삭제(soft delete)로 변경하는 것을 목표로 하고 있습니다. exam_application 테이블에 대해 soft delete를 적용한 것은 좋은 방향입니다. 하지만 검토 결과, application 테이블은 여전히 물리적으로 삭제되고 있어 데이터 정합성 문제가 발생할 우려가 있습니다. 또한, 변경된 쿼리의 의도를 명확히 하기 위해 변수명을 수정하는 것을 제안합니다. 자세한 내용은 각 코드 라인에 남긴 의견을 참고해 주세요.
| .toList(); | ||
|
|
||
| String deleteExamApplicationSql = "DELETE FROM exam_application WHERE application_id IN (:applicationIds)"; | ||
| String deleteExamApplicationSql = "UPDATE exam_application SET deleted = true WHERE application_id IN (:applicationIds)"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exam_application 테이블에 대해 soft delete를 적용하도록 수정되었지만, 이어서 실행되는 코드(33행)에서는 application 테이블의 데이터를 물리적으로 삭제(hard delete)하고 있습니다. 이 경우, 부모 테이블인 application의 데이터가 사라져 exam_application에 남은 데이터가 고아 레코드(orphaned record)가 되어 데이터 정합성에 문제가 발생할 수 있습니다. PR의 본래 목적인 데이터 보존을 위해서는 application 테이블에 대해서도 일관되게 soft delete를 적용하는 것을 강력히 권장합니다.
| .toList(); | ||
|
|
||
| String deleteExamApplicationSql = "DELETE FROM exam_application WHERE application_id IN (:applicationIds)"; | ||
| String deleteExamApplicationSql = "UPDATE exam_application SET deleted = true WHERE application_id IN (:applicationIds)"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/life/mosu/mosuserver/infra/persistence/jpa/ApplicationJpaRepositoryImpl.java (1)
33-35: Orphanexam_applicationrows due to hard DELETENo DB‐level foreign key exists on
exam_application.application_id, soDELETE FROM application WHERE application_id IN (:applicationIds)will succeed but leave dangling
exam_applicationrows withdeleted = false. Soft-delete the parent and cascade to its children:File: src/main/java/life/mosu/mosuserver/infra/persistence/jpa/ApplicationJpaRepositoryImpl.java (lines 33–35)
[suggested fix]- String deleteApplicationSql = "DELETE FROM application WHERE application_id IN (:applicationIds)"; - namedParameterJdbcTemplate.update(deleteApplicationSql, - new MapSqlParameterSource("applicationIds", applicationIds)); + String softDeleteApplicationSql = "UPDATE application SET deleted = true WHERE application_id IN (:applicationIds)"; + namedParameterJdbcTemplate.update(softDeleteApplicationSql, + new MapSqlParameterSource("applicationIds", applicationIds)); + String softDeleteExamApplicationSql = "UPDATE exam_application SET deleted = true WHERE application_id IN (:applicationIds)"; + namedParameterJdbcTemplate.update(softDeleteExamApplicationSql, + new MapSqlParameterSource("applicationIds", applicationIds));
🧹 Nitpick comments (3)
src/main/java/life/mosu/mosuserver/infra/persistence/jpa/ApplicationJpaRepositoryImpl.java (3)
29-31: Rename the local to reflect UPDATE, not DELETEMinor clarity win; keeps code intent obvious.
- String deleteExamApplicationSql = "UPDATE exam_application SET deleted = true WHERE application_id IN (:applicationIds)"; - namedParameterJdbcTemplate.update(deleteExamApplicationSql, + String softDeleteExamApplicationSql = "UPDATE exam_application SET deleted = true WHERE application_id IN (:applicationIds)"; + namedParameterJdbcTemplate.update(softDeleteExamApplicationSql, new MapSqlParameterSource("applicationIds", applicationIds));
29-35: Indexing and soft-delete hygiene
- Ensure an index exists for
exam_application(application_id)and, if queries filter by live rows, consider(application_id, deleted)or a partial indexWHERE deleted = false(Postgres).- Consider adding
deleted_at(and optionallydeleted_by) to aid audits and retention policies.Example (Postgres):
CREATE INDEX IF NOT EXISTS idx_exam_app_application_id ON exam_application(application_id); CREATE INDEX IF NOT EXISTS idx_exam_app_application_id_live ON exam_application(application_id) WHERE deleted = false;
21-31: Avoid oversized IN lists in batch runsIf
applicationIdscan be large, chunk updates to stay under driver/DB limits and reduce lock pressure.Example snippet:
private static <T> List<List<T>> chunks(List<T> list, int size) { List<List<T>> out = new java.util.ArrayList<>(); for (int i = 0; i < list.size(); i += size) { out.add(list.subList(i, Math.min(i + size, list.size()))); } return out; } // inside method for (List<Long> chunk : chunks(applicationIds, 1000)) { namedParameterJdbcTemplate.update( "UPDATE exam_application SET deleted = true WHERE application_id IN (:applicationIds)", new MapSqlParameterSource("applicationIds", chunk)); // and the parent statement accordingly }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/main/java/life/mosu/mosuserver/infra/persistence/jpa/ApplicationJpaRepositoryImpl.java(1 hunks)
🔇 Additional comments (1)
src/main/java/life/mosu/mosuserver/infra/persistence/jpa/ApplicationJpaRepositoryImpl.java (1)
29-31: Soft-delete swap for exam_application: LGTMSwitching to
UPDATE ... SET deleted = truealigns with the PR objective and keeps records for audit.
✨ 구현한 기능
📢 논의하고 싶은 내용
🎸 기타
Summary by CodeRabbit
New Features
Bug Fixes
Chores