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

Update link format (to fix inconsistency with desktop) #16810

Closed
qoqobolo opened this issue Jul 28, 2023 · 18 comments
Closed

Update link format (to fix inconsistency with desktop) #16810

qoqobolo opened this issue Jul 28, 2023 · 18 comments
Labels
feature feature requests

Comments

@qoqobolo
Copy link
Contributor

qoqobolo commented Jul 28, 2023

Bug Report

Problem

The link format should be updated for three types of URLs:

  • link to user
  • link to a community
  • link to a channel inside a community

New links format (on Desktop; example for profile links): https://status.app/u/iwSACghxb3FvYm9sbwM=#zQ3shrgSaffLisbiqj2brkDK8Le7wpbpbpzriWd8kPUmdYhs1

Links on Mobile: https://join.status.im/u/zQ3shS1FWUrdQeoLeZKxYhcuFGUW93Rj23psb5XYKzW9z653b

As a result, users can't add each other to contacts by pasting profile links into the search field.

Screenshot 2023-07-28 at 16 34 25 Screenshot 2023-07-28 at 16 41 49

Reproduction

On Mobile

  1. Copy a profile/community/channel link on Desktop (https://status.app/u/... format)
  2. Try to use it on Mobile

Additional Information

  • Status version: nightly 28/07
  • Operating System: Android, iOS
@alwx
Copy link
Contributor

alwx commented Jul 31, 2023

Should all the links to join.status.im be replaced with status.app? If yes then there is this one: #16821

@John-44
Copy link

John-44 commented Jul 31, 2023

Should all the links to join.status.im be replaced with status.app? If yes then there is this one: #16821

@alwx there is much more than simply changing the domain needed to support the new link format. I recommend speaking to @felicio from the web team who designed the new link format and also recommended reviewing the specification for the new link format. The link format is for three types of URLs: link to user, link to a community, link to a channel inside a community.

Desktop team should have already implemented backend to support new link format in status-go, speak to @jrainville for more details.

The old link format should be removed for everywhere, any time a user copies or shares a link to a user, community or channel they should be sharing the new link format. Any input field that accepts a user or community public key should also accept a new format URL.

cc @siddarthkay

@qoqobolo qoqobolo changed the title Update profile link format (to fix inconsistency with desktop) Update link format (to fix inconsistency with desktop) Aug 1, 2023
@alwx alwx removed their assignment Aug 11, 2023
@alwx
Copy link
Contributor

alwx commented Aug 11, 2023

Conclusion: status.app doesn't exist (yet) so it seems like desktop is wrong.

@jrainville
Copy link
Member

I think the plan is to release the new website (status.app) at the same time as doing the public beta for Desktop, which will be at the end of this month.

So yes, the website doesn't exist (publicly) yet, but we're preemptively changing the urls to match with the new website for the release.

@John-44
Copy link

John-44 commented Aug 13, 2023

Conclusion: status.app doesn't exist (yet) so it seems like desktop is wrong.

status.app exists and is currently behind basic authentication until it launches. The User, Community, and Channel web preview pages using the status.app domain are all working as well, again currently behind basic authentication. Contact Web team to get the credentials so you can test. Replacing the old share URLs in the mobile app with the new status.app share URLs (that user a new very different link format) is a high priority item.

@John-44 John-44 added this to the 1.25.0 - Alpha milestone Aug 13, 2023
@alwx
Copy link
Contributor

alwx commented Aug 14, 2023

@John-44 ok, the work is mainly done in this PR: #16821
What's remaining is updating https://github.com/status-im/universal-links-handler, will do it as well this week.

@John-44
Copy link

John-44 commented Aug 15, 2023

@John-44 ok, the work is mainly done in this PR: #16821
What's remaining is updating https://github.com/status-im/universal-links-handler, will do it as well this week.

@alwx thanks! Can you confirm that this update doesn’t just change the domain from status.im to status.app, but also includes the update to the new URL format that includes metadata in the URL, pub key after a # at the end, etc…?

If this has all been done, can you test it with the status.app screens that are currently behind basic authentication? If you have any questions about the new URL format, @felicio (from the web team) is the person to speak to.

@John-44
Copy link

John-44 commented Aug 15, 2023

What's remaining is updating https://github.com/status-im/universal-links-handler, will do it as well this week.

@alwx I’m also not sure this needs to be updated, if all it does is serve up the webpages for people who don’t have Status app (and the URL handler) installed, then this is deprecated and replaced by what the web team have already built (and therefore doesn’t need to be updated).

Before updating this, I suggest both speaking to Pavel and @felicio in the web team, and testing the new format links that are being generated with what they have built.

@alwx
Copy link
Contributor

alwx commented Aug 16, 2023

@John-44 it literally updates to status.app everywhere so that status.im is no longer used throughout the app. I guess that's exactly what's needed.

@jrainville
Copy link
Member

@alwx there is more to it.

The new link uses the new status.app domain, but it also uses a new format/API. As you saw, it no longer just passes the ID in the link, instead it is encoded data, that contains the community name and member count, for example.

Here are the Desktop PRs that implement this change to use the new status-go APIs:
status-im/status-desktop#11436
status-im/status-desktop#11619

Feel free to contact @MishkaRogachev or @borismelnik if you have questions as to how the APIs work.

@jakubgs
Copy link
Member

jakubgs commented Aug 17, 2023

But it still means that the link does nothing for someone that doesn't have the app installed, or has the link handling not configured correctly on their system, right? Is that what we want?

@John-44
Copy link

John-44 commented Aug 17, 2023

But it still means that the link does nothing for someone that doesn't have the app installed, or has the link handling not configured correctly on their system, right? Is that what we want?

The link will work for somebody who doesn't have the app installed! Remember that Status Mobile is currently pre-Alpha, and we aren't recommending that anybody external runs it yet. By the time that Status Mobile has it's first public Alpha release, the new status.app website will be live with the basic authentication that is currently preventing public access removed.

In fact, the reason why this is high priority for the mobile team is because updating Status Mobile to use the new link format is a blocker on releasing a public Alpha of Status Mobile. When a user uses the first public Status Mobile alpha release, we want the links that point to: Status Users, Status Communities, and Channels inside a Status Community to be in the new format.

So we need to make this change now, and the fact that these pages are currently behind basic authentication shouldn't block anything, because any Status CC can ping the web team to get the credentials.

@John-44
Copy link

John-44 commented Aug 17, 2023

@John-44 it literally updates to status.app everywhere so that status.im is no longer used throughout the app. I guess that's exactly what's needed.

@alwx no, there is a lot lot more to it than just changing the domain. See my comment from two weeks ago here #16810 (comment)

Also the new URLs that are generated after your PR need to be tested! As mentioned in my message here #16810 (comment) you need to ask the web team for the basic authentication credentials so that you can test that the URLs being generated work correctly with the new web onboarding pages they have built. Testing with the new web pages would have shown that just changing the domain from status.im to status.app doesn't work.

You also really need to organize a call with @felicio from the web team so he can talk you through the new URL format specification. Folks from desktop team can help, but @felicio designed the new URL format so he is the best person to give you a first overview. @felicio has also written a specification somewhere, but I can't find a link to it.

Note that the new 'Status share URL format' applies to three different types of links:

  • Links to Status Users

  • Links to Status Communities

  • Links to a specific channel inside a Status Community.

All of these need to be updated to the new format as part of this work item.

Ensuring that all input fields that accept User chat keys and/or Community pub keys also accept the new format URLs is also part of this work item (as previously mentioned here #16810 (comment) )

Thanks!

@cammellos
Copy link
Contributor

@alwx thanks, we can pick your pr from where's left, it requires some more backend integration since not only the domain is changed but also the format of the url has changed, so we'll need to address those changes as well.
I'll keep you posted and we can wire up the backend, since we'll try to keep the processing there for the most part.

@jakubgs
Copy link
Member

jakubgs commented Aug 17, 2023

If the assumption is that this is a partially non-functional change and need to be completed before actual release that's fine then.

But as Andrea says, the URL schema changed, so this might be more involved than just changing domains.

@alwx
Copy link
Contributor

alwx commented Aug 17, 2023

@cammellos that would be the best because I'm off next week, and it would be weird to block the progress for the whole week. So feel free to pick it up and work on it.

@John-44
Copy link

John-44 commented Aug 17, 2023

If the assumption is that this is a partially non-functional change and need to be completed before actual release that's fine then.

But as Andrea says, the URL schema changed, so this might be more involved than just changing domains.

Yes, this change is much more involved than simply changing the domain

@qoqobolo
Copy link
Contributor Author

Closing as a dupe of #15325

@ilmotta ilmotta removed the bug label Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature feature requests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants