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

fix cannot read data when opening a deeplink #8109

Merged
merged 3 commits into from
Jul 26, 2024
Merged

fix cannot read data when opening a deeplink #8109

merged 3 commits into from
Jul 26, 2024

Conversation

enahum
Copy link
Contributor

@enahum enahum commented Jul 25, 2024

Summary

The initial error was caused by the launch parsing of deep links where the deeplink did not match the patterns for channel or permalink, so in this PR apart of fixing the issue when the deep link is invalid we also handle a deeplink for a server that we do not have registered yet by presenting the server screen an populating with the url of the deep link

Ticket Link

https://mattermost.atlassian.net/browse/MM-58467

Checklist

  • Added or updated unit tests (required for all new features)

Release Note

Fixed deep links that did not match the patterns of channel or permalink and was crashing the app

@enahum enahum added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Jul 25, 2024
Copy link
Contributor

@rahimrahman rahimrahman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have only one extra test case I thought could be useful, but not a blocker.

app/utils/deep_link/index.test.ts Show resolved Hide resolved
@saturninoabril saturninoabril added the E2E iOS tests for PR Run iOS E2E Detox tests label Jul 26, 2024
@saturninoabril saturninoabril requested review from lindy65 and removed request for saturninoabril July 26, 2024 09:43
@saturninoabril
Copy link
Member

Requesting review from Lindy for she was able to repro the issue - mattermost/mattermost#26803 (comment)

For the failed Detox (on build), it's not related here and was happening even from main. Will investigate separately - https://community.mattermost.com/core/pl/t1zi5wwic3fdidzeh51z67gybc

@lindy65 lindy65 added Build Apps for PR Build the mobile app for iOS and Android to test Build App for Android Build the mobile app for Android and removed Build Apps for PR Build the mobile app for iOS and Android to test labels Jul 26, 2024
@lindy65
Copy link

lindy65 commented Jul 26, 2024

@enahum - building apps for the PR is failing

@enahum
Copy link
Contributor Author

enahum commented Jul 26, 2024

/update-branch

@enahum enahum added the Build Apps for PR Build the mobile app for iOS and Android to test label Jul 26, 2024
@enahum
Copy link
Contributor Author

enahum commented Jul 26, 2024

@lindy65 build successful https://community.mattermost.com/core/pl/ermnggnht3rbzbqhauaqxbzo4r

@github-actions github-actions bot removed the E2E iOS tests for PR Run iOS E2E Detox tests label Jul 26, 2024
Copy link

@lindy65 lindy65 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @enahum - I have tested according to repro steps and I am taken to the account creation page in a browser. Creating an account works without issue and I can log into MM mobile browser

@lindy65 lindy65 added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter Build Apps for PR Build the mobile app for iOS and Android to test 3: QA Review Requires review by a QA tester Build App for Android Build the mobile app for Android labels Jul 26, 2024
@enahum enahum merged commit 4048b70 into main Jul 26, 2024
39 checks passed
@enahum enahum deleted the MM-58467 branch July 26, 2024 19:37
@amyblais amyblais added this to the v2.20.0 milestone Jul 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request QA Review Done release-note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants