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] #199 - 출석하기 API 연결 및 에러 대응 #201

Merged
merged 25 commits into from
Apr 23, 2023

Conversation

0inn
Copy link
Contributor

@0inn 0inn commented Apr 22, 2023

🌴 PR 요약

🌱 작업한 브랜치

🌱 PR Point

출석하기 기능 관련 API 연결하고, 에러 대응했습니다.

📌 참고 사항

  1. testWithError 추가
    에러 상황 테스트가 필요해서 404 에러로 추가했습니다 !

  2. OPAPIError 구현
    서버에서 String 타입의 에러 메세지를 받아 그에 맞게 출석하기 버튼의 title과 isEnabled를 지정해줘야했습니다.
    UseCase에서 해당 로직을 구현했고, 그래서 OPAPIError와 BaseEntity는 Core 모듈에 추가해주었습니다.

  3. 운영서비스용 opRequest~()추가
    운영서비스의 응답값의 경우, BaseEntity가 포함된 응답값으로 떨어지며, 에러도 마찬가지입니다.
    에러 처리를 하기 위해서 운영서비스용 request를 추가하여 BaseEntity로 디코딩하는 로직을 추가해주었습니다.

  4. 운영서비스 url path 수정
    서버 변경값 반영했습니다 . .!

  5. 출석하기 API 에러 대응
    현재, 출석 조회 화면에서 출석이 가능한 시간대에만 200이고, 나머지 경우엔 error 상황입니다.
    이 때, 200으로 model을 받은 경우에만, 하단 출석하기 버튼을 활성화하였고,
    나머지 에러상황에선 비활성화 및 받은 에러 메세지를 화면에 맞게 반환하여 보여줍니다.

  6. 출석 코드 서버 연결
    출석하기 버튼을 눌러 성공한 경우, 출석 숫자 코드 모달창을 dismiss합니다.
    실패한 경우, 해당 에러 메세지를 그대로 표시하고, 숫자 코드 부분을 초기화 합니다.

출석 성공 출석 실패

가능한 모든 케이스에 대해 테스트를 해봤지만, 실제 서버로 테스트를 한 번도 진행하지 못했습니다 . .^^(데이터 바꾸기가 불가능해서)
일단 구현은 전부 했습니다 !

📸 스크린샷

출석하기 버튼 케이스
image

📮 관련 이슈

@0inn 0inn added Feat 새로운 기능 구현 영인☁️ labels Apr 22, 2023
@0inn 0inn self-assigned this Apr 22, 2023
Copy link
Contributor

@L-j-h-c L-j-h-c left a comment

Choose a reason for hiding this comment

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

수고하셨어요~~!

if model.type == SessionType.hasAttendance.rawValue {
self.setAttendances(model.attendances)
owner.self.setAttendances(model.attendances)
Copy link
Contributor

Choose a reason for hiding this comment

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

😎 owner.self 😎

Copy link
Contributor Author

Choose a reason for hiding this comment

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

헉. 당장 수정하겠습니다 . ^^

Copy link
Member

@lsj8706 lsj8706 left a comment

Choose a reason for hiding this comment

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

역시 야무지게 잘하시네요!!!
저도 코드 많이 참고하겠습니다!! 👍


import Foundation

public enum OPAPIError: LocalizedError {
Copy link
Member

Choose a reason for hiding this comment

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

LocalizedError 활용 좋네요!!
저도 에러 처리할 때 참고하겠습니다!

print("failure: postAttendance \(error)")
case .finished:
print("completion: postAttendance \(event)")
.catch({ error in
Copy link
Member

Choose a reason for hiding this comment

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

멋져요 👍👍

if let error = error as? OPAPIError {
var errorMsg = error.errorDescription ?? ""
self.setLectureRoundErrorTitle(&errorMsg)
self.lectureRoundErrorTitle.send(errorMsg)
Copy link
Member

Choose a reason for hiding this comment

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

궁금한 점이 있어요!
에러 메시지를 viewModel로 보내는 방식을 사용하게 된다면 아마도 에러가 발생할 수 있는 API 개수만큼 에러 메시지 Publisher를 생성해야하겠죠..?
혹시 기존의 lectureRound의 에러타입을 Never가 아닌 OPAPIError로 정의해서 따로 메시지 퍼블리셔를 안만들고 lectureRound에 OPAPIError를 send(completion:) 하는 방식은 어떨까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Error 핸들링 로직은 UseCase에 있는게 맞다는 판단으로 여기서 처리했습니다.
단순히 메세지만 넘기는 것도 아닌 메세지 가공(?)도 필요해서요 . . !

Copy link
Member

Choose a reason for hiding this comment

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

엇! 네!! 에러 핸들리은 UseCase에 있는 것이 맞아요!
제가 궁금했던 거는 lectureRoundErrorTitle이라는 퍼블리셔를 만들어서 에러 메시지를 보내고 있는데 혹시 연결하는 api 개수가 많아지면 ~~~ErrorTiile 이라는 퍼블리셔를 계속 만들어야 하는 지가 궁금했어요!


import Foundation

enum SampleData {
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

@devxsby devxsby left a comment

Choose a reason for hiding this comment

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

뷰도 너무 잘나왔고 에러처리랑 잔잔바리 수정까지 넘 좋아요.
진짜 쵝오. 왤케 잘함.? 👁️👁️
갠톡으로 연락드린 거 2개만 추가 번용 부탁 드립니다 스앵님,, 감사합니다 ,,, 고생하셨어용 ,,

Comment on lines 172 to 174
} catch let error {
promise(.failure(error))
}
Copy link
Member

Choose a reason for hiding this comment

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

질문있습니당 요기 catch 하는 부분에서 에러 thorw하는데 예외가 반환이 안되고 다음 라인으로 넘어가는데 return 추가하고 함수 나가도록 해야하는거 아닌가요오...??

Suggested change
} catch let error {
promise(.failure(error))
}
} catch let error {
promise(.failure(error))
return
}

Copy link
Contributor Author

@0inn 0inn Apr 22, 2023

Choose a reason for hiding this comment

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

아래 케이스도 확인해야할 것 같아서
retryError가 아니라면 해당 디코딩 로직을 넣어줄게요 !

Comment on lines 176 to 183
if case MoyaError.underlying(let error, _) = error,
case AFError.requestRetryFailed(let retryError, _) = error,
let retryError = retryError as? APIError,
retryError == APIError.tokenReissuanceFailed {
promise(.failure(retryError))
} else {
promise(.failure(error))
}
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
Contributor Author

Choose a reason for hiding this comment

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

위 코멘트처럼 바꿔야할 것 같아서 수정해둘게요 !


extension AttendanceAPI {

public var sampleData: Data {
Copy link
Member

Choose a reason for hiding this comment

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

그녀의 고군 분투 . . .

Comment on lines 91 to 99
.catch({ error in
if let error = error as? OPAPIError {
var errorMsg = error.errorDescription ?? ""
self.setLectureRoundErrorTitle(&errorMsg)
self.lectureRoundErrorTitle.send(errorMsg)
}

return Just<AttendanceRoundModel?>(nil)
})
Copy link
Member

Choose a reason for hiding this comment

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

위에 코멘트 남긴 부분 반영되면 호출부는 이렇게 될거같아여

Suggested change
.catch({ error in
if let error = error as? OPAPIError {
var errorMsg = error.errorDescription ?? ""
self.setLectureRoundErrorTitle(&errorMsg)
self.lectureRoundErrorTitle.send(errorMsg)
}
return Just<AttendanceRoundModel?>(nil)
})
.catch({ error in
if let errorMsg = AttendanceErrorMsgType.getTitle(for: error) {
self.lectureRoundErrorTitle.send(errorMsg)
}
return Just<AttendanceRoundModel?>(nil)
})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

조아요 ~~! 👍🏻

case .afterSecondAttendance:
return I18N.Attendance.afterNthAttendance(2)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

저는 요기 에러 케이스 부분에 static 함수로 title까지 반환하는 메서드가 있으면 조금 더 간단할 수 있을까 생각해요...!

public static func getTitle(for error: Error) -> String? {
        guard let errorMsg = (error as? OPAPIError)?.errorDescription else {
            return nil
        }
        
        return Self.allCases.first { $0.rawValue == errorMsg }?.title
    }

이렇게하면 setLectureRoundErrorTitle 함수를 없앨 수 있을 것 같습니다!!! 영인님은 어떻게 생각하시나용???!! 편하게 의견주세요~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

반영해두겠습니당 ~! 더 편해졌네용 ☺️

public enum OPAPIError: LocalizedError {
case attendanceError(BaseEntity<Data>)

public var errorDescription: String?{
Copy link
Member

Choose a reason for hiding this comment

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

진짜 나 개깐깐징어같은데 여기 한 칸 안띄웠어 . . .

Suggested change
public var errorDescription: String?{
public var errorDescription: String? {

내가 미안해 . . .

self.round = round
}

public static let EMPTY: Self = .init(subLectureId: 0, round: 0)
Copy link
Member

Choose a reason for hiding this comment

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

👍🏻

var attendSuccess: PassthroughSubject<Bool, Never> { get set }
var attendErrorMsg: PassthroughSubject<String, Never>{ 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.

Suggested change
var attendErrorMsg: PassthroughSubject<String, Never>{ get set }
var attendErrorMsg: PassthroughSubject<String, Never> { get set }

죄송함다 , , 쩝 ,.

@@ -264,6 +280,23 @@ extension AttendanceVC {
owner.attendanceButton.isEnabled = isEnabled
}
.store(in: self.cancelBag)

output.attendSuccess
.filter { $0 }
Copy link
Member

Choose a reason for hiding this comment

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

좋다 ~!

@devxsby
Copy link
Member

devxsby commented Apr 23, 2023

새로 푸시한거 확인 다 했습니다! 고생하셨어용 🫶🏻

@devxsby
Copy link
Member

devxsby commented Apr 23, 2023

@0inn 0inn merged commit d82477c into sopt-makers:develop Apr 23, 2023
@0inn 0inn deleted the feat/#199 branch April 23, 2023 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feat 새로운 기능 구현 size/XXL 영인☁️
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feat] 출석하기 API 연결
4 participants