feat: proposal for parsing shared urls #3644
Conversation
Pull Request Checklist
|
Jenkins BuildsClick to see older builds (17)
|
MishkaRogachev
force-pushed
the
feat/parse-shared-url
branch
from
14b69bb to
cd2f4c2
Compare
| switch group { | ||
| case "c": | ||
| return m.ParseCommuntyShareURL(data, true) | ||
| case "c#": |
There was a problem hiding this comment.
Probably you had some input for this, but I couldn't find what's c# in this context?
There was a problem hiding this comment.
Answered below :)
| @@ -88,6 +88,8 @@ var communityAdvertiseIntervalSecond int64 = 60 * 60 | |||
| // messageCacheIntervalMs is how long we should keep processed messages in the cache, in ms | |||
| var messageCacheIntervalMs uint64 = 1000 * 60 * 60 * 48 | |||
|
|
|||
| var baseStatusUrl string = "https://status.app/" | |||
There was a problem hiding this comment.
Should we maybe also support:
http://status.app/...directly without url scheme- upper/mixed case
https://STATUS.App/...
?
There was a problem hiding this comment.
Good questions! There are discussion on urls here: status-im/status-web#327, lets raise those questions there
There was a problem hiding this comment.
I think this one is the most up-to-date doc?
https://github.com/vacp2p/rfc/blob/43c07f5290b644d668d3d4e6701a14075ef2d67b/content/docs/rfcs/60/README.md
(from vacp2p/rfc#600)
| @@ -6463,3 +6465,27 @@ func (m *Messenger) handleSyncSocialLinks(message *protobuf.SyncSocialLinks, cal | |||
| func (m *Messenger) GetDeleteForMeMessages() ([]*protobuf.DeleteForMeMessage, error) { | |||
| return m.persistence.GetDeleteForMeMessages() | |||
| } | |||
|
|
|||
| func (m *Messenger) ParseSharedURL(url string) (*MessengerResponse, error) { | |||
| if !strings.HasPrefix(url, baseStatusUrl) { | |||
There was a problem hiding this comment.
Maybe convert url to lower case here?
Or do we consider only everything in lower case as valid?
protocol/messenger_communities.go
Outdated
| if byID { | ||
| // TODO: decompress community key | ||
| communityId := types.HexBytes(url) | ||
| community, err := m.GetCommunityByID(communityId) |
There was a problem hiding this comment.
hmmm... This will only work for "known" communities.
But I suppose in most cases user will user shared url for unknown community. Probably we should start fetching community details here, os smth like that.
Same for profile below. There it will use BuildContact and return smth for unknown contact, but probably still fetching name/image is a good idea.
There was a problem hiding this comment.
There are 2 groups: 'c#' which only provides community ID and c with community data in protobuf. Here i'm waiting for conversion functions from Boris
| return nil, err | ||
| } | ||
|
|
||
| contact, err := m.BuildContact(&requests.BuildContact{PublicKey: types.EncodeHex(crypto.FromECDSAPub(pubKey))}) |
There was a problem hiding this comment.
Should we support ENS names in shared urls, like status.app/u/vitalik.eth? 🤔
|
Basically the solution is great, just a bunch of minor questions 🙂 |
MishkaRogachev
force-pushed
the
feat/parse-shared-url
branch
4 times, most recently
from
5f4f81f to
6953065
Compare
MishkaRogachev
force-pushed
the
feat/parse-shared-url
branch
from
6953065 to
e0f4f45
Compare
|
@igor-sirotin i'm closing this PR in favour of #3665 |
For status-im/status-desktop#10851
Implement parsing shared urls in a new format
Important changes: