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: Add chaining RxJava #2117

Merged
merged 1 commit into from
Jul 26, 2019
Merged

feat: Add chaining RxJava #2117

merged 1 commit into from
Jul 26, 2019

Conversation

anhanh11001
Copy link
Contributor

Details:

  • Add chaining RxJava on loading order details, signing up, editting user profile
  • Remove unused methods

Fixes: #2116

nikit19
nikit19 previously approved these changes Jul 14, 2019
else
authService.uploadImage(UploadImage(encodedImage)).flatMap {
authService.updateUser(User(id = id, firstName = firstName, lastName = lastName,
avatarUrl = it.url, details = details), id)
Copy link
Member

Choose a reason for hiding this comment

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

Repetition, please use data class copy constructor

val signUpObservable: Single<User> = authService.signUp(signUp).flatMap {
email = signUp.email
password = signUp.password
authService.login(email.nullToEmpty(), password.nullToEmpty()).flatMap {
Copy link
Member

Choose a reason for hiding this comment

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

Chaining should not be nested

Copy link
Member

Choose a reason for hiding this comment

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

Still not resolved. Chaining should not be nested. That's the point of chaining

liveHarshit
liveHarshit previously approved these changes Jul 15, 2019
Copy link
Member

@iamareebjamal iamareebjamal left a comment

Choose a reason for hiding this comment

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

Remove obsolete variables

val signUpObservable: Single<User> = authService.signUp(signUp).flatMap {
email = signUp.email
password = signUp.password
authService.login(email.nullToEmpty(), password.nullToEmpty()).flatMap {
Copy link
Member

Choose a reason for hiding this comment

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

Still not resolved. Chaining should not be nested. That's the point of chaining

@anhanh11001 anhanh11001 requested a review from nikit19 July 25, 2019 01:55
@anhanh11001
Copy link
Contributor Author

@iamareebjamal, please review

@iamareebjamal iamareebjamal merged commit 573e2ca into fossasia:development Jul 26, 2019
@anhanh11001 anhanh11001 deleted the 2116_chaining_rx_java branch July 26, 2019 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add RxJava chaining request call
4 participants