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

feat: 스플레시 추가 및 유저 생성 api 연결 #42

Merged
merged 8 commits into from
Jul 15, 2024

Conversation

hryeong66
Copy link
Collaborator

What is this PR? 🔍

이슈

설명

  • 우선 splash 화면이 appear 되면 create User 호출하게 해놓았습니답
  • 데이터를 받아온 후 3초 후에 mainTab이 뜨도록 임시로 수정해놓았습니다.

Changes 📝

  • UserManager를 추가해서 deviceId 만들때 사용한 userUUID랑 memeLevel 넣어놓았습니다
    memeLevel은 종난오빠가 써주겠져?

Screenshot 📸

image

To Reviewers 🙏

  • splash 뷰에 bindViewModel()로 넣어놓은게 호출이 안되는데 왤까여....
    혹시 아시는 분 있으면 얘기해주시면 따봉드립니다
  • 앟 그리고 splash 안에 action을 어떻게 넣으면 좋을지 고민이라...
    우선은 start finish로 넣었는데 좋은 네이밍이나 방식 있으면 얘기해주세여 지금은 빨리 PR 올리가 갈겁니다!!!!🤸‍♂️

@hryeong66 hryeong66 added the 🧘🏽‍♂️ Feature 기능 개발 label Jul 11, 2024
@hryeong66 hryeong66 self-assigned this Jul 11, 2024
Copy link
Member

@chansooo chansooo 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 23 to 25
UserManager.setupUserUUID()
let router = SplashRouter(navigationController)
self.navigationController.setNavigationBarHidden(true, animated: false)
Copy link
Member

Choose a reason for hiding this comment

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

스플래시 뷰가 생겨서 SplashViewModel에서 첫 사용자인지 확인을 하면 더 좋겠네요~

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

넵 요부분 useCase 변경하고 SplashViewModel에서 checkUserInfo 호출하도록 수정했습니다~!
082efa7

public func checkUserInfo() async throws -> UserDetail {
    if UserInfo.shared.deviceId.isEmpty {
      return try await createUser()
    } else {
      return try await getUserDetail()
    }
  }

import PPACModels

public protocol CreateUserUseCase {
var userRepository: UserRepository { get set }
Copy link
Member

Choose a reason for hiding this comment

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

해당 protocol에서 repository을 알 필요가 있을까요??? get set으로 하신 이유가 있을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

으음... 그렇네요 protocol에서 삭제했습니답! 082efa7

Comment on lines 42 to 55
var urlRequest = URLRequest(url: url).append(body: parameter)
urlRequest.httpMethod = httpMethod.rawValue.uppercased()

var defaultHeaders = [
"x-device-id": UserManager.uuid,
"accept": "application/json",
"Content-Type": "application/json"
]

if let additionalHeaders = headers {
for (key, value) in additionalHeaders {
defaultHeaders[key] = value
}

urlRequest.allHTTPHeaderFields = defaultHeaders
return urlRequest
}
Copy link
Member

Choose a reason for hiding this comment

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

헤더에 직접적으로 UserManager를 통해 uuid를 삽입하는 것보다는 interceptor 구현해주고, networkmanager에 삽입시켜주는 형식이 더 좋을 듯 합니다


// MARK: - Properties
@ObservedObject private var viewModel: SplashViewModel
@State private var cancleable = Set<AnyCancellable>()
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
Collaborator Author

Choose a reason for hiding this comment

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

넵 요거 필요없어서 삭제했습니다!
확인감사합니당~ 082efa7

}

private func bindViewModel() {
viewModel.$state
Copy link
Member

Choose a reason for hiding this comment

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

state가 이벤트 방출하는 시점이 함수가 불리는 시점보다 앞인 것 같습니다. 그리고 "뷰를 꺼라"는 명령은 viewmodel에 있는 것이 더 어울릴 것 같네요.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

넵! 요것 state에 isVisible 필드 넣어서 사용하는 걸로 수정했습니다!
082efa7

Comment on lines 16 to 17
func popView()
func showMainTabView()
Copy link
Member

Choose a reason for hiding this comment

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

popView가 showMainTabView와 분리되어 있는 이유가 있을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

앟 그렇네요 요거 popView 삭제했습니다082efa7

@hryeong66 hryeong66 force-pushed the feature/homeTap-hyerjang-#41 branch 2 times, most recently from fd26d68 to 082efa7 Compare July 14, 2024 08:45
@hryeong66 hryeong66 force-pushed the feature/homeTap-hyerjang-#41 branch from 082efa7 to 7699892 Compare July 15, 2024 09:14
@hryeong66 hryeong66 merged commit 0e14e13 into develop Jul 15, 2024
3 checks passed
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.

[Feature] 홈탭 디자인 수정
2 participants