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

QA/126-signup 회원가입시 약관~ 스낵바 출현하며 가입이 불가한 현상 #169

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

yangsooplus
Copy link
Member

🌱 Key changes

  • 만 14세 이상을 확인하는 약관의 더미데이터가 서버 유효성 검사에 걸린 모양입니다
  • 안전하게 가입하기 직전 더미데이터에 해당하는 0번을 제거하도록 수정

✅ To Reviewers

📸 스크린샷

스크린샷
파일첨부바람

@yangsooplus yangsooplus added the QA label May 27, 2024
@yangsooplus yangsooplus requested a review from jinukeu May 27, 2024 12:47
@yangsooplus yangsooplus self-assigned this May 27, 2024
@@ -96,7 +96,7 @@ class SignUpViewModel @Inject constructor(
} else {
null
},
termAgreement = uiState.value.agreedTerms,
termAgreement = uiState.value.agreedTerms.filter { it > 0 },
Copy link
Member

Choose a reason for hiding this comment

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

p2:

it > 0 이 명확하게 어떤 내용인지 감이 잘 안오네요!
추가로 나중에 약관 동의 항목이 더 생기거나 순서가 바뀌면 예상치 못한 동작을 일으킬 것 같습니다!

it 대신 enum class를 사용하고 it > 0 대신 it != (만14세 이상 enum class) 이렇게 바꾸는거 어떠세요!?

Copy link
Member Author

Choose a reason for hiding this comment

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

다른 약관은 서버에서 내려주고 있고, 14세 약관만 로컬에서 사용하는 약관이에요
약관 번호로 체크 여부를 판단하느랴 정수형 번호로 부여되어 있어요
회원가입 시 [1, 2]의 array 형태로 동의한 약관의 번호를 보내줍니다

그래서 서버 응답을 굳이굳이 Enum class로 바꾸어 사용하는 방법은 힘들 것 같고..
100번대부터는 로컬에서 사용하는 약관 번호로 사용하는 방법은 어떨까요?

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

Choose a reason for hiding this comment

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

기록) 서버에서 처리하기로 함

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants