-
Notifications
You must be signed in to change notification settings - Fork 7
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
[Feature/#869] 오늘의 솝마디 대시보드 기능 구현 #872
Changes from 21 commits
1aa4880
427fcc1
1a75a66
e265c4e
3b483ac
4241e3f
533a29f
f655b50
e26989c
5950cec
71001df
1e42c44
625d11e
430b830
83c0641
8d86bcb
340c78e
2f9ffff
69a0acc
8981910
a7c5b50
9384ee5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
/build |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
/* | ||
* MIT License | ||
* Copyright 2023-2024 SOPT - Shout Our Passion Together | ||
* | ||
* Permission is hereby granted, free of charge, to any person obtaining a copy | ||
* of this software and associated documentation files (the "Software"), to deal | ||
* in the Software without restriction, including without limitation the rights | ||
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | ||
* copies of the Software, and to permit persons to whom the Software is | ||
* furnished to do so, subject to the following conditions: | ||
* | ||
* The above copyright notice and this permission notice shall be included in all | ||
* copies or substantial portions of the Software. | ||
* | ||
* https://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE | ||
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE | ||
* SOFTWARE. | ||
*/ | ||
|
||
plugins { | ||
sopt("feature") | ||
} | ||
|
||
android { | ||
namespace = "org.sopt.official.data.fortune" | ||
} | ||
|
||
dependencies { | ||
implementation(projects.domain.fortune) | ||
implementation(projects.core.network) | ||
implementation(projects.core.common) | ||
implementation(platform(libs.okhttp.bom)) | ||
implementation(libs.bundles.okhttp) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
<?xml version="1.0" encoding="utf-8"?> | ||
<!-- | ||
MIT License | ||
|
||
Copyright (c) 2023-2024 SOPT Makers | ||
|
||
Permission is hereby granted, free of charge, to any person obtaining a copy | ||
of this software and associated documentation files (the "Software"), to deal | ||
in the Software without restriction, including without limitation the rights | ||
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | ||
copies of the Software, and to permit persons to whom the Software is | ||
furnished to do so, subject to the following conditions: | ||
|
||
The above copyright notice and this permission notice shall be included in all | ||
copies or substantial portions of the Software. | ||
|
||
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE | ||
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE | ||
SOFTWARE. | ||
--> | ||
<manifest> | ||
|
||
</manifest> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
package org.sopt.official.data.fortune.di | ||
|
||
import dagger.Module | ||
import dagger.Provides | ||
import dagger.hilt.InstallIn | ||
import dagger.hilt.components.SingletonComponent | ||
import org.sopt.official.common.di.AppRetrofit | ||
import org.sopt.official.data.fortune.remote.api.FortuneApi | ||
import retrofit2.Retrofit | ||
import retrofit2.create | ||
import javax.inject.Singleton | ||
|
||
@Module | ||
@InstallIn(SingletonComponent::class) | ||
internal object ApiModule { | ||
|
||
@Provides | ||
@Singleton | ||
internal fun provideFortuneApi(@AppRetrofit(true) retrofit: Retrofit): FortuneApi = retrofit.create() | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
package org.sopt.official.data.fortune.di | ||
|
||
import dagger.Binds | ||
import dagger.Module | ||
import dagger.hilt.InstallIn | ||
import dagger.hilt.components.SingletonComponent | ||
import org.sopt.official.data.fortune.repository.DefaultFortuneRepository | ||
import org.sopt.official.domain.fortune.repository.FortuneRepository | ||
import javax.inject.Singleton | ||
|
||
@Module | ||
@InstallIn(SingletonComponent::class) | ||
internal interface RepositoryModule { | ||
|
||
@Binds | ||
@Singleton | ||
abstract fun bindDefaultFortuneRepository(defaultFortuneRepository: DefaultFortuneRepository): FortuneRepository | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
package org.sopt.official.data.fortune.mapper | ||
|
||
import org.sopt.official.data.fortune.remote.response.TodayFortuneCardResponse | ||
import org.sopt.official.data.fortune.remote.response.TodayFortuneWordResponse | ||
import org.sopt.official.domain.fortune.model.TodayFortuneCard | ||
import org.sopt.official.domain.fortune.model.TodayFortuneWord | ||
|
||
internal fun TodayFortuneCardResponse.toDomain(): TodayFortuneCard = TodayFortuneCard( | ||
description = description, | ||
imageColorCode = imageColorCode, | ||
imageUrl = imageUrl, | ||
name = name, | ||
) | ||
|
||
internal fun TodayFortuneWordResponse.toDomain(): TodayFortuneWord = TodayFortuneWord( | ||
userName = userName, | ||
title = title, | ||
) | ||
Comment on lines
+8
to
+18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 저는 mapper를 클래스의 하위 함수로 만들어서 같이 두는 편인데 따로 분리하신 이유가 있나요?? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 스타일 차이라고 생각하는데요 data class의 주 역할은 model로써 데이터를 wrapping하는 역할이지 mapping까지의 책임은 없다고 생각해요. 해당 model을 wrapping 하고 싶은 주체가 매핑을 하는게 맞을 것 같습니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 저도 처음에는 굳이 분리할 필요 없다고 생각했는데, 추후에 복잡한 로직이 들어갈 수도 있고 명시적으로 분리해서 표시하는 게 직관적일 것 같아서 최근에는 분리해두려고 합니다. 스타일 차이인 것 같긴 하네요! |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
package org.sopt.official.data.fortune.remote.api | ||
|
||
import org.sopt.official.data.fortune.remote.response.TodayFortuneCardResponse | ||
import org.sopt.official.data.fortune.remote.response.TodayFortuneWordResponse | ||
import retrofit2.http.GET | ||
import retrofit2.http.Query | ||
|
||
internal interface FortuneApi { | ||
|
||
@GET("fortune/word") | ||
suspend fun getTodayFortuneWord( | ||
@Query("todayDate") todayDate: String, | ||
): TodayFortuneWordResponse | ||
|
||
@GET("fortune/card/today") | ||
suspend fun getTodayFortuneCard(): TodayFortuneCardResponse | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
package org.sopt.official.data.fortune.remote.response | ||
|
||
import kotlinx.serialization.SerialName | ||
import kotlinx.serialization.Serializable | ||
|
||
@Serializable | ||
internal data class TodayFortuneCardResponse( | ||
@SerialName("description") | ||
val description: String, | ||
@SerialName("imageColorCode") | ||
val imageColorCode: String, | ||
@SerialName("imageUrl") | ||
val imageUrl: String, | ||
@SerialName("name") | ||
val name: String, | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
package org.sopt.official.data.fortune.remote.response | ||
|
||
import kotlinx.serialization.SerialName | ||
import kotlinx.serialization.Serializable | ||
|
||
@Serializable | ||
internal data class TodayFortuneWordResponse( | ||
@SerialName("userName") | ||
val userName: String, | ||
@SerialName("title") | ||
val title: String, | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
package org.sopt.official.data.fortune.repository | ||
|
||
import org.sopt.official.data.fortune.mapper.toDomain | ||
import org.sopt.official.data.fortune.remote.api.FortuneApi | ||
import org.sopt.official.domain.fortune.model.TodayFortuneCard | ||
import org.sopt.official.domain.fortune.model.TodayFortuneWord | ||
import org.sopt.official.domain.fortune.repository.FortuneRepository | ||
import javax.inject.Inject | ||
|
||
internal class DefaultFortuneRepository @Inject constructor( | ||
private val fortuneApi: FortuneApi, | ||
) : FortuneRepository { | ||
|
||
override suspend fun fetchTodayFortuneWord(date: String): TodayFortuneWord = fortuneApi.getTodayFortuneWord(date).toDomain() | ||
|
||
override suspend fun fetchTodayFortuneCard(): TodayFortuneCard = fortuneApi.getTodayFortuneCard().toDomain() | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
/build |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
/* | ||
* MIT License | ||
* Copyright 2023-2024 SOPT - Shout Our Passion Together | ||
* | ||
* Permission is hereby granted, free of charge, to any person obtaining a copy | ||
* of this software and associated documentation files (the "Software"), to deal | ||
* in the Software without restriction, including without limitation the rights | ||
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | ||
* copies of the Software, and to permit persons to whom the Software is | ||
* furnished to do so, subject to the following conditions: | ||
* | ||
* The above copyright notice and this permission notice shall be included in all | ||
* copies or substantial portions of the Software. | ||
* | ||
* https://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE | ||
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE | ||
* SOFTWARE. | ||
*/ | ||
|
||
plugins { | ||
sopt("kotlin") | ||
} | ||
|
||
kotlin { | ||
jvmToolchain(17) | ||
} | ||
|
||
dependencies { | ||
implementation(libs.javax.inject) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
package org.sopt.official.domain.fortune.model | ||
|
||
data class TodayFortuneCard( | ||
val description: String, | ||
val imageColorCode: String, | ||
val imageUrl: String, | ||
val name: String, | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
package org.sopt.official.domain.fortune.model | ||
|
||
data class TodayFortuneWord( | ||
val userName: String, | ||
val title: String, | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
package org.sopt.official.domain.fortune.repository | ||
|
||
import org.sopt.official.domain.fortune.model.TodayFortuneCard | ||
import org.sopt.official.domain.fortune.model.TodayFortuneWord | ||
|
||
interface FortuneRepository { | ||
suspend fun fetchTodayFortuneWord(date: String): TodayFortuneWord | ||
suspend fun fetchTodayFortuneCard(): TodayFortuneCard | ||
} |
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
@@ -0,0 +1,14 @@ | ||||
package org.sopt.official.domain.fortune.usecase | ||||
|
||||
import java.text.SimpleDateFormat | ||||
import java.util.Locale | ||||
import javax.inject.Inject | ||||
|
||||
class GetTodayDateUseCase @Inject constructor() { | ||||
|
||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 그리고 이정도 로직은 UseCase 굳이 만들어야하나 하는 생각이 있긴 합니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @l2hyunwoo 삽인정입니다. 근데 뭐 UI에서 사용할 로직도 아니긴해서 ideal하겐 도메인이 적합할 것 같아서 분리했습니다. |
||||
operator fun invoke(): String { | ||||
val currentDate = System.currentTimeMillis() | ||||
|
||||
return SimpleDateFormat("yyyy-MM-dd", Locale.KOREAN).format(currentDate) | ||||
} | ||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
package org.sopt.official.domain.fortune.usecase | ||
|
||
import org.sopt.official.domain.fortune.model.TodayFortuneWord | ||
import org.sopt.official.domain.fortune.repository.FortuneRepository | ||
import javax.inject.Inject | ||
|
||
class GetTodayFortuneUseCase @Inject constructor( | ||
private val fortuneRepository: FortuneRepository, | ||
private val getTodayDateUseCase: GetTodayDateUseCase, | ||
) { | ||
|
||
suspend operator fun invoke(): TodayFortuneWord = fortuneRepository.fetchTodayFortuneWord(getTodayDateUseCase()) | ||
} |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. JUnit5 Compose 테스트 지원안하나? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. 개인적으로 생각하는건, 우리의 코드베이스가 JUnit5로 맞춰져있으면 코드 통일성도 올라가고 Test 데이터 제공할때도 CsvSource나 ParamterProvider였나? 암튼 함수로 데이터 제공할 수 있어서 좀 더 편하게 코드를 생산할 수 있을거라 기대해서 테스트 상관없이 JUnit5로 스펙이 맞춰지면 좋을 것 같다고 생각하걸랑 @sopt-makers/android 어떻게 생각하시는지요들? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. @s9hn @chattymin UI 테스트 좋은데 개발자가 손수 적는 것보다 스크린샷 테스팅해서 PR에 올라오는게 생산성 좋을듯 (동민아 해줘) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 근데 또 개인적으론, 스크린샷 테스팅도 배웠는데 아직 많이 지원이 .....미숙합니다. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
package org.sopt.official.feature.fortune.fortuneDetail | ||
|
||
import androidx.compose.foundation.layout.PaddingValues | ||
import androidx.compose.ui.semantics.SemanticsProperties | ||
import androidx.compose.ui.semantics.getOrNull | ||
import androidx.compose.ui.test.assertIsDisplayed | ||
import androidx.compose.ui.test.junit4.createComposeRule | ||
import androidx.compose.ui.test.onNodeWithContentDescription | ||
import androidx.compose.ui.test.onNodeWithText | ||
import org.junit.Rule | ||
import org.junit.Test | ||
import org.sopt.official.designsystem.SoptTheme | ||
import org.sopt.official.feature.fortune.feature.fortuneDetail.FortuneDetailScreen | ||
import org.sopt.official.feature.fortune.feature.fortuneDetail.model.FortuneDetailUiState | ||
|
||
internal class FortuneDetailScreenTest { | ||
|
||
@get:Rule | ||
val composeRule = createComposeRule() | ||
|
||
@Test | ||
fun 서버통신이_성공하면_이름_솝마디_날짜가_노출된다() { | ||
// given: | ||
val date = "2024-09-26" | ||
val name = "이현우" | ||
val content = "안녕하세요안녕하세요안녕하세요안녕하세요안녕하세요" | ||
|
||
// when: | ||
|
||
composeRule.setContent { | ||
SoptTheme { | ||
FortuneDetailScreen( | ||
paddingValue = PaddingValues(), | ||
date = date, | ||
onFortuneAmuletClick = { }, | ||
uiState = FortuneDetailUiState.TodaySentence( | ||
userName = name, | ||
content = content, | ||
) | ||
) | ||
} | ||
} | ||
|
||
// then: | ||
val todayFortune = composeRule.onNodeWithContentDescription("todaySentence") | ||
.fetchSemanticsNode().config.getOrNull(SemanticsProperties.Text)?.joinToString(separator = "").orEmpty() | ||
|
||
composeRule.waitForIdle() | ||
|
||
composeRule.onNodeWithText(date).assertIsDisplayed() | ||
assert(todayFortune.contains(name)) | ||
assert(todayFortune.contains(content)) | ||
} | ||
} |
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.
data 레이어인데
sopt("feature")
를 사용하는게 안맞는거 같네요.다른 data 모듈들도 보니 다 feature용을 쓰고있네요. feature에는 최근에 추가한 immutableList처럼 정말 feature에서만 쓰이는 것들이 있는데 data와 공유하기 보다는 data에 해당하는 친구를 새로 만드는게 좋을 것 같아요. data에 해당하는 custom plugin 조만간 만들어서 PR올리겠슴다.