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/#98 사용자가 검색한 키워드 기록 및 조회 기능 구현 #120

Merged
merged 14 commits into from
Feb 15, 2024

Conversation

iowa329
Copy link
Collaborator

@iowa329 iowa329 commented Feb 7, 2024

🧑🏻‍💻 PR 작업 내역 요약

비로그인/로그인 사용자의 키워드 검색 내용을 기록하고 조회하는 기능을 구현하였습니다.

  • 로그인 사용자의 경우 서버와의 연결을 통해 검색기록을 조회하고 개별 혹은 전체 키워드를 지울 수 있게 네트워크 코드를 추가하였습니다
  • 비로그인 사용자의 경우 CoreData를 사용하여 디바이스 로컬에 저장하고 조회하는 방식으로 기능을 구현하였습니다

📋 변경 사항

검색기록 카테고리 탭바

  • 이전 회의내용에 따라 1차 출시까지 검색기록의 카테고리별 분류 기능을 보류하고 단순한 사용자 키워드 검색내역 표시로 구현하기 위해 탭바를 UI상 표시되지 않게 조치하였습니다

CoreData

  • CoreData 엔티티 항목에 검색기록 관리를 위한 SearchHistory Entity가 추가되었습니다

APISession

  • 키워드 삭제 등의 기능구현을 위해 HTTP Method의 DELETE 방식과 관련된 서버코드를 추가하였습니다

🔍 Code Review

비로그인 환경에서 전체지우기 버튼을 클릭하면 기능은 정상작동하지만 전체지우기 버튼이 비활성화(isEnabled = false)가 되지 않는 문제가 있습니다. 키워드를 일일이 개별로 삭제하면 같은 방식으로 버튼이 비활성화(isEnabled = false) 되면서 전체지우기 버튼 디자인이 비활성화 형태로 변경되어서 같은 코드에서 왜 다른 결과가 나오는지 몰라 문제를 해결하지 못했습니다 🥲

📸 ScreenShot

연결된 이슈 Close

close #98

@iowa329 iowa329 requested a review from kth1210 February 7, 2024 20:32
Copy link
Member

@kth1210 kth1210 left a comment

Choose a reason for hiding this comment

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

ㅎㄷㄷ 새벽에 몇시까지 작업하신건가요 희재님 ㅋㅋㅋㅋㅋㅋ
고생하셨습니다~!
PR 작성 완료되면 다시 한 번 확인해볼게요~

Comment on lines 538 to 540

print(list.count > 0)

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.

아 저 친구 제거했습니다 새벽이 되어서 총기가 떨어진 저를 자책합니다...ㅎ

@iowa329 iowa329 changed the title [PR 작성중] Feat/#98 search history network Feat/#98 사용자가 검색한 키워드 기록 및 조회 기능 구현 Feb 8, 2024
@iowa329 iowa329 self-assigned this Feb 8, 2024
@iowa329 iowa329 added View 화면(layout) 관련 개발 Network 서버(네트워크) 코드 관련 개발 Feature 새로운 기능 개발 희재 희재과 관련된 개발(Issue or PR) 검색 검색 관련 개발 labels Feb 8, 2024
Copy link
Member

@kth1210 kth1210 left a comment

Choose a reason for hiding this comment

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

흠 코드리뷰란에 남겨주신 사항 저도 확인해봤는데 이상하긴 하네요..?
출력 찍어봐도 정상적으로 나오고 있더라구요,,

전체 지우기 후에 다시 검색바를 선택하니까 또 비활성화가 적용이 되는 것 같아서 스레드의 문제인가? 싶었는데 로그인 여부에 따라 이 현상이 차이나는게 이상하네요..

viewModel output 관리 관련해서 코멘트 남겨두었으니 확인해주시면 감사드립니당

Comment on lines 194 to 198
private func isShowSearchResultUI(show: Bool) {
if !show {
updateSearchHistoryListStatus()
viewModel.output.searchHistory.isUpdated.accept(true)
searchHistoryListCollectionView.scrollToTop()
}
Copy link
Member

Choose a reason for hiding this comment

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

이 부분도 저희 viewModel 관리 방법에 대해서 고민이 있는 부분인데요,,
VC에서 viewModel의 output에 새로운 값을 accept 한다는게 어색한 것 같아요ㅠㅜ (이전에 이런 경우가 꽤 있었지만)
viewModel -> VC 방향은 output으로,
VC -> viewModel 방향은 input으로 관리해주어야 발생할 수 있는 사이드 이펙트를 막을 수 있을 것 같습니다..

간단하게 요약하자면 VC는 viewModel의 output을 보여주기만 하고, 들어오는 입력은 input으로 처리하는 것이죠 🤔
이렇게 되면 output이 어디서 바뀌어서 화면에 잘못 노출될지 알 수 없으니까요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이슈로 등록한 후에 차후 MVVM 패턴 수정을 해보도록 하겠습니다!

Comment on lines +249 to +255
private func configureSearchHistory() {
DispatchQueue.main.asyncAfter(deadline: DispatchTime.now() + 0.01) { [weak self] in
guard let self = self else { return }

self.viewModel.output.searchHistory.isNeedUpdated.accept(true)
}
}
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

@iowa329 iowa329 Feb 15, 2024

Choose a reason for hiding this comment

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

비로그인 상황일 경우 처음 SearchVC에 진입하면서 configureSearchHistory() 를 통해 로컬 디바이스에 CoreData로 저장되어 있는 검색기록을 불러오는데 검색화면이 전부 구성되기 전에 이미 데이터를 불러오고 reloadData()를 하는 경우가 발생해서 실질적으로 화면에 아무것도 나타나지 않는 문제가 있었습니다. VC의 라이프 사이클에 따른 문제라고 보고 해당 코드 실행 시간을 지연시켰더니 올바르게 표시가 되어서 해당 코드를 작성하게 되었습니다

근데 어떻게 보면 self.viewModel.output.searchHistory.isNeedUpdated.accept(true) 이 코드가 의미하는 부분이 명확하지 않아 헷갈릴 수 있는 것 같습니다. 해당 부분은 차후 위에 언급해주신 MVVM 패턴의 Input, Ouput 을 적용해보면서 코드를 이해할 수 있게 수정해보도록 하겠습니다!

@iowa329 iowa329 merged commit c357eab into develop Feb 15, 2024
1 check passed
@iowa329 iowa329 deleted the feat/#98-SearchHistory-Network branch February 15, 2024 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature 새로운 기능 개발 Network 서버(네트워크) 코드 관련 개발 View 화면(layout) 관련 개발 검색 검색 관련 개발 희재 희재과 관련된 개발(Issue or PR)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEAT] 최근 검색어 1차 배포 버전 기능 연결
2 participants