-
Notifications
You must be signed in to change notification settings - Fork 979
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
Resolve username to ENS after 1-1 chat opened by ENS search #12100
Conversation
Jenkins BuildsClick to see older builds (28)
|
96% of end-end tests have passed
Failed tests (3)Click to expand
Passed tests (77)Click to expand
|
ISSUE 1: ENS name changes to a 3-random name in the chat list when focusing on the search field, if 1-1 chat with this user was started using
|
Good catch @qoqobolo, I will work on both issues and let you know. |
acf441b
to
0031d55
Compare
@qoqobolo Fixes are pushed. Let me know if other issue arises! |
@briansztamfater thanks for fix! |
ISSUE 2: ENS is displayed as a nickname in user's profile if User1 adds to contacts User2 via
|
@qoqobolo Fixed both and should be also fixed the issue when closing / reopening the app |
@briansztamfater thank you! All three issues have been fixed, great! ISSUE 4: ENS names aren't resolved in the mention list if chat with ENS user was started by
|
99% of end-end tests have passed
Failed tests (1)Click to expand
Passed tests (79)Click to expand
|
@qoqobolo Hey! No problem, PR is getting bigger than expected but I will check one by one if they are related to these changes. |
@qoqobolo New issues should be fixed now. Let me know how it goes :) |
src/status_im/add_new/core.cljs
Outdated
:valid) | ||
:error error | ||
:ens-name (resolver/ens-name-parse new-ens-name)})} | ||
(merge {:db (as-> db $ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you please elaborate on using as->
. here ? thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was to add an if statement inside the macro, but I'm pretty sure there are better ways to do it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cond->
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That looks way cleaner :)
:chats/loading? false) | ||
:dispatch-n [[:chat/start-timeline-chat] | ||
[:start-profile-chat (get-in db [:multiaccount :public-key])]]} | ||
(doseq [chat chats-v] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is somehting really wrong, i don't understand this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we cant fx/merge doseq
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we want to doseq all chats? i believe we need to verify only one entered name ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When closing / opening the app the ens-names on chats are lost so the main idea was to check ens-names and verify them again for each chat that was started through ENS search. I'm open to other approaches as this is not one I'm proud of.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When closing / opening the app the ens-names on chats are lost
hm why are they lost ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If user is not saved to contacts after initializing a chat, ens name is lost. I'm trying to figure out why that's happening, but maybe we could investigate that in another issue and simplify this PR, because that is happening right now also when starting a chat with an ENS user from the search bar.
src/status_im/contact/db.cljs
Outdated
@@ -116,6 +116,10 @@ | |||
([db public-key] | |||
(active? (get-in db [:contacts/contacts public-key])))) | |||
|
|||
(defn add-ens-name | |||
[contact ens-name] | |||
(if ens-name (assoc-in contact [:name] ens-name) contact)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why assoc-in
?
src/status_im/subs.cljs
Outdated
(assoc contacts chat-id (get contacts chat-id (-> chat-id | ||
contact.db/public-key->new-contact | ||
contact.db/enrich-contact))) | ||
contact.db/enrich-contact | ||
(contact.db/add-ens-name ens-name?)))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can just use (get ens-names chat-id)
directly here
@qoqobolo @flexsurfer Hey so I just simplified this PR as I think I over engineered at first. There's one issue that I detected but maybe would be a good idea to cover it in another issue, and it is when you don't add ENS user to contacts, the ENS name is lost after closing / opening the app. But that's already happenning when starting a chat with an ENS user from the search bar, so I would file a new issue and investigate there. Let me know what do you think. |
@briansztamfater okay, I'll log a new issue with that case. As for the previous problems - they are all fixed. Thanks for your great work! |
c702b56
to
250d68f
Compare
…arch from 'Start new chat' by ENS Signed-off-by: Brian Sztamfater <brian@status.im>
250d68f
to
929925a
Compare
fixes #12027
Summary
We need to resolve username to ENS after 1-1 chat opened by ENS search. This PR approach is to get the ens name from
contacts/new-identity
db key if exists, and include it incontacts/contact-two-names-by-identity
returning contact. Also include ens-name as default nickname if adding to contacts from the chat.Platforms
Areas that maybe impacted
Functional
Steps to test
status: ready