-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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 contact saved handle format #740
base: master
Are you sure you want to change the base?
Conversation
Hey @laktyushin can you please help reviewing this pull request? |
@@ -1072,7 +1072,8 @@ public func deviceContactInfoController(context: AccountContext, updatedPresenta | |||
if let editingName = state.editingState?.editingName, case let .personName(firstName, lastName, _) = editingName, (!firstName.isEmpty || !lastName.isEmpty) { | |||
var urls = filteredData.urls | |||
if let createForPeer = createForPeer { | |||
let appProfile = DeviceContactUrlData(appProfile: createForPeer.id) | |||
let addressName: String = createForPeer.addressName ?? "" | |||
let appProfile = DeviceContactUrlData(addressName: addressName) |
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.
It looks like you're using username instead of a raw numeric user ID, so the contact link may become invalid later as usernames may change, whereas user ID stays the same, so I would recommend using it instead.
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.
Up: I did some research and it seems like other clients (desktop, web) have no support for deep links right now so maybe it is fine to stick with usernames.
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.
Yes, username is working fine currently. So this pr should be good to go then?
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.
Well, this code was using deep links before, so maybe there was a reason for it. Anyways now it seems ok to me so let's wait for the others to decide (as I'm not a contributor).
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.
hey can we get this merged plz 🥹🙏
Resolves: #685
Contact saved with peer id instead of handlename as https://t.me/@idId(rawValue:%xxx
After change: