-
Notifications
You must be signed in to change notification settings - Fork 2
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/#48 프로필 화면 구현 #139
The head ref may contain hidden characters: "Feature/#48-\uD504\uB85C\uD544_\uD654\uBA74_\uAD6C\uD604"
Changes from 18 commits
c87bc17
16e263e
9d88772
a64be8f
3534dd9
e2c2c6f
fe77950
de86666
a50e817
5a3192f
4577a10
e66ba27
b1e5ded
81d00ed
a214fd6
8caac36
8b6ae13
5b5e0c5
94cb706
c44439b
bacd0ee
933d0bd
aace342
0319245
696f2ac
b27e3a5
11582ed
8470ca7
0828bc2
b7544c9
006b04a
e69b430
a218c2c
d3948ce
d4781c6
e4dba6d
6a94947
f9f90d2
ef7d87a
3913d17
bd24a29
aeb2b30
f87e92e
5a91bca
127a874
f35a5de
bf98e2b
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,7 @@ | ||
package com.emmsale.data.activity | ||
|
||
data class Activity1( | ||
val id: Long, | ||
val activityType: ActivityType, | ||
val name: String | ||
tmdgh1592 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
package com.emmsale.data.activity | ||
|
||
enum class ActivityType { | ||
EDUCATION, CLUB, EVENT, JOB | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
package com.emmsale.data.member | ||
|
||
import com.emmsale.data.activity.Activity1 | ||
import com.emmsale.data.activity.ActivityType | ||
|
||
data class Member1( | ||
val id: Long, | ||
val name: String, | ||
val description: String, | ||
val imageUrl: String, | ||
val activities: Map<ActivityType, List<Activity1>>, | ||
) { | ||
operator fun get(activityType: ActivityType): List<Activity1> = | ||
activities[activityType] ?: listOf() | ||
ki960213 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,16 +1,46 @@ | ||
package com.emmsale.data.member | ||
|
||
import com.emmsale.data.common.ApiError | ||
import com.emmsale.data.common.ApiResult | ||
import com.emmsale.data.common.ApiSuccess | ||
import com.emmsale.data.common.handleApi | ||
import com.emmsale.data.member.dto.MemberApiModel | ||
import kotlinx.coroutines.CoroutineDispatcher | ||
import kotlinx.coroutines.Dispatchers | ||
import kotlinx.coroutines.async | ||
import kotlinx.coroutines.withContext | ||
|
||
class MemberRepositoryImpl( | ||
private val dispatcher: CoroutineDispatcher = Dispatchers.IO, | ||
private val memberService: MemberService, | ||
) : MemberRepository { | ||
|
||
override suspend fun fetchMember(memberId: Long): ApiResult<Member1> = withContext(dispatcher) { | ||
val memberResponseDeferred = async { memberService.fetchMember(memberId) } | ||
val activitiesResponseDeferred = async { memberService.fetchActivities() } | ||
tmdgh1592 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
val memberResponse = memberResponseDeferred.await() | ||
val activitiesResponse = activitiesResponseDeferred.await() | ||
ki960213 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
val memberApiModel = memberResponse.body() ?: return@withContext ApiError( | ||
memberResponse.code(), | ||
memberResponse.errorBody().toString() | ||
) | ||
val activitiesApiModel = activitiesResponse.body() ?: return@withContext ApiError( | ||
activitiesResponse.code(), | ||
activitiesResponse.errorBody().toString() | ||
) | ||
|
||
ApiSuccess( | ||
Member1( | ||
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. 아 그렇네요 제가 착각했습니다 ㅎㅎ 아니면 private 함수로 따로 뺴도 좋을 것 같아요. 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. 아니면
이런 방법은 어떤가요 |
||
id = memberApiModel.id, | ||
name = memberApiModel.name, | ||
description = memberApiModel.description, | ||
imageUrl = memberApiModel.imageUrl, | ||
activities = activitiesApiModel.associate { it.getActivityType() to it.toData() } | ||
) | ||
) | ||
} | ||
|
||
override suspend fun updateMember(member: Member): ApiResult<Unit> = withContext(dispatcher) { | ||
handleApi(memberService.updateMember(MemberApiModel.from(member))) { } | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,22 @@ | ||
package com.emmsale.data.member | ||
|
||
import com.emmsale.data.member.dto.MemberActivitiesBindActivityTypeApiModel | ||
import com.emmsale.data.member.dto.MemberApiModel | ||
import com.emmsale.data.member.dto.MemberWithoutActivitiesApiModel | ||
import retrofit2.Response | ||
import retrofit2.http.Body | ||
import retrofit2.http.GET | ||
import retrofit2.http.POST | ||
import retrofit2.http.Path | ||
|
||
interface MemberService { | ||
|
||
@GET("members/{memberId}") | ||
suspend fun fetchMember(@Path("memberId") memberId: Long): Response<MemberWithoutActivitiesApiModel> | ||
|
||
@GET("members/activities") | ||
suspend fun fetchActivities(): Response<List<MemberActivitiesBindActivityTypeApiModel>> | ||
|
||
@POST("/members") | ||
suspend fun updateMember(@Body member: MemberApiModel): Response<Unit> | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
package com.emmsale.data.member.dto | ||
|
||
import com.emmsale.data.activity.Activity1 | ||
import com.emmsale.data.activity.ActivityType | ||
import kotlinx.serialization.Serializable | ||
|
||
@Serializable | ||
data class MemberActivitiesBindActivityTypeApiModel( | ||
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. 제생각엔 이거 저희가 처음 말했던
이 데이터 모델을 공통으로 쓰는게 좋아보이긴 합니다... 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. 네이밍 짓기가 힘들지만 ApiModel의 클래스명은 꽤 자세히 적어야 한다고 생각합니다. 왜냐하면 외부 서비스의 메세지 형태는 매우 다양해질 수 있기 때문입니다. 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. 저도 길면 가독성이 떨어진다는 것에 공감하지만, 짧으면서 ApiModel의 정확한 의미를 반영하는 이름이 떠오르지 않네요ㅠㅠ |
||
val activityType: String, | ||
val memberActivityResponses: List<MemberActivityApiModel>, | ||
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. 이건 선택이지만, @SerialName(...)을 설정해두면, 혹시 나중에 프로퍼티명이 실수나 고의로 변경된다고 하더라도, 역/직렬화 이름은 그대로 유지되기 때문에 버그가 발생하는 상황을 방지할 수 있습니다! 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. dto는 딱 외부 서비스와의 통신을 위한 데이터 묶음이므로 프로퍼티 명을 외부 서비스와 관련 없는 이름으로 선언할 필요가 없다고 생각합니다. 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. 혹시 @SerialName의 값과 ApiModel의 속성명이 달라지는 경우가 있나요? 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. 저의 경우, 서버에서 제공하는 변수명은 remaningDays이지만, 클라이언트에서는 dDay로 사용하고 싶은 경우가 있었습니다. 그래서 클라이언트는 @SerialName 어노테이션을 통해 원하는 변수명으로 변환하여 사용하기도 합니다. 그 외에도 어노테이션을 사용하면 IDE에서 노란색으로 표기해주기 때문에 눈에 더 확 띈다는 점은 보너스이구요! 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. 서버에서 제공하는 변수명이 무슨 뜻인지 알기 힘든 경우 @SerialName을 사용하는 것이 좋아보이네요! |
||
) { | ||
fun toData(): List<Activity1> { | ||
return memberActivityResponses.map { | ||
Activity1( | ||
id = it.id, | ||
activityType = getActivityType(), | ||
name = it.name | ||
) | ||
} | ||
} | ||
|
||
fun getActivityType(): ActivityType = | ||
when (this.activityType) { | ||
"동아리" -> ActivityType.CLUB | ||
"컨퍼런스" -> ActivityType.EVENT | ||
"교육" -> ActivityType.EDUCATION | ||
"직무" -> ActivityType.JOB | ||
else -> throw IllegalStateException("회원의 활동 Json 데이터를 도메인 모델로 매핑하는 데 실패했습니다. 서버와 Api 스펙을 다시 상의해보세요.") | ||
ki960213 marked this conversation as resolved.
Show resolved
Hide resolved
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. ActivityType에게 역할을 부여해도 좋을 것 같습니다! 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. Api 요청 시 반환되는 값 중 "activityType": "동아리" 이러한 값들은 서버와 협상한 일종의 신호라고 생각합니다. 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. 현재 커디 프로젝트에는 도메인 모델이 없어서 ActivityType이 도메인 로직을 수행할 것이라고는 생각하지 못했네요! 또한, 위와 같은 상황을 위해, 데이터 모델과 도메인 모델이라는 개념이 각각 존재하는 것이 아닐까 싶네요 : ) |
||
} | ||
} | ||
|
||
@Serializable | ||
data class MemberActivityApiModel( | ||
val id: Long, | ||
val name: String, | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
package com.emmsale.data.member.dto | ||
|
||
import kotlinx.serialization.Serializable | ||
|
||
@Serializable | ||
data class MemberWithoutActivitiesApiModel( | ||
val id: Long, | ||
val name: String, | ||
val description: String = "", | ||
val imageUrl: String | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
package com.emmsale.presentation.ui.main | ||
|
||
import android.content.Context | ||
import android.content.Intent | ||
import androidx.appcompat.app.AppCompatActivity | ||
import android.os.Bundle | ||
import androidx.fragment.app.Fragment | ||
import com.emmsale.R | ||
import com.emmsale.databinding.ActivityMainBinding | ||
import com.emmsale.presentation.ui.main.events.EventsFragment | ||
import com.emmsale.presentation.ui.main.myProfile.MyProfileFragment | ||
|
||
class MainActivity : AppCompatActivity() { | ||
|
||
private val binding: ActivityMainBinding by lazy { | ||
ActivityMainBinding.inflate(layoutInflater) | ||
} | ||
override fun onCreate(savedInstanceState: Bundle?) { | ||
super.onCreate(savedInstanceState) | ||
setContentView(binding.root) | ||
|
||
initBottomNavigationView() | ||
} | ||
|
||
private fun initBottomNavigationView() { | ||
val bnvMain = binding.bnvMain | ||
ki960213 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
bnvMain.setOnItemSelectedListener { | ||
when(it.itemId) { | ||
R.id.mi_main_profile -> changeFragment(MyProfileFragment()) | ||
R.id.mi_main_home -> changeFragment(EventsFragment()) | ||
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. 탭을 선택할 때마다 매번 Fragment를 새로 생성하고 있습니다. EventsFragment는 매번 새로운 데이터를 불러오기 위함일 수 있지만, 혹시 위와 같이 구현하신 이유가 있을까요? 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. 이때 당시 새로고침 기능을 구현하지 않아서 프래그먼트를 매번 생성하도록 했습니다. 지금은 새로고침 기능을 구현했으니 show/hide로 바꾸겠습니다! 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. ViewPager 2를 이용하면 더 쉽게 구현 가능할것 같아요! ViewPager 사기입니다 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. 화면을 스와이프해서 넘기는 기능은 사용하지 않을거고 단지 탭을 클릭했을 때 다른 프래그먼트를 숨기는 걸 원하는 데, ViewPager를 사용하는 게 더 나을까요? 궁금합니다 😕 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. 화면이 2개라면 ViewPager까지 사용해야 하나 싶네요. |
||
} | ||
return@setOnItemSelectedListener true | ||
} | ||
|
||
bnvMain.selectedItemId = R.id.mi_main_home | ||
} | ||
|
||
private fun changeFragment(fragment: Fragment) { | ||
supportFragmentManager.beginTransaction().replace(R.id.fcv_main, fragment).commit() | ||
ki960213 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
companion object { | ||
fun startActivity(context: Context) { | ||
val intent = Intent(context, MainActivity::class.java) | ||
context.startActivity(intent) | ||
} | ||
Comment on lines
+60
to
+63
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. 아래와 같이 한 줄에 작성해도 좋을 것 같아요~! fun startActivity(context: Context) {
startActivity(Intent(context, MainActivity::class.java))
} 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,19 @@ | ||
package com.emmsale.presentation.ui.main.events | ||
|
||
import android.os.Bundle | ||
import androidx.fragment.app.Fragment | ||
import android.view.LayoutInflater | ||
import android.view.View | ||
import android.view.ViewGroup | ||
import com.emmsale.R | ||
|
||
class EventsFragment : Fragment() { | ||
|
||
override fun onCreateView( | ||
inflater: LayoutInflater, container: ViewGroup?, | ||
savedInstanceState: Bundle?, | ||
): View? { | ||
// Inflate the layout for this fragment | ||
return inflater.inflate(R.layout.fragment_events, container, false) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
package com.emmsale.presentation.ui.main.myProfile | ||
|
||
import android.os.Bundle | ||
import android.view.View | ||
import androidx.fragment.app.viewModels | ||
import com.emmsale.R | ||
import com.emmsale.databinding.FragmentMyProfileBinding | ||
import com.emmsale.presentation.base.fragment.BaseFragment | ||
import com.emmsale.presentation.common.extension.showToast | ||
import com.emmsale.presentation.ui.main.myProfile.adapter.ActivitiesAdapter | ||
import com.emmsale.presentation.ui.main.myProfile.adapter.JobsAdapter | ||
import com.emmsale.presentation.ui.main.myProfile.itemDecoration.ActivitiesAdapterDecoration | ||
|
||
class MyProfileFragment : BaseFragment<FragmentMyProfileBinding>() { | ||
override val layoutResId: Int = R.layout.fragment_my_profile | ||
|
||
private val viewModel: MyProfileViewModel by viewModels { | ||
MyProfileViewModel.factory | ||
} | ||
|
||
override fun onViewCreated(view: View, savedInstanceState: Bundle?) { | ||
super.onViewCreated(view, savedInstanceState) | ||
|
||
initDataBinding() | ||
initUiLogic() | ||
initRecyclerViews() | ||
|
||
viewModel.fetchMember() | ||
} | ||
|
||
private fun initDataBinding() { | ||
binding.viewModel = viewModel | ||
} | ||
|
||
private fun initUiLogic() { | ||
viewModel.uiState.observe(viewLifecycleOwner) { | ||
if (it.isError) { | ||
context?.showToast(it.errorMessage) | ||
} | ||
} | ||
} | ||
ki960213 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
private fun initRecyclerViews() { | ||
initJobsRecyclerView() | ||
initActivitiesRecyclerView() | ||
} | ||
|
||
private fun initJobsRecyclerView() { | ||
binding.rvMyprofileJobs.apply { | ||
adapter = JobsAdapter() | ||
itemAnimator = null | ||
} | ||
} | ||
|
||
private fun initActivitiesRecyclerView() { | ||
listOf( | ||
binding.rvMyprofileEducations, | ||
binding.rvMyprofileClubs, | ||
binding.rvMyprofileEvents | ||
).forEach { | ||
it.apply { | ||
adapter = ActivitiesAdapter() | ||
itemAnimator = null | ||
addItemDecoration(ActivitiesAdapterDecoration.getInstance()) | ||
} | ||
} | ||
} | ||
} |
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.
지금은 DataModel 을 이렇게 Activity1 과 Acitivty 이런 방식으로 두지만, 추후에 시간이 여유롭다면 Acitivity1 하나만 있는게 좋을 것 같습니다. 나중에Onboarding 을 Activity1 의 데이터 모델을 이용하여 해결했으면 좋겠네요.
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.
커디 프로젝트,,, 빠른 리팩토링이 시급합니다..