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

fix: Add error utils, fix stripe error message and charge data class #2269

Merged
merged 1 commit into from Aug 14, 2019

Conversation

liveHarshit
Copy link
Member

Part of #2133

@auto-label auto-label bot added the fix label Aug 13, 2019
nikit19
nikit19 previously approved these changes Aug 13, 2019
mutableMessage.value = resource.getString(R.string.payment_not_complete_message)
if (it is HttpException) {
val body = it.response()?.errorBody()
val jsonObject = JSONObject(body?.string()).getJSONArray(ERRORS)[0]
Copy link
Member

Choose a reason for hiding this comment

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

This will crash the app if not present

Copy link
Member Author

Choose a reason for hiding this comment

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

So I think, try-catch can be used.

Copy link
Member

Choose a reason for hiding this comment

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

Copy ErrorUtils from orga app and use getErrorDetails, so this can be handled once for everywhere

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated

@liveHarshit liveHarshit changed the title fix: Stripe error details and charge class fix: Add error utils, fix stripe error message and charge data class Aug 13, 2019
errorCode == FORBIDDEN ||
errorCode == NOT_FOUND ||
errorCode == METHOD_NOT_ALLOWED ||
errorCode == REQUEST_TIMEOUT) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any problem using Kotlin when expression for this here, I think it will look cleaner

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, can't get it?

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 Author

Choose a reason for hiding this comment

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

updated

anhanh11001
anhanh11001 previously approved these changes Aug 13, 2019
Copy link
Contributor

@anhanh11001 anhanh11001 left a comment

Choose a reason for hiding this comment

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

Rest look good to me

ShridharGoel
ShridharGoel previously approved these changes Aug 13, 2019
@fossasia fossasia deleted a comment Aug 14, 2019
@fossasia fossasia deleted a comment Aug 14, 2019
@fossasia fossasia deleted a comment Aug 14, 2019
@fossasia fossasia deleted a comment Aug 14, 2019
@fossasia fossasia deleted a comment Aug 14, 2019
@iamareebjamal iamareebjamal merged commit 691a8b3 into fossasia:development Aug 14, 2019
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.

None yet

5 participants