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] 오픈채팅방 유효성검사 #204

Merged
merged 8 commits into from
Mar 7, 2024
Merged

Conversation

DoReMinWoo
Copy link
Member

@DoReMinWoo DoReMinWoo commented Feb 26, 2024

Related issue 🛠

Work Description ✏️

  • 오픈 채팅방 유효성 검사
  • 기존단체입장 초대코드 화면 android:windowSoftInputMode="adjustPan" 속성 적용

Screenshot 📸

Recording_2024-03-06-22-50-04.mp4

복사 붙어넣기한 텍스트는
"카카오톡 오픈채팅을 시작해보세요.
어쩌구저쩌구

어쩌구저쩌구방
https://open.kakao.com/o/gkRbyM4f"
로 붙어 넣은거에용

Uncompleted Tasks 😅

N/A

To Reviewers 📢

기존 단체 입장 초대코드 화면이 작은 기기에서 어떻게 나오는지 확인을 해야하는데 작은 기기로 애뮬을 만들었는데 소셜로그인에서 자꾸 튕겨서 제대로 된 테스트가 안되네요...ㅠㅠ 일단 adjustPan으로 설정해뒀습니다.

@DoReMinWoo DoReMinWoo self-assigned this Feb 26, 2024
@DoReMinWoo DoReMinWoo requested a review from a team as a code owner February 26, 2024 12:25
@DoReMinWoo DoReMinWoo linked an issue Feb 26, 2024 that may be closed by this pull request
2 tasks
Copy link
Collaborator

@HAJIEUN02 HAJIEUN02 left a comment

Choose a reason for hiding this comment

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

귯귯핑

PingleSnackbar.makeSnackbar(
binding.root,
stringOf(R.string.plan_open_chatting_snackbar),
126
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
Collaborator

Choose a reason for hiding this comment

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

스낵바 로직들이 어떤 것에 대한 스낵바인지 바로바로 알아보기가 어려워서 이거 함수화하거나 공통적으로 알아볼 수 있게 고치면 좋을 거 같아용 나중에 !

@@ -86,6 +88,18 @@ class PlanActivity : BindingActivity<ActivityPlanBinding>(R.layout.activity_plan
planViewModel.postPlanMeeting()
}

PLAN_OPEN_CHATTING_FRAGMENT_INDEX -> {
if (planViewModel.validityOpenChattingLink()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기 if~else문은 checkValidation 등으로 함수화해주면 코드가 더 깔끔해질 것 같아욤

companion object {
const val FIRST_PAGE_POSITION = 0
const val DEFAULT_OLD_POSITION = -1
const val DEFAULT_RECRUITMENT = "1"
const val START_RECRUITMENT = 2
const val END_RECRUITMENT = 99
const val BLANK_STRING = " "
const val OPEN_CHATTING_LINK_VALIDITY = "https://open.kakao.com/"
Copy link
Collaborator

Choose a reason for hiding this comment

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

근데 https:// 없이 open.kakao.com/으로만 시작해도 연결 잘 되지 않나염?,, 이 부분 먼가 고민이 필요할 것 같아요

Copy link
Collaborator

Choose a reason for hiding this comment

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

테스트 결과..
https://open.kakao.com/o/o
https:open.kakao.com/o/o
open.kakao.com/o/o
/open.kakao.com/o/o
//open.kakao.com/o/o
전부 다 잘 들어가져요.. 쩝

Copy link
Collaborator

Choose a reason for hiding this comment

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

그러면 링크에 open.kakao.com이 포함되는지 여부로 유효성 검사 진행해줘도 될 것 같고요,, 어찌 됐든 모든 경우의 수를 다 막을 순 없을 것 같아서,, 아요 측에서 이 부분 맡으신 분이랑 어떤 식으로 진행할 건지 논의가 필요할 것 같네요

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
Member Author

Choose a reason for hiding this comment

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

open.kakao.com가 포함되는 방향으로 논의했고 ios 측 답변오면 맞춰서 작업할게요!~

Copy link
Collaborator

@jihyunniiii jihyunniiii left a comment

Choose a reason for hiding this comment

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

요거 하나만 바꿔주고 머지 하ㅅㅔ요 ! 고생했어용

Comment on lines 36 to 38
planViewModel.planOpenChattingLink.flowWithLifecycle(lifecycle).onEach { openChattingLink ->
planViewModel.validityOpenChattingLink()
}.launchIn(lifecycleScope)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
planViewModel.planOpenChattingLink.flowWithLifecycle(lifecycle).onEach { openChattingLink ->
planViewModel.validityOpenChattingLink()
}.launchIn(lifecycleScope)
planViewModel.planOpenChattingLink.flowWithLifecycle(viewLifecycleOwner.lifecycle).onEach { openChattingLink ->
planViewModel.validityOpenChattingLink()
}.launchIn(viewLifecycleOwner.lifecycleScope)

프래그먼트에서 라이프 사이클을 주입할 때 한 번 더 생각합니둥 ㅋ.ㅋ 갠적으로 이거 참 중요한 개념이라 생각합니당

@DoReMinWoo DoReMinWoo merged commit 003633f into develop Mar 7, 2024
1 check passed
@DoReMinWoo DoReMinWoo deleted the mod-open-link-validity branch March 7, 2024 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[mod] 오픈채팅방 유효성검사
4 participants