-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Conversation
Also fix a typo in a comment
@@ -617,6 +617,17 @@ def add_threepid(self, user_id, medium, address, validated_at): | |||
self.hs.get_clock().time_msec() | |||
) | |||
|
|||
@defer.inlineCallbacks |
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.
might be nice to fix the s/presenc e/presence/ typo on line 610
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.
Fixed, although accidentally on develop.
@@ -617,6 +617,17 @@ def add_threepid(self, user_id, medium, address, validated_at): | |||
self.hs.get_clock().time_msec() | |||
) | |||
|
|||
@defer.inlineCallbacks | |||
def delete_threepid(self, user_id, medium, address): | |||
# 'Canonicalise' email addresses as per above |
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're going to hate me for asking this, but: should we be canonicalising MSISDNs here? or are we assuming the client has already determined normal form? (If so, a comment would be useful)
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.
In this case, the msisdn canonicalisation will have been done by the ID server (or the thing doing the verification). In fact, with us now having decided to have a separate thing to do the actual validation of 3pids, the email canonicalisation would go there too (it's a bit tricky for email given that squelching the localpart down to lowercase is not technically valid).
lgtm other than 2 comments |
Also fix a typo in a comment