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.

Sign up for GitHub

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

Jump to bottom

Feat : 식단 api 수정 #158

Merged
merged 8 commits into from
Mar 20, 2024
Merged

Feat : 식단 api 수정 #158

merged 8 commits into from
Mar 20, 2024

Conversation

huiwoo-jo
Copy link
Contributor

📮 관련 이슈

✍️ 구현 내용

  • api 수정에 따라 식단 화면의 코드를 수정하였습니다.

📷 구현 영상

  • image

✔️ 확인 사항

  • 컨벤션에 맞는 PR 타이틀
  • 관련 이슈 연결
  • PR 관련 정보 연결 (작업자, 라벨, 마일스톤 등)
  • Github Action 통과

@huiwoo-jo huiwoo-jo added 🤗 FEATURE Develop this project 🩵희우 labels Feb 19, 2024
@huiwoo-jo huiwoo-jo added this to the 2차 스프린트 milestone Feb 19, 2024
@huiwoo-jo huiwoo-jo self-assigned this Feb 19, 2024
huiwoo-jo
huiwoo-jo commented Feb 19, 2024
View reviewed changes
Copy link
Contributor Author

@huiwoo-jo huiwoo-jo left a comment

Choose a reason for hiding this comment

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

참고하실 내용들을 조금 적었습니당

app/src/main/java/com/dongyang/android/youdongknowme/standard/network/RetrofitObject.kt Outdated

