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

[MOD] 보관함 - 내가 그린 코스 / 사용자가 설정한 제목이 표시 되도록 수정 #310

Merged
merged 11 commits into from
Feb 7, 2024

Conversation

dongx0915
Copy link
Member

@dongx0915 dongx0915 commented Jan 7, 2024

📌 개요

✨ 작업 내용

  • 보관함 - 내가 그린 코스 화면에서 주소가 아닌 사용자가 설정한 제목이 표시 되도록 수정
  • 서버 Api 변경 사항 반영 (동작에 오류가 있어 같이 수정 하였습니다.)
    • 코스 업로드 응답 형식 수정
    • 내가 그린 코스 리스트 응답 형식 수정
    • 코스 상세 조회 응답 형식 수정
  • 코스 업로드 요청 코드 null 처리 (DrawViewModel.kt)

✨ PR 포인트

  • Mapper 메소드 위치, null 처리 기본 값 (댓글 참고)
  • 컨벤션이나 누락된 작업 없는 지 같이 확인 부탁드립니다~!

📸 스크린샷/동영상

KakaoTalk_Video_2024-01-07-18-22-11.mp4

@dongx0915 dongx0915 added MOD ☁ 코드 및 내부 파일 수정 동현 🐨 동현 담당 labels Jan 7, 2024
@dongx0915 dongx0915 requested review from leeeha, sxunea and unam98 January 7, 2024 09:23
@dongx0915 dongx0915 self-assigned this Jan 7, 2024
Comment on lines 102 to 104
distance = distanceSum.value ?: 0.0f,
departureAddress = departureAddress.value ?: "", //커스텀의 경우 지금 여기에 들어가는 게 아무것도 없음.
departureName = departureName.value ?: ""
Copy link
Member Author

Choose a reason for hiding this comment

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

null 처리 기본 값 이대로 지정해도 괜찮을 지 확인 부탁드립니다.
기존엔 하나라도 null이면 runCatching에 잡혀서 onFailure로 빠지고 있더라구요

일단은 임시로 null 처리 해두었습니다.

Copy link
Contributor

Choose a reason for hiding this comment

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

기본값을 세팅한다는 건 앱을 죽이지 않고 살리겠다는 것인데 포인트는 DrawActivity의 data가 CountDown -> Run 액티비티로 넘어간다는 것입니다. 앱은 안 죽었지만 사용자가 의도했던 것과 다른 코스/출발지 등등으로 세팅되어 RunActivity가 실행되는 것은 의미가 없다고 판단하여

현재 아래와 같이 값에 이상이 있을 경우 토스트 메세지를 띄우고 페이지를 안 넘기게 막아놨습니다.
이와 관련된 논의 스레드입니다.
https://w1670909332-6ay129099.slack.com/archives/C04EXNB86LB/p1703075472448109

                if (courseData.courseId == null || courseData.departure == null || courseData.distance == null) {
                    Toast.makeText(
                        this@DrawActivity,
                        ERROR_COURSE_NULL,
                        Toast.LENGTH_LONG
                    ).show()
                }

null 처리 기본값 지정 시 위의 toast 메세지 로직이 안 먹히게 되어 이 부분은 기본값 없이 두는 게 좋을 것 같습니다.

혹시 더 좋은 의견이 있다면 궁금합니다 :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

앱은 안 죽었지만 사용자가 의도했던 것과 다른 코스/출발지 등등으로 세팅되어 RunActivity가 실행되는 것은 의미가 없다고 판단하여

그러면 RunActivity와 같이 올바른 기능 수행이 어려울 때는 페이지를 넘기지 않고, GET과 같은 데이터를 가져와서 ui에 붙이는 통신들(ex. 마라톤 코스 GET)은 기본값을 지정해주는 것으로 이해하면 될까요 ? 앱이 터지지 않는 것도, 의미 있는 사용도 챙기다보니 기준이 좀 모호한 것 같아서요 다른 분들도 생각이 궁금합니다 !

Copy link
Contributor

Choose a reason for hiding this comment

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

@sxunea

기준이라는 개념보다는 그때그때 상황 보고 기획 단과 논의하여 결정하면 될 것 같습니다! 경우의 수가 다양하니까요

