Skip to content

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