Skip to content

[Hotfix] #498 - 신규 홈뷰 및 솝트로그: 두 번째 QA 내용 반영 #499

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

Merged
merged 18 commits into from
Feb 20, 2025

Conversation

dlwogus0128
Copy link
Contributor

@dlwogus0128 dlwogus0128 commented Feb 19, 2025

🌴 PR 요약

신규 홈뷰 및 솝트로그: 두 번째 QA 내용을 반영했습니다.

🌱 작업한 브랜치

🌱 PR Point

  • 금일 QA에서 발견됐던 내용들 중에서 할 수 있는 것들은 최대한 해결해뒀습니다~! (이슈에 체크리스트 만들었어용)
  • QA 내용 외에도 (아무도 발견하지 못했던..) 저의 자잘한 실수들 혹은 잘못된 분기, 레이아웃 등등 다양하게 수정해뒀습니다.
  • 근데 캘린더 디테일뷰에서 가장 가까운 날짜의 이벤트 확인 -> 이 부분은 윤서 님이 봐주시는 게 더 정확할 것 같아서 따로 수정 안했습니다!

📌 참고 사항

  • 지금 걱정되는 게 있는데요... mainFlow를 homeFlow로 변경하면서 딥링크 로직들이 조금씩 꼬이는 것 같은데 (ㅜ)
    제가 딥링크 로직을 짠 게 아니라서 좀 파악할 시간이 필요할 것 같습니다...
  • 딥링크 관련 코드들을 직접 건드려 놓은 부분들이 있는데
  1. HomeDeepLink 호출 시 runMainFlow 대신 runHomeFlow로 변경
  2. bindNotification() -> childCoordinators에 MainCoordinator 포함되었는지 확인하는 부분 HomeCoordinator로 변경
  3. handleDeepLink에서 self.router.dismissModule(animated: false) 주석 처리

2번 로직은 (확실치 않지만) 가장 root가 되는 메인 플로우가 진행되고 있는지를 확인하는 것 같은데, 저희 메인 플로우가 홈으로 변경되어서 해당 부분을 수정했거든요. 그랬더니 딥링크로 호출되는 화면들이 present 되자마자 dismiss 되는 현상이 나타나, 3번으로 대응했더니 해결됐습니다.

분명 딥링크 핸들링하기 전에 root controller를 dismiss 해야 하는 이유가 있었을텐데... 이유를 아직 잘 모르지만 일단 미심쩍은 상태로 해결했습니다.. -> 해결..했습니다 ..

딥링크 부분 읽다보니까 저희 탭바 생기면.. 딥링크에 관련해서 한 번 논의해봐야 될 것 같네요 ..

  • 네트워크 모달 등 홈뷰 에러 처리도 필요할 것 같은데, QA 이슈에 함께 달만한 내용은 아닌 것 같아서 따로 이슈파서 작업하겠습니다.
  • 그리고 레거시 뷰들 중에 메모리 릭 나는 부분들이 꽤 있네요... 일단은 흐린눈 하겠습니다
  • 탭바도 작업하신 부분까지 올려주시면 고쳐보겠습니다~ @yungu0010

📸 스크린샷

생략

📮 관련 이슈

오늘의 솝마디 -> 홈으로 돌아가기 동작 안되는 문제 (Home 딥링크 -> HomeFlow로 변경)
홈 -> 캘린더 진입 후 pop 시 버튼 터치 안되는 문제 (finishFlow 삭제)
(비회원) 홈 -> 대시보드 setData 설정
알림 모두 읽음 버튼 터치 시 홈화면에 반영
솝트로그 데이터 잘 보여지는지 확인
알림 디테일뷰 이후로 넘어가지 않는 문제 확인
@dlwogus0128 dlwogus0128 added Fix 문제 해결, 코드 수정 재현✦ labels Feb 19, 2025
@dlwogus0128 dlwogus0128 self-assigned this Feb 19, 2025
Copy link

height bot commented Feb 19, 2025

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

Copy link
Contributor

@yungu0010 yungu0010 left a comment

Choose a reason for hiding this comment

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

QA반영해주셔서 감사합니다🥹
캘린더 스크롤 부분은 메신저로 따로 전달드릴게요! 같은 PR에 반영해주실 수 있을까요?

참고사항에 적어주신 내용을 테스트해보고자 2번까지 실행 후 확인해보았는데,, 제 환경에서는 정상동작하여 한 번 더 테스트 해봐주심 감사하겠습니다 !!

적어주신 딥링크 부분에 대한 제 추론을 말씀드리자면,,
사용자가 앱 내 다른 모듈을 실행 중일 때 딥링크 알림을 받게 되면 기존에 띄워둔 모듈을 모두 삭제하고
deepLink.execute()부분에서 다시 스택에 쌓는것이 아닐가 ,, 추측해봅니다.. .

+) handleDeepLink에서 dismissModule로 인한 문제가 있다면 handleWebLink 부분도 확인해보아야할 것 같아요!

빠른 QA 대응 너무너무 감사합니다 🫶🫶🫶 탭바도 바로 올릴게요 !

