Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes issues with the Newzet welcome newsletter functionality by refactoring the storage architecture and improving configuration management.
- Refactored S3Service to a more flexible StorageService supporting both AWS S3 and Supabase storage
- Made newsletter ID configurable via environment variables instead of hardcoded values
- Replaced hardcoded content paths and image URLs with dynamic database values
Reviewed Changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/com/newzet/api/config/S3TestConfig.java | Added Supabase test configuration properties alongside existing AWS S3 config |
| src/test/java/com/newzet/api/common/storage/StorageServiceTest.java | New comprehensive test suite for the StorageService with both AWS and Supabase scenarios |
| src/test/java/com/newzet/api/common/s3/S3ServiceTest.java | Removed old S3Service test file |
| src/main/resources/application.yml | Added Supabase storage configuration and newsletter ID configuration |
| src/main/java/com/newzet/api/welcome/orchestrator/WelcomeOrchestrator.java | Updated to use configurable newsletter ID and dynamic image URLs from database |
| src/main/java/com/newzet/api/welcome/api/WelcomeApi.java | Added @RequireAuth annotation for authentication |
| src/main/java/com/newzet/api/config/storage/StorageConfig.java | New unified storage configuration supporting both AWS S3 and Supabase |
| src/main/java/com/newzet/api/config/s3/S3Config.java | Removed old S3-only configuration |
| src/main/java/com/newzet/api/common/storage/StorageService.java | New service supporting multiple storage backends with conditional logic |
| src/main/java/com/newzet/api/common/s3/S3Service.java | Removed old S3-only service |
| src/main/java/com/newzet/api/article/orchestrator/ArticleOrchestrator.java | Updated to use new StorageService with storage type detection |
| src/main/java/com/newzet/api/article/domain/Article.java | Added isSaveInStorage() method to determine storage type based on file extension |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/main/java/com/newzet/api/welcome/orchestrator/WelcomeOrchestrator.java
Outdated
Show resolved
Hide resolved
| private static final String WELCOME_MAIL_URL = "newzet_content/welcome_letter"; | ||
| private static final String WELCOME_MAIL_IMAGE_URL = "https://newzet-lib.s3.ap-northeast-2.amazonaws.com/newsletter-image/trend_issue/nz_logo.webp"; | ||
|
|
||
| private static final String WELCOME_MAIL_URL = "welcome_letter"; |
There was a problem hiding this comment.
The WELCOME_MAIL_URL was changed from 'newzet_content/welcome_letter' to just 'welcome_letter', removing the directory prefix. Ensure this path change aligns with the actual storage structure.
| private static final String WELCOME_MAIL_URL = "welcome_letter"; | |
| private static final String WELCOME_MAIL_URL = "newzet_content/welcome_letter"; |
| public boolean isSaveInStorage() { | ||
| return contentUrl.endsWith(".html"); | ||
| } |
There was a problem hiding this comment.
The logic for determining storage type based on file extension '.html' seems fragile. Consider using a more explicit field or enum to indicate storage type rather than inferring from file extension.
…strator.java import 오류 수정 Co-authored-by: Copilot <[email protected]>
GitJIHO
left a comment
GitJIHO
left a comment
There was a problem hiding this comment.
고생하셨습니다 👍
dev에 머지후에 환경변수로 인한 서버 실행오류만 일어나지 않는다면 문제없을 것 같네요!
|
CI 터지는 부분이 배치처리 관련 부분인데 환경변수 때문이 아닌가 싶네요 😢 |
개요
배경
변경된 점
관련 이슈
close #136