Refactor: Admin/Event/Ticket 미사용 컬럼·테이블 정리 (#327)#328
Conversation
Summary of ChangesHello, 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은 데이터베이스 감사 결과 실제로 사용되지 않는 테이블, 컬럼, enum 값 및 관련 코드를 정리하여 시스템의 불필요한 부분을 제거하고 코드베이스를 간소화하는 것을 목표로 합니다. 이를 통해 유지보수성을 향상시키고 잠재적인 혼란을 줄이며, 애플리케이션의 효율성을 높입니다. 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은 사용되지 않는 데이터베이스 테이블, 컬럼 및 관련 코드를 정리하는 대규모 리팩토링을 수행하여 코드베이스를 깔끔하게 단순화했습니다. 하지만 이벤트 보상 청구 로직에서 중요한 보안 회귀가 확인되었습니다. isCompleted 플래그와 같은 자격 확인이 제거되어, 어떤 사용자든 필요한 기준을 충족하지 않고도 활성 이벤트에 대한 보상을 청구할 수 있습니다. 보상 시스템의 잠재적 남용을 방지하기 위해 병합 전에 이 문제를 해결해야 합니다. 또한, AdminEventRewardFacade의 메서드 이름이 변경된 기능과 일치하지 않아 명확성 개선이 필요하며, InsightFacade의 역할이 축소되어 아키텍처 단순화를 위해 제거를 고려할 수 있습니다.
| private async grantFeedbackRewardByUserId( | ||
| eventCode: string, | ||
| params: GrantRewardByUserIdParams | ||
| ): Promise<{ userId: number; rewardStatus: EventRewardStatus; rewardGrantedAt: Date }> { |
There was a problem hiding this comment.
❌ 문제: 이 메서드의 이름인 grantFeedbackRewardByUserId는 피드백 제출(EventFeedbackSubmission) 관련 기능이 제거된 현재 컨텍스트와 맞지 않아 혼란을 줄 수 있습니다.
✅ 제안: 메서드의 역할이 수동 이벤트 보상 지급으로 변경되었으므로, grantManualRewardByUserId와 같이 그 역할을 더 명확하게 나타내는 이름으로 변경하는 것을 고려해 보세요. 이 메서드를 호출하는 grantReward 메서드 내부에서도 함께 수정이 필요합니다.
| private async grantFeedbackRewardByUserId( | |
| eventCode: string, | |
| params: GrantRewardByUserIdParams | |
| ): Promise<{ userId: number; rewardStatus: EventRewardStatus; rewardGrantedAt: Date }> { | |
| private async grantManualRewardByUserId( | |
| eventCode: string, | |
| params: GrantRewardByUserIdParams | |
| ): Promise<{ userId: number; rewardStatus: EventRewardStatus; rewardGrantedAt: Date }> { |
References
- 리뷰 포커스 우선순위 '중간'에 해당하는 '네이밍 컨벤션' 관련 사항입니다. 메서드의 이름이 실제 기능을 명확하게 반영하지 못하고 있어 수정이 필요합니다. (link)
| import { Injectable } from '@nestjs/common'; | ||
| import { CreateInsightLogReqDTO, InsightLogResDTO } from '../dtos/insight-log.dto'; | ||
| import { InsightService } from '../services/insight.service'; | ||
| import { EventRewardLifecycleFacade } from 'src/modules/event/application/facades/event-reward-lifecycle.facade'; | ||
| import { Transactional } from 'typeorm-transactional'; | ||
|
|
||
| @Injectable() | ||
| export class InsightFacade { | ||
| constructor( | ||
| private readonly insightService: InsightService, | ||
| private readonly eventRewardLifecycleFacade: EventRewardLifecycleFacade | ||
| ) {} | ||
| constructor(private readonly insightService: InsightService) {} | ||
|
|
||
| @Transactional() | ||
| async createInsight(userId: number, body: CreateInsightLogReqDTO): Promise<InsightLogResDTO> { | ||
| const insight = await this.insightService.createInsight(userId, body); | ||
| await this.eventRewardLifecycleFacade.trackInsightChallengeProgress(userId); | ||
| return insight; | ||
| return this.insightService.createInsight(userId, body); | ||
| } | ||
| } |
There was a problem hiding this comment.
❌ 문제: EventRewardLifecycleFacade 의존성 제거 후 InsightFacade가 InsightService를 그대로 호출만 하는 불필요한 레이어로 남게 되었습니다.
✅ 제안: InsightFacade를 제거하고 InsightController가 InsightService를 직접 사용하도록 리팩토링하는 것을 고려해 보세요. 아키텍처 가이드라인(93라인)에 따르면 Controller가 Service를 직접 의존하는 것이 허용됩니다. 이를 통해 불필요한 추상화 계층을 제거하고 코드를 단순화할 수 있습니다.
References
- 아키텍처 규칙에 따라 Facade는 여러 도메인 서비스를 조율하는 역할을 하지만, 현재 InsightFacade는 단순 호출 전달 역할만 하고 있어 불필요한 계층이 되었습니다. 아키텍처를 단순화하고 가독성을 높이기 위해 제거하는 것이 좋습니다. (link)
1 participant
Summary
DB 감사 결과 코드에서도 DB에서도 실제로 사용되지 않는 테이블, 컬럼, enum 값을 정리합니다.
Changes
테이블 삭제
event_feedback_submission테이블 및 관련 코드 전체 삭제 (entity, service, repository, enum 5개 파일)컬럼 삭제
event:description,max_participation,goal_config삭제event_participation:progress,is_completed,completed_at,last_progressed_at,granted_by,grant_reason삭제Enum 정리
EventRewardStatus:UNDER_REVIEW,REJECTED제거코드 정리
AdminEventRewardFacade: EventFeedbackSubmission 관련 로직 및externalSubmissionId제거EventRewardLifecycleFacade:trackInsightChallengeProgress제거 (미사용 진행형 이벤트 기능)EventRewardReadService:getProgressCard제거EventController:progress-card엔드포인트 제거EventProgressCardResDTO제거InsightFacade: EventRewardLifecycleFacade 의존성 제거AdminModule: TypeORM import 및 provider 정리Modal.js: 외부 제출 ID 필드 제거Type of Change
Target Environment
dev)Related Issues
Testing
pnpm run build— 0 issues)pnpm run lint)Checklist
docs/development/CODE_STYLE.md)docs/development/GIT_CONVENTIONS.md)docs/development/NAMING_CONVENTIONS.md)pnpm run build)pnpm run lint)Screenshots (Optional)
N/A
Additional Notes
ticket.paymentId,ticket.eventParticipationId,ticket.ticketGrantId는 감사 추적용으로 보류