val baseInterceptor = Interceptor { chain ->
val request = chain.request().newBuilder()
val originalHttpUrl = chain.request().url

val url = originalHttpUrl.newBuilder().addQueryParameter("api_key", BuildConfig.API_KEY).build()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

api key를 별도로 사용하지 않기에 우선 삭제하였습니다.

Copy link
Member

Choose a reason for hiding this comment

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

그럼 이 상황에서 Interceptor 는 어떤 역할을 하나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

코멘트 확인이 늦었습니다
api_key를 지우면 따로 값을 조작할 필요가 없으니 Interceptor의 의미가 없어지는군요..
api_key를 제거하는 것만 생각해서 역할을 생각하지 못했습니다
해당 코드는 다시 복구했습니다

app/src/main/java/com/dongyang/android/youdongknowme/ui/view/cafeteria/CafeteriaFragment.kt Outdated Show resolved Hide resolved
hoyahozz
hoyahozz requested changes Feb 20, 2024
View reviewed changes
Copy link
Member

@hoyahozz hoyahozz left a comment

Choose a reason for hiding this comment

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

예전에 제가 작성한 코드를 보니 어지럽군요..

코멘트 확인 부탁드립니다 😄

app/src/main/java/com/dongyang/android/youdongknowme/ui/view/cafeteria/CafeteriaViewModel.kt Outdated Show resolved Hide resolved
app/src/main/java/com/dongyang/android/youdongknowme/data/remote/entity/Cafeteria.kt Outdated Show resolved Hide resolved
app/src/main/java/com/dongyang/android/youdongknowme/standard/network/RetrofitObject.kt Outdated
@@ -13,13 +13,13 @@ object RetrofitObject {

fun getNetwork(): Retrofit {

val baseUrl = "http://3.38.118.184:8000"
val baseUrl = "https://triphub.kr/"
Copy link
Member

Choose a reason for hiding this comment

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

baseUrl 은 일반적으로 숨김 처리합니다~

Copy link
Collaborator

Choose a reason for hiding this comment

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

이 부분 공유해드릴게용

Copy link
Contributor Author

Choose a reason for hiding this comment

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

API_KEY와 같이 buildConfig에 선언하였는데 맞는 방식인지 잘 모르겠네요😅 확인 부탁드려요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

⭐ github CI 문제로 인하여 추후 변경 예정

app/src/main/java/com/dongyang/android/youdongknowme/standard/network/RetrofitObject.kt Outdated

val baseInterceptor = Interceptor { chain ->
val request = chain.request().newBuilder()
val originalHttpUrl = chain.request().url

val url = originalHttpUrl.newBuilder().addQueryParameter("api_key", BuildConfig.API_KEY).build()
Copy link
Member

Choose a reason for hiding this comment

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

그럼 이 상황에서 Interceptor 는 어떤 역할을 하나요?

app/src/main/java/com/dongyang/android/youdongknowme/ui/view/cafeteria/CafeteriaViewModel.kt Outdated Show resolved Hide resolved
app/src/main/java/com/dongyang/android/youdongknowme/ui/view/cafeteria/CafeteriaViewModel.kt Outdated Show resolved Hide resolved
app/src/main/java/com/dongyang/android/youdongknowme/ui/view/cafeteria/CafeteriaFragment.kt Outdated Show resolved Hide resolved
@huiwoo-jo huiwoo-jo changed the title Feature : 식단 api 수정 Feat : 식단 api 수정 Feb 20, 2024
m6z1
m6z1 approved these changes Feb 21, 2024
View reviewed changes
Copy link
Collaborator

@m6z1 m6z1 left a comment

Choose a reason for hiding this comment

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

good

@huiwoo-jo huiwoo-jo requested a review from hoyahozz March 11, 2024 16:00
m6z1
m6z1 approved these changes Mar 16, 2024
View reviewed changes
app/src/main/java/com/dongyang/android/youdongknowme/ui/view/cafeteria/CafeteriaViewModel.kt
@@ -36,6 +36,8 @@ class CafeteriaViewModel(
private val _menus: MutableLiveData<List<String>> = MutableLiveData()
val menus: LiveData<List<String>> = _menus

private val emptyMenu = listOf(resourceProvider.getString(R.string.cafeteria_no_menu))
Copy link
Collaborator

Choose a reason for hiding this comment

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

resourceProvide 는 어떤 것인가용

Copy link
Contributor Author

Choose a reason for hiding this comment

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

아래와 같이 정호님이 추천해주신 코드에 포함되어있는 내용입니다!
#158 (comment)

ResourceProvider 는 중복 코드를 제외 및 유지보수성을 위해서 사용한다고합니다!
resource의 로딩을 한 뒤 재사용이 가능하며 context에 의지하지 않아도 된다고 합니다

저도 이번에 새롭게 배웠습니다 👍

Copy link
Member

Choose a reason for hiding this comment

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

ViewModel 에서 context 에 대한 의존성을 지니는 것을 지양해야 하는 이유에 대해서도 공부해보시면 좋을 것 같아요~

app/src/main/java/com/dongyang/android/youdongknowme/ui/view/cafeteria/CafeteriaViewModel.kt
}

return menus
_menus.postValue(cafeteriaList.find { it.date == selectedDate }?.menus ?: emptyMenu)
Copy link
Collaborator

Choose a reason for hiding this comment

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

오호 ~~ 코틀린 활용을 잘 하시는 군요

Copy link
Contributor Author

@huiwoo-jo huiwoo-jo Mar 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

정호님이 제안해주신 코드입니다 😎

#158 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

개인적 취향이지만 줄바꿈을 해야 코드가 더 눈에 잘 들어오더라구요!

@huiwoo-jo huiwoo-jo merged commit e162536 into develop Mar 20, 2024
1 check passed
@huiwoo-jo huiwoo-jo deleted the feature/api-cafeteria branch March 20, 2024 23:48
@huiwoo-jo huiwoo-jo mentioned this pull request Mar 23, 2024
Closed
7 tasks
@huiwoo-jo huiwoo-jo mentioned this pull request Mar 31, 2024
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hoyahozz hoyahozz hoyahozz approved these changes

@m6z1 m6z1 m6z1 approved these changes

Assignees

@huiwoo-jo huiwoo-jo

Labels
🤗 FEATURE Develop this project 🩵희우
Projects
None yet
Milestone
2차 스프린트
Development

Successfully merging this pull request may close these issues.

식단 api 수정
3 participants
@huiwoo-jo @hoyahozz @m6z1