@@ -101,7 +102,7 @@ extension DashBoardCardCVC {
self.descriptionLabel.setLineSpacing(lineSpacing: 5)
self.rightArrowWithCircleImageView.isHidden = false
guard let history = model.history else { return }
userHistoryView.setData(userType: userType, recentHistory: 35, allHistory: history)
userHistoryView.setData(userType: userType, recentHistory: history.first, allHistory: history)
Copy link
Contributor

Choose a reason for hiding this comment

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

allHistory에 인덱스 0번 제외해주지 않아도 되나요 . ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

이거 내부 코드에서 첫번째 값을 지워주고 있습니다~!

private func makeHistoryView(allHistory: [Int]?) {
        // 활동 기수들의 내역을 나열합니다.
        guard var allHistory = allHistory, !allHistory.isEmpty else { return }
        allHistory.removeFirst()

Comment on lines 128 to 145
input.requestUserInfo
.dropFirst()
.handleEvents(receiveOutput: { _ in
output.isLoading.send(true)
})
.flatMap { _ in
self.useCase.getUserInfo()
}
.compactMap { $0 }
.flatMap { userInfo in
self.useCase.getHomeDescription().map { $0.toPresentation(history: userInfo.historyList, isAllConfirm: userInfo.isAllConfirm) }
}
.withUnretained(self)
.sink { owner, data in
output.dashBoard.send(data)
output.isLoading.send(false)
}
.store(in: cancelBag)
Copy link
Contributor

Choose a reason for hiding this comment

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

이 부분 추가하신 이유가 궁금합니다. 기존 로직에서도 getHomeDscription()을 호출하고 있어서요! dashBoard 부분만 따로 업데이트해야하는 경우가 있나요? (연관되는 HomeVC 부분 코드도 궁금해요~!)

Copy link
Contributor Author

@dlwogus0128 dlwogus0128 Feb 20, 2025

Choose a reason for hiding this comment

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

화면을 나갔다가 들어왔을 때 바뀔 가능성이 있는 부분(userInfo)만 새로 api를 호출하기 위해 이렇게 구현했어요! 셀을 초기에 append하는 viewDidLoad 이후부터 dashBoard 값을 다시 받아,reconfigureItems로 데이터만 바꾸어주기 위해 이렇게 .dropFirst()로 첫번째 요청 이후부터 사용하고 있는데요.

input.requestUserInfo
         .dropFirst()

근데 어제 새벽에 테스트하면서 보니까, 앱 서비스도 화면이 바뀐 이후에 뱃지 값이 변경이 안돼서 그냥 뷰 appear될 때마다 모든 api 재호출 해도 될 것 같아요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

수정해서 반영했습니다~

@dlwogus0128
Copy link
Contributor Author

캘린더 스크롤 부분은 메신저로 따로 전달드릴게요! 같은 PR에 반영해주실 수 있을까요?

네 확인했습니다~! 반영해둘게요

적어주신 딥링크 부분에 대한 제 추론을 말씀드리자면,,
사용자가 앱 내 다른 모듈을 실행 중일 때 딥링크 알림을 받게 되면 기존에 띄워둔 모듈을 모두 삭제하고
deepLink.execute()부분에서 다시 스택에 쌓는것이 아닐가 ,, 추측해봅니다.. .

아 맞는 것 같아요...!

참고사항에 적어주신 내용을 테스트해보고자 2번까지 실행 후 확인해보았는데,, 제 환경에서는 정상동작하여 한 번 더 테스트 해봐주심 감사하겠습니다 !!

왜지.. 왜지 새벽의 저주..인가... 세수하고 다시 한 번 확인해볼게요...

Comment on lines 82 to 90
override func viewDidLayoutSubviews() {
super.viewDidLayoutSubviews()

if let gradientLayer = gradientView.layer.sublayers?.first as? CAGradientLayer {
gradientLayer.frame = gradientView.bounds
}

scrollToRecentSchedule()
}
Copy link
Contributor

@yungu0010 yungu0010 Feb 20, 2025

Choose a reason for hiding this comment

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

헙 급하게 지우느라 잘못 수정했습니다 .. scrollToRecentSchedule()만 지우고 기존 코드 다시 복구해주세요!! 죄송합니다😥

@dlwogus0128
Copy link
Contributor Author

dlwogus0128 commented Feb 20, 2025

  1. HomeDeepLink 호출 시 runMainFlow 대신 runHomeFlow로 변경
  2. bindNotification() -> childCoordinators에 MainCoordinator 포함되었는지 확인하는 부분 HomeCoordinator로 변경

@yungu0010 님은 같은 코드에서 실행이 잘 돼서... ㅜㅜ 진짜 혹시 몰라서 제 demo 앱을 삭제했다가 다시 깔았는데 해결이 됐네요
이거 원인이 뭐죠? (....?)

@dlwogus0128 dlwogus0128 merged commit a4a0a1e into develop Feb 20, 2025
@dlwogus0128 dlwogus0128 deleted the hotfix/#498-second-qa branch February 20, 2025 05:34
@meltsplit meltsplit restored the hotfix/#498-second-qa branch February 20, 2025 12:13
@yungu0010 yungu0010 deleted the hotfix/#498-second-qa branch March 25, 2025 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fix 문제 해결, 코드 수정 size/L 재현✦
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Hotfix] 신규 홈뷰, 솝트로그 - 2차 QA
2 participants