카테고리별 푸시 on/off #341
Open
카테고리별 푸시 on/off #341
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SeonghaeJo
requested review from
asp345,
davin111 and
Hank-Choi
and removed request for
a team
Hank-Choi
reviewed
Feb 10, 2025
Comment on lines
9
to
33
| enum class PushCategory( | ||
| @JsonValue val value: Int, | ||
| ) { | ||
| LECTURE_UPDATE(1), | ||
| VACANCY_NOTIFICATION(2), | ||
| ; | ||
|
|
||
| companion object { | ||
| private val valueMap = PushCategory.entries.associateBy { e -> e.value } | ||
|
|
||
| fun getOfValue(value: Int): PushCategory? = valueMap[value] | ||
| } | ||
| } | ||
|
|
||
| @ReadingConverter | ||
| @Component | ||
| class PushCategoryReadConverter : Converter<Int, PushCategory> { | ||
| override fun convert(source: Int): PushCategory = PushCategory.getOfValue(source)!! | ||
| } | ||
|
|
||
| @WritingConverter | ||
| @Component | ||
| class PushCategoryWriteConverter : Converter<PushCategory, Int> { | ||
| override fun convert(source: PushCategory): Int = source.value | ||
| } |
There was a problem hiding this comment.
DB에 int 값으로 두면 파악하기 힘들다고 생각합니다.
걍 enum string 그대로 넣어도 될듯합니다.
원래 int로 되어있는건 구버전 대응용이에요
Comment on lines
10
to
18
| @Document(collection = "push_opt_out") | ||
| @CompoundIndex(def = "{ 'user_id': 1, 'push_category': 1 }") | ||
| data class PushOptOut( | ||
| @Id | ||
| val id: String? = null, | ||
| @Indexed | ||
| @Field("user_id", targetType = FieldType.OBJECT_ID) | ||
| val userId: String, | ||
| @Field("push_category") |
There was a problem hiding this comment.
그리고 DB 필드, 컬렉션명도 camel로 명명하자고 얘기한적 있긴 합니다. #70 (comment)
snake case 로 된건 다 옛날 옛적 컬렉션, 필드에요
Comment on lines
+12
to
+21
| data class PushOptOut( | ||
| @Id | ||
| val id: String? = null, | ||
| @Indexed | ||
| @Field("user_id", targetType = FieldType.OBJECT_ID) | ||
| val userId: String, | ||
| @Field("push_category") | ||
| @Indexed | ||
| val pushCategory: PushCategory, | ||
| ) |
There was a problem hiding this comment.
row 하나로 유저의 모든 설정값을 보면 좋을 것 같은데 현재는 좀 RDB스러운 접근이라고 생각합니다.
개인적으론 해당 컬렉션을 조회할 때 바로 어떤 유저가 어떤 푸시를 키고 껐는지 알 수 있었으면 좋겠습니다.
아래처럼 되면 좋을 것 같아요
ex)
{
"userId": "abcdefg123123",
"pushSetting": [
{
"category": "LECTURE_UPDATE",
"enable": true
},
{
"category": "VACANCY_NOTIFICATION",
"enable": false
},
...
]
}
Comment on lines
+83
to
+92
|
|
||
| override suspend fun sendCategoricalPush( | ||
| pushMessage: PushMessage, | ||
| userId: String, | ||
| pushCategory: PushCategory, | ||
| ) { | ||
| if (pushCategory == PushCategory.NORMAL || !pushOptOutRepository.existsByUserIdAndPushCategory(userId, pushCategory)) { | ||
| sendPush(pushMessage, userId) | ||
| } | ||
| } |
There was a problem hiding this comment.
PushService가 PushCategory에 대해 알 필요가 있을까요?
PushPreferenceService가 있는 것 같은데 유저 필터링은 거기서 해결해도 될 것 같아요
Comment on lines
+9
to
+11
| enum class PushCategory( | ||
| @JsonValue val value: Int, | ||
| ) { |
There was a problem hiding this comment.
이 PushCategory라는 이름이 헷갈리네요
NotificationType이랑 거의 비슷해서 헷갈려요
유저가 설정할 수 있는 값이라는 PushPreference같은 느낌이 이름에 같이 들어갔으면 좋겠습니다.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
push_opt_out DB collection 추가
기존 푸시 전송 로직 수정
SugangSnuNotificationService에서 반영됩니다.VacancyNotifierService에서 발생하는데, 여기서PushWithNotificationService를 사용하므로 해당 서비스를 수정하였습니다.Push on/off api
PushPreferenceHandler와PushPreferenceService중점으로 보시면 됩니다.docs, 테스트는 시간날 때 진행할게요