Skip to content
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/#68] university selection api #77

Merged
merged 24 commits into from
Jul 17, 2024

Conversation

0se0
Copy link
Member

@0se0 0se0 commented Jul 16, 2024

⛳️ Work Description

  • 대학 목록 정보 조회 get
  • 사용자 대학교 설정 정보 생성 post

📸 Screenshot

university-selection-apimp4.mp4

📢 To Reviewers

@0se0 0se0 added FEATURE ✨ 새로운 기능 구현 세영 🦌 labels Jul 16, 2024
@0se0 0se0 added this to the 1차 스프린트 서버 작업 milestone Jul 16, 2024
@0se0 0se0 requested review from chattymin and hyeeum July 16, 2024 18:20
@0se0 0se0 self-assigned this Jul 16, 2024
Copy link
Member

@chattymin chattymin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고하셨습니다
수정 부탁드릴게요

internal abstract class DataSourceModule {
@Binds
@Singleton
abstract fun bindUniversitySelectionDataSource(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
abstract fun bindUniversitySelectionDataSource(
abstract fun bindsUniversitySelectionDataSource(

오타수정해주세요

object ServiceModule {
@Provides
@Singleton
fun provideUniversitySelectionService(@JWT retrofit: Retrofit): UniversitySelectionService =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기도 오타 수정해주세요

Comment on lines 21 to 26
val dto = UniversitySelectionRequestDto(
universityId = request.universityId,
name = request.name,
longitude = request.longitude,
latitude = request.latitude
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이렇게 하지 마시고 mapper사용해주세요

Comment on lines 14 to 17
return runCatching {
val response = dataSource.getUniversitySelection()
response.data.universities.map { it.toUniversitySelectionEntity() }
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

불필요한 변수 만들지 말고 바로 해주세요

Comment on lines 7 to 30
@Serializable
data class University(
@SerialName("id")
val id: Int,
@SerialName("name")
val name: String,
@SerialName("longitude")
val longitude: Double,
@SerialName("latitude")
val latitude: Double
) {
fun toUniversitySelectionEntity() = UniversitySelectionEntity(
id = id,
name = name,
longitude = longitude,
latitude = latitude,
)
}

@Serializable
data class UniversitySelectionResponseDto(
@SerialName("universities")
val universities: List<University>
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

호출되는 순서대로 선언해주세요

Suggested change
@Serializable
data class University(
@SerialName("id")
val id: Int,
@SerialName("name")
val name: String,
@SerialName("longitude")
val longitude: Double,
@SerialName("latitude")
val latitude: Double
) {
fun toUniversitySelectionEntity() = UniversitySelectionEntity(
id = id,
name = name,
longitude = longitude,
latitude = latitude,
)
}
@Serializable
data class UniversitySelectionResponseDto(
@SerialName("universities")
val universities: List<University>
)
@Serializable
data class UniversitySelectionResponseDto(
@SerialName("universities")
val universities: List<University>
)
@Serializable
data class University(
@SerialName("id")
val id: Int,
@SerialName("name")
val name: String,
@SerialName("longitude")
val longitude: Double,
@SerialName("latitude")
val latitude: Double
) {
fun toUniversitySelectionEntity() = UniversitySelectionEntity(
id = id,
name = name,
longitude = longitude,
latitude = latitude,
)
}

universities: PersistentList<UniversitySelectionModel>,
selectedUniversity: String?,
onSelectUniversity: (String) -> Unit,
universities: List<UniversitySelectionEntity>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

List -> PersistantList

isSelected = selectedUniversity == university.name,
onSelectUniversity = onSelectUniversity
isSelected = selectedUniversity == university,
onSelectUniversity = { onSelectUniversity(university) }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

리플렉션 써주세요

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

박동민 바보 ㅋㅋ 이거 안대

val universities: List<UniversitySelectionEntity> = emptyList(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

State에서도 List -> PersistantList 해주세요

).sortedBy { it.name }
_universitySelectionState.value =
_universitySelectionState.value.copy(universities = dummyData)
val result = universitySelectionRepository.getUniversitySelection()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

불필요한 변수 선언하지 말아주세요

Comment on lines 44 to 67
val selectedUniversity = _universitySelectionState.value.selectedUniversity
if (selectedUniversity != null) {
val request = UniversitySelectionRequest(
universityId = selectedUniversity.id.toLong(),
name = selectedUniversity.name,
longitude = selectedUniversity.longitude,
latitude = selectedUniversity.latitude
)
viewModelScope.launch {
val result = universitySelectionRepository.postUniversitySelection(request)
result.onSuccess { university ->
val updatedUniversities =
_universitySelectionState.value.universities + university

_universitySelectionState.value = _universitySelectionState.value.copy(
universities = updatedUniversities
)
}.onFailure {
// 에러 처리
}
}
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기도 불필요하게 변수 선언 하지 말아주세요

Copy link
Contributor

@hyeeum hyeeum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

동멘님이 필요한 부분 다 달아주셔서 달만한 내용이 없네요
저도 리뷰확인하면서 많이 알아갑니다:0

@0se0 0se0 requested a review from chattymin July 17, 2024 13:39
Copy link
Member

@chattymin chattymin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다~~
역시 채고!!!

@0se0 0se0 merged commit 0f20e90 into develop Jul 17, 2024
@0se0 0se0 deleted the feature/#68-university-selection-api branch July 17, 2024 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FEATURE ✨ 새로운 기능 구현 세영 🦌
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] University selection api
3 participants