[✨feat] Banner 구현 #41
Conversation
| @Column(length = 255) | ||
| private var imageUrl: String, | ||
| @Column(length = 255) | ||
| private var link: String, |
There was a problem hiding this comment.
imageUrl과 link 둘 다 외부 리소스를 참조하는 값으로 보여서 한 번 여쭤보고 싶었어요!
현재는 각각 private var로 선언되어 있는데, 혹시 외부에서 해당 값을 직접 읽거나 사용하는 케이스는 없다고 판단하신 걸까요?
또는 추후에 URL 포맷 검증이나, 접근 제한 같은 로직이 붙게 된다면
ImageUrl, Link와 같은 VO로 분리하는 것도 고려해볼 수 있을 것 같아서요 🙂
지금 구조도 충분히 깔끔하지만, 혹시 어떤 방향성을 가지고 계셨는지 궁금했습니다!
There was a problem hiding this comment.
Banner 엔티티의 특징에 대해 더 고민해봤어야 했던 것 같네요😅
말씀해 주신 imageUrl과 link는 public 하게 두고 VO로 분리해 두겠습니다! 상세 로직은 추후 개발하면서 더 작성하도록 할게요!
| @Column | ||
| private var priority: Int, | ||
| ) : BaseRootEntity() |
There was a problem hiding this comment.
priority는 배너 노출 순서 같은 역할을 하게 될 값으로 보이는데요!
혹시 기본값을 별도로 두지 않으신 건, 반드시 명시적으로 설정되어야 한다고 판단하신 걸까요?
아니면 추후 비즈니스 로직에서 정렬 기준 등을 따로 분리해서 다루실 계획이 있으셨을지 궁금했어요.
우선순위 관련 값은 은근히 서비스 흐름에 영향을 주는 경우가 많아서, 혹시 고려하신 방향이 있을까 궁금해졌습니다 🙂
There was a problem hiding this comment.
예리한 조언 감사합니다!
아직 배너가 어떤 점을 기준으로 정렬되고 있는지 제대로 파악이 되지 않아 별도로 두지 않고 있었습니다!
해당 부분은 조금 더 알아보고 적용해보도록 하겠습니다..!
There was a problem hiding this comment.
Pull Request Overview
This PR implements a new Banner entity as well as its associated Link and ImageUrl value objects, all built using JPA annotations to map the entity properties.
- Introduces the Banner entity inheriting from BaseRootEntity
- Adds Link and ImageUrl data classes with a length constraint on their string values
- Maps embedded fields with attribute override annotations in Banner
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/main/kotlin/com/terning/server/kotlin/domain/banner/Link.kt | New Link data class for encapsulating URL values. |
| src/main/kotlin/com/terning/server/kotlin/domain/banner/ImageUrl.kt | New ImageUrl data class for encapsulating image URL values. |
| src/main/kotlin/com/terning/server/kotlin/domain/banner/Banner.kt | New Banner entity incorporating embedded Link and ImageUrl. |
Comments suppressed due to low confidence (1)
src/main/kotlin/com/terning/server/kotlin/domain/banner/Banner.kt:25
- The Banner entity currently lacks tests to verify its JPA mappings and behavior. Consider adding unit or integration tests to ensure the entity functions as expected in production.
@Column
private var priority: Int,
📄 Work Description
💭 Thoughts
✅ Testing Result
🗂 Related Issue