Skip to content

Conversation

@trancenoid
Copy link
Member

Fixes #683

Changes: Added code to check if the profile picture URL is not empty, as Picasso requires a non-empty url. Thus if there is no profile picture initially, there should be no attempt to load such an image.

@trancenoid trancenoid changed the title fixes issue #683 fixes issue #683 App crash on trying to edit profile when the user dont have a profile picture Nov 3, 2018
@trancenoid
Copy link
Member Author

@iamareebjamal Would you please review my PR?

.placeholder(icon)
.transform(CircleTransform())
.into(rootView.profilePhoto)
if(!imageUrl.equals("")) { // picasso requires the imageUrl to be non empty
Copy link
Member

Choose a reason for hiding this comment

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

Use TextUtils.isEmpty

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@iamareebjamal
Copy link
Member

Follow commit message guidelines

…icture

This fix addresses the problem which crashed the app when a user
with no profile picture tries to edit the profile. The imageUrl must not be an empty string, as required by Picasso.get() method
@trancenoid
Copy link
Member Author

@iamareebjamal Updated, but I think I made another commit which is not advises as per the rules, I dont really know the procedure to amend a commit from a forked repo

val drawable = AppCompatResources.getDrawable(ctx, R.drawable.ic_account_circle_grey_24dp)
drawable?.let { icon ->
if(!imageUrl.equals("")) { // picasso requires the imageUrl to be non empty
if(!imageUrl.isEmpty()) { // picasso requires the imageUrl to be non empty
Copy link
Member

Choose a reason for hiding this comment

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

It'll fail on null

Copy link
Member Author

@trancenoid trancenoid Nov 3, 2018

Choose a reason for hiding this comment

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

                val userFirstName = it.firstName.nullToEmpty()
                val userLastName = it.lastName.nullToEmpty()
                val imageUrl = it.avatarUrl.nullToEmpty()
                rootView.firstName.setText(userFirstName)
                rootView.lastName.setText(userLastName)
                context?.let { ctx ->
                    val drawable = AppCompatResources.getDrawable(ctx,   drawable.ic_account_circle_grey_24dp)
                    drawable?.let { icon ->
                        if(!imageUrl.isEmpty()) { // picasso requires the imageUrl to be non empty
                            Picasso.get()
                                         .load(imageUrl).....

@iamareebjamal I think val imageUrl = it.avatarUrl.nullToEmpty() will take care of that

Copy link
Member

Choose a reason for hiding this comment

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

OK. So, we can add the if statement even above the context.let

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, I will do it

Copy link
Member Author

Choose a reason for hiding this comment

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

@iamareebjamal
Copy link
Member

Updated, but I think I made another commit which is not advises as per the rules, I dont really know the procedure to amend a commit from a forked repo

Just update the PR title

@trancenoid trancenoid changed the title fixes issue #683 App crash on trying to edit profile when the user dont have a profile picture Fix : issue #683 : Crash on editing profile with no profile picture Nov 3, 2018
…icture

This fix addresses the problem which crashed the app when a user
with no profile picture tries to edit the profile. The imageUrl must not be an empty string, as required by Picasso.get() method
nikit19
nikit19 previously approved these changes Nov 4, 2018
.transform(CircleTransform())
.into(rootView.profilePhoto)

}
Copy link
Member

Choose a reason for hiding this comment

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

Fix indentation

Copy link
Member Author

Choose a reason for hiding this comment

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

…icture

This fix addresses the problem which crashed the app when a user
with no profile picture tries to edit the profile. The imageUrl must not be an empty string, as required by Picasso.get() method
# Conflicts:
#	app/src/main/java/org/fossasia/openevent/general/auth/EditProfileFragment.kt
.placeholder(icon)
.transform(CircleTransform())
.into(rootView.profilePhoto)
if(!imageUrl.isEmpty()) { // picasso requires the imageUrl to be non empty
Copy link
Member

Choose a reason for hiding this comment

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

Space after if

Copy link
Member Author

Choose a reason for hiding this comment

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

@iamareebjamal Sorry, missed that. I have removed it

Copy link
Member

Choose a reason for hiding this comment

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

Space should be added

Copy link
Member Author

Choose a reason for hiding this comment

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

@iamareebjamal is it correctly done now ?

…icture

This fix addresses the problem which crashed the app when a user
with no profile picture tries to edit the profile. The imageUrl must not be an empty string, as required by Picasso.get() method
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.

Opposite of what I wanted

…icture

This fix addresses the problem which crashed the app when a user
with no profile picture tries to edit the profile. The imageUrl must not be an empty string, as required by Picasso.get() method
if (!imageUrl.isEmpty()) { // picasso requires the imageUrl to be non empty
context?.let { ctx ->
val drawable = AppCompatResources.getDrawable(ctx, R.drawable.ic_account_circle_grey_24dp)
drawable?.let{ icon ->
Copy link
Member

Choose a reason for hiding this comment

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

Space after let

Copy link
Member Author

Choose a reason for hiding this comment

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

…icture

This fix addresses the problem which crashed the app when a user
with no profile picture tries to edit the profile. The imageUrl must not be an empty string, as required by Picasso.get() method
@iamareebjamal
Copy link
Member

OK, now just change the PR title according to the commit guidelines and we are done

@trancenoid trancenoid changed the title Fix : issue #683 : Crash on editing profile with no profile picture fix : App crash while trying to editing a profile with no profile picture Nov 5, 2018
@trancenoid
Copy link
Member Author

@iamareebjamal I have changed the PR title as requested.

@iamareebjamal iamareebjamal merged commit f489b46 into fossasia:development Nov 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants