"Remove current photo" is shown for new account and profile info is duplicated in header after any change

[churik]

Bug Report

Problem

Expected behavior

1. Removing photo is not shown when photo is not set
2. The header is not changed after removing/setting custom photo

If you scrolling and switching between profile and other tabs.

Actual behavior

Reproduction

• Open Status
• Create new multiaccount
• Open Profile
• (issue 1) Tap on photo > Edit
• (issue 2) Remove photo (or set new)

Additional Information

• Status version: nightly 17/10/2019
• Operating System: Android, iOS

[churik]

@rachelhamlin issue doesn't block user from anything, but it is common action for a majority of users.
So added in v1 (remove if you don't think it is important for v1)
I believe can be bounty.

[errorists]

🙌
I'm also experiencing the duplicate header more often than I'd like, (seen above on the video), would really appreciate if there's a fix for that.

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

This issue now has a funding of 80.0 DAI (80.0 USD @ $1.0/DAI) attached to it.

• If you would like to work on this issue you can 'start work' on the Gitcoin Issue Details page.
• Want to chip in? Add your own contribution here.
• Questions? Checkout Gitcoin Help or the Gitcoin Slack
• $86,040.17 more funded OSS Work available on the Gitcoin Issue Explorer

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

Work has been started.

These users each claimed they can complete the work by 2 weeks, 6 days from now.
Please review their action plans below:

1) acolytec3 has been approved to start work.

Identify source of bug, likely not updating a subscription or db field correctly when setting the profile picture, and code appropriate fix

Learn more on the Gitcoin Issue Details page.

[acolytec3]

@churik @yenda @flexsurfer @errorists Not sure who to direct this question to but in looking at the underlying code, is it safe to assume that if a user ever adds an image as their profile picture, it will always be jpeg and if it's the system generated identicon, it will always be png? It looks like the update-picture event assumes any user-provided picture will be jpeg. I'm hoping this is the case as it should make a fix relatively straight-forward in terms of adjusting the display rules for the above bug can be guided by inferring whether to display the "Remove photo" option based on whether the photo-path is jpeg or png.

[errorists]

To me it’s not as clear, I have pictures in my gallery that aren’t JPG and I can choose to use any one of them as my profile pic. Unless we save these locally as jpegs, chances are you’d end up using PNGs for avatar pictures too.

[acolytec3]

@errorists That's what I wondered too but when I set an image with a PNG extension as my profile pic, re-frisk is still showing the photo-path value as a base64/jpeg. I think that regardless of the actual format, it's being saved as a bas64/jpeg representation by update-picture and that seems to work just fine. So, I think should be a safe assumption unless somebody else knows otherwise. I'll get a PR started tonight or first thing tomorrow.

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

Work for 80.0 DAI (80.0 USD @ $1.0/DAI) has been submitted by:

1. @acolytec3

@StatusSceptre please take a look at the submitted work:

• PR by @acolytec3

---

• Learn more on the Gitcoin Issue Details page
• Want to chip in? Add your own contribution here.
• Questions? Checkout Gitcoin Help or the Gitcoin Slack
• $86,217.21 more funded OSS Work available on the Gitcoin Issue Explorer

[bitsikka]

@errorists That's what I wondered too but when I set an image with a PNG extension as my profile pic, re-frisk is still showing the photo-path value as a base64/jpeg. I think that regardless of the actual format, it's being saved as a bas64/jpeg representation by update-picture and that seems to work just fine. So, I think should be a safe assumption unless somebody else knows otherwise. I'll get a PR started tonight or first thing tomorrow.

@acolytec3

I have worked in these areas and have few notes I'd like to share

I too have used profile pic being saved as base64/jpeg to determine whether or not to render border around profile pic.

So that approach also works.

Still, fwiw, In the past, it worked as intended, with a little bit different implementation (and so this bug is a regression).

The logic used to be, to compare photo-path with string returned by identicon/identicon function.

in status-im.ui.screens.profile.user.views

(defn- header [{:keys [public-key photo-path] :as account}]
  [profile.components/profile-header
   {:contact                account
    :allow-icon-change?     true
    :include-remove-action? (not= (identicon/identicon public-key) photo-path)}])

[acolytec3]

@bitsikka I like that approach. Solves the issue more exactly than mine does. That said, it feels like execution time might be slightly longer if you're generating an identicon every time you render the page versus just looking at the first part of the string. I dunno if it's enough to make a difference in page load so will leave it to the reviewers on whether to change. Thanks for the suggestion!

[bitsikka]

@bitsikka I like that approach. Solves the issue more exactly than mine does. That said, it feels like execution time might be slightly longer if you're generating an identicon every time you render the page versus just looking at the first part of the string. I dunno if it's enough to make a difference in page load so will leave it to the reviewers on whether to change. Thanks for the suggestion!

@acolytec3

👍

good point about perf..
Something worth looking into maybe: recently, there was a change(related to perf - moving to golang) in the way identicons are generated(and maybe cached - not sure)

Edit: confirmed! identicon/identicon is memoized (plus, one/first time execution done in status-go) - pretty sure ok to use perf wise

[yenda]

@bitsikka @acolytec3 I think checking if the photo-path is the same as the identicon is the most robust solution. Also there is no point doing that check more than once. You can do it at login, and store some value in app-db like personalized-picture? that you set to true if the two are different.

[bitsikka]

@bitsikka @acolytec3 I think checking if the photo-path is the same as the identicon is the most robust solution. Also there is no point doing that check more than once. You can do it at login, and store some value in app-db like personalized-picture? that you set to true if the two are different.

@yenda
indeed the most robust.. 👍 I'm still catching up with updated changes :) and somewhat stuck behind in old implementations. I understand you mean that both identicon and photo-path fields are there in the new db, whereas iirc only photo-path was there before. Or, something like that.

[acolytec3]

@yenda I'll update my PR along those lines.

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

The funding of 80.0 DAI (80.0 USD @ $1.0/DAI) attached to this issue has been approved & issued to @acolytec3.

• Learn more on the Gitcoin Issue Details page
• Questions? Checkout Gitcoin Help or the Gitcoin Slack
• $124,836.95 more funded OSS Work available on the Gitcoin Issue Explorer