[PF-248] [3 of 5] Move URL and universal link handling from AppDelegate to SceneDelegate - #2992
Open
stevestreza-ksr wants to merge 6 commits into
Conversation
stevestreza-ksr
added this pull request to stack #2991
September 11, 2026 20:00
There was a problem hiding this comment.
🟡 Changes recommended
One or more issues must be addressed before approval.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Moves URL, universal-link, email redirect, and email verification handling from AppDelegate to SceneDelegate, using the shared deep-link router.
Changes:
- Adds SceneDelegate URL and user-activity handling, including cold-launch inputs.
- Moves URL-driven routing and verification logic into
SceneDelegateViewModel. - Updates tests and preserves AppDelegate push, Braze, and shortcut handling.
File summaries
| File | Description |
|---|---|
| Kickstarter-iOS/SceneDelegate.swift | Updated as part of this pull request. |
| Kickstarter-iOS/AppDelegate.swift | Updated as part of this pull request. |
| Kickstarter-Framework/Sources/Kickstarter-Framework/Kickstarter-iOS/SceneDelegateViewModel.swift | Updated as part of this pull request. |
| Kickstarter-Framework/Sources/Kickstarter-Framework/Kickstarter-iOS/AppDelegateViewModel.swift | Updated as part of this pull request. |
| Kickstarter-Framework/Sources/Kickstarter-Framework-iOSTests/Kickstarter-iOS/SceneDelegateViewModelTests.swift | Updated as part of this pull request. |
| Kickstarter-Framework/Sources/Kickstarter-Framework-iOSTests/Kickstarter-iOS/AppDelegateViewModelTests.swift | Updated as part of this pull request. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
stevestreza-ksr
force-pushed
the
stevestreza/scene-delegate/url-deep-links
branch
from
September 14, 2026 22:08
875697a to
6b1fd27
Compare
Collaborator
Generated by 🚫 Danger |
stevestreza-ksr
force-pushed
the
stevestreza/scene-delegate/url-deep-links
branch
3 times, most recently
from
September 15, 2026 21:11
5b62e72 to
76f953c
Compare
Moves everything that resolves from an opened URL or a continued user activity: application(_:open:options:), application(_:continue: restorationHandler:), the URLSessionTaskDelegate conformance used to resolve email-click redirects, and the cold-launch handling of connectionOptions.urlContexts / .userActivities in scene(_:willConnectTo:options:). SceneDelegateViewModel gains the inputs, outputs, and helpers that were purely about resolving a URL — applicationOpenUrl, applicationContinueUserActivity, foundRedirectUrl, continueUserActivityReturnValue, emailVerificationCompleted, findRedirectUrl, accessTokenFromUrl, emailVerificationCompletionData — along with the router-based navigation outputs (goToActivity, goToDiscovery, goToLoginWithIntent, goToMessageThread, goToMobileSafari, goToProfile, goToSearch, presentViewController, updateCurrentUserInEnvironment) that a resolved deep link produces, via the same DeepLinkNavigationRouter AppDelegateViewModel already uses. SceneDelegate wires all of these to the same UI actions AppDelegate used to perform. SceneDelegateViewModelTests carries the corresponding coverage, moved over test for test. AppDelegateViewModel drops the same inputs, outputs, and helpers. goToMobileSafari narrows to Braze's urlFromBraze now that the URL side of that merge is gone, and goToProfile is removed outright: URL was its only possible source (push and Braze's Navigation.deepLinkMatch allowlist both exclude /profile/:user_param), so it can no longer ever fire. Shortcut-item handling is untouched and stays on AppDelegate for now — it still shares deepLink with push and Braze, and goToSearch and goToDiscovery remain reachable through it, so their AppDelegate observers stay in place. AppDelegateViewModelTests drops every test that drove behavior through applicationOpenUrl or applicationContinueUserActivity — that coverage now lives on SceneDelegateViewModelTests. It keeps the shortcut tests and everything push/Braze-driven, and adds testOpenNotification_Follow_GoesToActivity: goToActivity's only remaining test coverage was through a URL, even though a push notification's .follow category also routes to it, so that path had no test of its own. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
stevestreza-ksr
force-pushed
the
stevestreza/scene-delegate/url-deep-links
branch
from
September 15, 2026 22:51
fc174c7 to
8272871
Compare
stevestreza-ksr
marked this pull request as ready for review
September 16, 2026 07:32
scottkicks
approved these changes
Sep 16, 2026
scottkicks
left a comment
Contributor
There was a problem hiding this comment.
lgtm
ngl these claude generated summaries make my head hurt sometimes 😅
amy-at-kickstarter
requested changes
Sep 16, 2026
amy-at-kickstarter
left a comment
Contributor
There was a problem hiding this comment.
It looks like not all of the tests were moved/copied over to DeepLinkNavigationRouterTests. We should preserve tests for the full range of deeplinks that are tested here.
|
|
||
| func testOpenNotification_UnrecognizedActivityType() { | ||
| let categories: [Activity.Category] = [.follow, .funding, .unknown, .watch] | ||
| let categories: [Activity.Category] = [.funding, .unknown, .watch] |
Contributor
There was a problem hiding this comment.
Was deleting .follow as a type intentional?
| } | ||
| } | ||
|
|
||
| func testPresentViewController_ProjectPreviewLink_DisplayPrelaunch_Nil() { |
Contributor
There was a problem hiding this comment.
I checked, and many of the tests that are deleted in this PR weren't moved into DeepLinkNavigationRouterTests in PR #2989.
Can we preserve this test coverage in the new test file?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📲 What
This PR migrates key elements of AppDelegate handling of URLs, deep links, and user activities to use SceneDelegate.
🤔 Why
Because we now use SceneDelegate, old routing methods no longer go through AppDelegate. Therefore, that existing functionality must be migrated to SceneDelegate.
🛠 How
Claude was directed to migrate existing behaviors related to link and user activity handling to SceneDelegate, moving code from one implementation to another, including view models and tests.
🤖 Claude-Generated Summary
Moves everything that resolves from an opened URL or a continued user activity: application(:open:options:), application(:continue: restorationHandler:), the URLSessionTaskDelegate conformance used to resolve email-click redirects, and the cold-launch handling of connectionOptions.urlContexts / .userActivities in scene(_:willConnectTo:options:).
SceneDelegateViewModel gains the inputs, outputs, and helpers that were purely about resolving a URL — applicationOpenUrl, applicationContinueUserActivity, foundRedirectUrl, continueUserActivityReturnValue, emailVerificationCompleted, findRedirectUrl, accessTokenFromUrl, emailVerificationCompletionData — along with the router-based navigation outputs (goToActivity, goToDiscovery, goToLoginWithIntent, goToMessageThread, goToMobileSafari, goToProfile, goToSearch, presentViewController,
updateCurrentUserInEnvironment) that a resolved deep link produces, via the same DeepLinkNavigationRouter AppDelegateViewModel already uses. SceneDelegate wires all of these to the same UI actions AppDelegate used to perform. SceneDelegateViewModelTests carries the corresponding coverage, moved over test for test.
AppDelegateViewModel drops the same inputs, outputs, and helpers. goToMobileSafari narrows to Braze's urlFromBraze now that the URL side of that merge is gone, and goToProfile is removed outright: URL was its only possible source (push and Braze's Navigation.deepLinkMatch allowlist both exclude /profile/:user_param), so it can no longer ever fire. Shortcut-item handling is untouched and stays on AppDelegate for now — it still shares deepLink with push and Braze, and goToSearch and goToDiscovery remain reachable through it, so their AppDelegate observers stay in place.
AppDelegateViewModelTests drops every test that drove behavior through applicationOpenUrl or applicationContinueUserActivity — that coverage now lives on SceneDelegateViewModelTests. It keeps the shortcut tests and everything push/Braze-driven, and adds
testOpenNotification_Follow_GoesToActivity: goToActivity's only remaining test coverage was through a URL, even though a push notification's .follow category also routes to it, so that path had no test of its own.