-
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
[feat] 메인 리스트 뷰 API 연동 및 전체적인 플로우 점검 #214
Conversation
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.
고생하셨습니당💖
category = category, | ||
teamId = teamId, | ||
order = order | ||
).data.meetings.map { meeting -> meeting.toPingleEntity() } |
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.
랭킹뷰에서는 Dto 내에서 List로 매핑해주는 함수를 작성해주셨는데 이번 api 연동에서는 RepositoryImpl에서 List로 매핑해주신 특별한 이유가 있나요?
@Serializable
data class ResponseRankingDto(
@SerialName("meetingCount")
val meetingCount: Int,
@SerialName("locations")
val locations: List<ResponseRankingLocationDto>
) {
@Serializable
data class ResponseRankingLocationDto(
@SerialName("name")
val name: String,
@SerialName("latestVisitedDate")
val latestVisitedDate: List<Int>,
@SerialName("locationCount")
val locationCount: Int
) {
fun toRankingLocationEntity() = RankingLocationEntity(
name = this.name,
latestVisitedDate = this.latestVisitedDate.subList(DATE_START_INDEX, DATE_END_INDEX),
locationCount = this.locationCount
)
}
fun toRankingEntity() = RankingEntity(
meetingCount = this.meetingCount,
locations = this.locations.map { responseRankingLocationDto -> responseRankingLocationDto.toRankingLocationEntity() }
)
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.
랭킹은 RankingEntity 내부에 리스트가 있는 형태이기 때문에 DTO에서 변환을 해주었고 여기는 리스트 자체만 있어서 impl에서 해줬어요 !
val result = runCatching { | ||
mapRemoteDataSource.getPingleList( | ||
mapRemoteDataSource.getMapPingleList( |
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.
바뀐 이름이 훨씬 직관적이구 좋은 것 같아욥 Pingle은 마이핑글이나 랭킹뷰에 뜨는 애들도 사실은 핑글이니까는
@Query(ORDER) order: String | ||
): BaseResponse<ResponseMainListDto> | ||
|
||
companion object { |
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.
Service에서 VERSION이나 MEETINGS 같은 상수 객체들은 사실 여러 파일들에서 동시에 사용할 수 있는데 매 파일마다 선언해주는게 과연 좋은 걸까요? Service 파일 만들 때마다 고민한건데요.. const val로 선언하면 다른 클래스에서도 MainListService.VERSION 이런 식으로 import해서 쓸 수 있잖아요?! 그러면 이런 식으로 companion object들을 한 파일 만들어서 ServiceCompanion 이런 클래스에 넣어두고 사용하면 더 좋지 않을까라는 생각을 해봤어요 그러면 클래스간 의존도야 물론 높아지긴 하겠지만 Service 파일마다 중복된 내용이 너무 많아서.. 뭐가 더 좋은걸까요?
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.
이거 나중에 상수화 한 거 파일 하나로 만들어서 불러 쓰는 게 좋을 것 같삼요 ㅜ 리팩토링 때 ㄱㄱ 저도 레퍼런스 좀 찾아보겠슴
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.
저도 파일 하나에서 관리하는게 맞는거같아요!
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.
리팩 때 고고 합시둥 이번 스프린트 끝나고 리팩 뭐 할지 회의 고고!
@@ -37,6 +39,10 @@ abstract class DataSourceModule { | |||
@Singleton | |||
abstract fun bindsJoinGroupRemoteDataSource(joinGroupRemoteDataSourceImpl: JoinGroupRemoteDataSourceImpl): JoinGroupRemoteDataSource | |||
|
|||
@Binds | |||
@Singleton | |||
abstract fun bindsMainListRemoteDataSource(mainListRemoteDateSourceImpl: MainListRemoteDateSourceImpl): MainListRemoteDataSource |
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.
Date -> Data 오타 났숨당
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.
예리핑
) { | ||
fun updateMainListPingleModel() = this.copy( | ||
pingleEntity = this.pingleEntity.copy( | ||
curParticipants = if (this.pingleEntity.isParticipating) this.pingleEntity.curParticipants + 1 else this.pingleEntity.curParticipants - 1, |
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.
요기 +1, -1 하는 부분 상수화하면 더 좋을 것 같아용!
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.
넹구 반영하겠슴
) | ||
combine( | ||
homeViewModel.searchWord.flowWithLifecycle(viewLifecycleOwner.lifecycle) | ||
.distinctUntilChanged(), |
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.
여기두 이 distinctUntilChanged 호출 안 해주면 값 안 뱉어내나용?
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.
값을 계속 뱉어내서 문제입니다,, onResume 될 때마다요
is UiState.Success -> { | ||
mainListPingleListUiState.data.let { mainListPingleList -> | ||
mainListAdapter.submitList(mainListPingleList) | ||
binding.rvMainList.smoothScrollToPosition(TOP) |
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.
여기는 2개니까 with(binding) 쓰면 좋을 것 같아요!
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.
yes ~
isExpanded.addOnPropertyChangedCallback(object : | ||
Observable.OnPropertyChangedCallback() { | ||
override fun onPropertyChanged(sender: Observable?, propertyId: Int) { | ||
setCardExpandable(isExpanded = isExpanded.get()) |
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.
카드 펼쳐지고 접히는 거 다시 봐도 댕멋지다!!!!
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.
ㅋㅋ 사실 별거 없슴요
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.
고생고생핑~
): BaseResponse<List<ResponsePinListDto>> = | ||
): BaseResponse<List<ResponsePinDto>> = |
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.
오홍 디테일을 챙기셨네요
@Query(ORDER) order: String | ||
): BaseResponse<ResponseMainListDto> | ||
|
||
companion object { |
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.
저도 파일 하나에서 관리하는게 맞는거같아요!
is UiState.Empty -> { | ||
binding.fabHomeChange.visibility = View.VISIBLE | ||
} | ||
|
||
is UiState.Success -> { | ||
binding.fabHomeChange.visibility = | ||
if (uiState.data.second.isNotEmpty()) View.INVISIBLE else View.VISIBLE | ||
} |
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.
is UiState.Empty -> { | |
binding.fabHomeChange.visibility = View.VISIBLE | |
} | |
is UiState.Success -> { | |
binding.fabHomeChange.visibility = | |
if (uiState.data.second.isNotEmpty()) View.INVISIBLE else View.VISIBLE | |
} | |
is UiState.Empty -> binding.fabHomeChange.visibility = View.VISIBLE | |
is UiState.Success -> binding.fabHomeChange.visibility = | |
if (uiState.data.second.isNotEmpty()) View.INVISIBLE else View.VISIBLE |
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.
반영할게염 ~
Related issue 🛠
Work Description ✏️
Screenshot 📸
Screen_recording_20240303_215955.mp4
Uncompleted Tasks 😅
To Reviewers 📢