Copy link
Member Author

Choose a reason for hiding this comment

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

@unam98 해당 부분 Revert 해두었습니다~!

@dongx0915 dongx0915 changed the title [Mod] 보관함 - 내가 그린 코스 / 사용자가 설정한 제목이 표시 되도록 수정 [MOD] 보관함 - 내가 그린 코스 / 사용자가 설정한 제목이 표시 되도록 수정 Jan 7, 2024
Copy link
Contributor

@unam98 unam98 left a comment

Choose a reason for hiding this comment

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

수고 하셨습니다~!
null 처리 관련돼서 논의가 필요한 부분이 있어 확인 부탁드립니다!

Comment on lines 102 to 104
distance = distanceSum.value ?: 0.0f,
departureAddress = departureAddress.value ?: "", //커스텀의 경우 지금 여기에 들어가는 게 아무것도 없음.
departureName = departureName.value ?: ""
Copy link
Contributor

Choose a reason for hiding this comment

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

기본값을 세팅한다는 건 앱을 죽이지 않고 살리겠다는 것인데 포인트는 DrawActivity의 data가 CountDown -> Run 액티비티로 넘어간다는 것입니다. 앱은 안 죽었지만 사용자가 의도했던 것과 다른 코스/출발지 등등으로 세팅되어 RunActivity가 실행되는 것은 의미가 없다고 판단하여

현재 아래와 같이 값에 이상이 있을 경우 토스트 메세지를 띄우고 페이지를 안 넘기게 막아놨습니다.
이와 관련된 논의 스레드입니다.
https://w1670909332-6ay129099.slack.com/archives/C04EXNB86LB/p1703075472448109

                if (courseData.courseId == null || courseData.departure == null || courseData.distance == null) {
                    Toast.makeText(
                        this@DrawActivity,
                        ERROR_COURSE_NULL,
                        Toast.LENGTH_LONG
                    ).show()
                }

null 처리 기본값 지정 시 위의 toast 메세지 로직이 안 먹히게 되어 이 부분은 기본값 없이 두는 게 좋을 것 같습니다.

혹시 더 좋은 의견이 있다면 궁금합니다 :)

Copy link
Collaborator

@sxunea sxunea left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 : )

)
}
return changedData
return remoteStorageDataSource.getMyDrawCourse().body()?.let {
Copy link
Collaborator

@sxunea sxunea Jan 9, 2024

Choose a reason for hiding this comment

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

기존 코드이긴 하지만 BaseResponse가 생겼으니 Response 바꾸는 김에 적용하면 더욱 간결할 것 같아요 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

@sxunea 감사합니다. 수정하였습니다~!

Comment on lines 102 to 104
distance = distanceSum.value ?: 0.0f,
departureAddress = departureAddress.value ?: "", //커스텀의 경우 지금 여기에 들어가는 게 아무것도 없음.
departureName = departureName.value ?: ""
Copy link
Collaborator

Choose a reason for hiding this comment

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

앱은 안 죽었지만 사용자가 의도했던 것과 다른 코스/출발지 등등으로 세팅되어 RunActivity가 실행되는 것은 의미가 없다고 판단하여

그러면 RunActivity와 같이 올바른 기능 수행이 어려울 때는 페이지를 넘기지 않고, GET과 같은 데이터를 가져와서 ui에 붙이는 통신들(ex. 마라톤 코스 GET)은 기본값을 지정해주는 것으로 이해하면 될까요 ? 앱이 터지지 않는 것도, 의미 있는 사용도 챙기다보니 기준이 좀 모호한 것 같아서요 다른 분들도 생각이 궁금합니다 !

Copy link
Member

@leeeha leeeha left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!

Copy link
Contributor

@unam98 unam98 left a comment

Choose a reason for hiding this comment

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

수고하셨습니당

@dongx0915 dongx0915 merged commit 33126ee into develop Feb 7, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MOD ☁ 코드 및 내부 파일 수정 동현 🐨 동현 담당
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEAT] 보관함-내가그린코스 / 지역명 -> 코스 이름
4 participants