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_: saving profile image changes ens display name to non-ens #5156

Merged
merged 1 commit into from
May 15, 2024

Conversation

kounkou
Copy link
Contributor

@kounkou kounkou commented May 14, 2024

Description

When a user updates the preferred name, we should just ignore if the name is an ENS name, because the ENS name is set only in the ENS popup. But this also means that if a user set the profile name to an ENS name, this change will also be ignored.

Detailed description/Analysis

I think I understand the problem. Let me try to summarize it.

Previously, we faced issue where the user could change the displayed name to x.y.z, and this prevented other clients from displaying messages due to the validation on the display name.

However, ENS names have the format x.y.z, for example : kounkou.stateofus.eth, and in the current ticket, we fall into a dilemma where we have 2 cases :

1- If we have a display name not being in the x.y.z format and we want to change to the x.y.z format, the validation will fail.

2- If we had 2 ENS names to choose from, and we decided to move from one to another ENS name, then the validation will fail for the same.

Case 2 raises the question :

  • How is the ENS name set in the first place ?

This is because upon updating the ENS name, the actual call to update settings is https://github.com/status-im/status-desktop/blob/master/src/app_service/service/settings/service.nim#L148

Solution Proposal

When a user update the preferred name, we should just ignore if the name is an ENS name, because the ENS name is set only in the ENS popup. But this also means that if a user set the profile name to an ENS name, this change will also be ignored.

fixes status-im/status-desktop#14048

@kounkou kounkou self-assigned this May 14, 2024
@kounkou kounkou requested a review from mprakhov May 14, 2024 08:23
@status-im-auto
Copy link
Member

status-im-auto commented May 14, 2024

Jenkins Builds

Click to see older builds (8)
Commit #️⃣ Finished (UTC) Duration Platform Result
✖️ 1ad6dff #1 2024-05-14 08:25:36 ~3 min tests 📄log
✔️ 1ad6dff #1 2024-05-14 08:26:57 ~4 min linux 📦zip
✔️ 1ad6dff #1 2024-05-14 08:27:42 ~5 min ios 📦zip
✔️ 1ad6dff #1 2024-05-14 08:28:37 ~6 min android 📦aar
✔️ d62876c #2 2024-05-14 08:30:37 ~1 min android 📦aar
✔️ d62876c #2 2024-05-14 08:30:52 ~2 min linux 📦zip
✔️ d62876c #2 2024-05-14 08:32:01 ~3 min ios 📦zip
✔️ d62876c #2 2024-05-14 09:11:19 ~42 min tests 📄log
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ f86103c #3 2024-05-14 15:20:04 ~2 min android 📦aar
✔️ f86103c #3 2024-05-14 15:20:13 ~2 min linux 📦zip
✔️ f86103c #3 2024-05-14 15:21:29 ~3 min ios 📦zip
✔️ f86103c #3 2024-05-14 16:02:07 ~44 min tests 📄log

@kounkou kounkou removed the request for review from mprakhov May 14, 2024 08:26
@kounkou kounkou force-pushed the fix-saving-profile-image-changes-ens-display-name branch from 1ad6dff to d62876c Compare May 14, 2024 08:28
@kounkou kounkou requested a review from mprakhov May 14, 2024 08:28
Copy link
Member

@jrainville jrainville left a comment

Choose a reason for hiding this comment

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

Looks good apart from the IsENS function needing a small fix

common/utils.go Outdated Show resolved Hide resolved
When a user updates the preferred name, we should just ignore if the name is an ENS name,
because the ENS name is set only in the ENS popup. But this also means that if a user set
the profile name to an ENS name, this change will also be ignored.
@kounkou kounkou force-pushed the fix-saving-profile-image-changes-ens-display-name branch from d62876c to f86103c Compare May 14, 2024 15:17
Copy link
Contributor

@MishkaRogachev MishkaRogachev left a comment

Choose a reason for hiding this comment

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

Looks good!

@kounkou kounkou merged commit bf56cb7 into develop May 15, 2024
9 checks passed
@kounkou kounkou deleted the fix-saving-profile-image-changes-ens-display-name branch May 15, 2024 19:37
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.

Saving profile image changes ENS display name to non-ENS
4 participants