Migrate from deprecated uni_links to app_links package #2708
Conversation
WalkthroughThis pull request focuses on migrating the deep linking functionality from the discontinued Changes
Sequence DiagramsequenceDiagram
participant App
participant AppLinks
participant URIHandler
App->>AppLinks: Create AppLinks instance
App->>AppLinks: Get initial link
AppLinks-->>App: Return initial URI
App->>URIHandler: Process initial URI
App->>AppLinks: Listen to URI link stream
AppLinks-->>App: Notify of new URIs
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
Other🎯 Please be considerate of our volunteers' time. Contacting the person who assigned the reviewers is not advised unless they ask for your input. Do not @ the person who did the assignment otherwise. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
test/widget_tests/pre_auth_screens/splash_screen_test.dart (1)
Line range hint
231-310: Restructure test cases for better organization.The test cases should be:
- Organized within descriptive test groups
- Have clear test names
- Use proper setUp and tearDown
Would you like me to provide a complete restructured version of these test cases?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
lib/splash_screen.dart(3 hunks)pubspec.yaml(2 hunks)test/widget_tests/pre_auth_screens/splash_screen_test.dart(4 hunks)
🧰 Additional context used
🪛 GitHub Actions: PR Workflow
pubspec.yaml
[error] Unauthorized modification or deletion of sensitive file. Apply 'ignore-sensitive-files-pr' label to override.
🔇 Additional comments (6)
pubspec.yaml (3)
13-13: LGTM: SDK version update is reasonable.The SDK version update to ">=2.17.0 <=3.6.0" allows for newer Dart versions while maintaining backward compatibility.
🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Unauthorized modification or deletion of sensitive file. Apply 'ignore-sensitive-files-pr' label to override.
21-22: LGTM: Core package migration looks good.The replacement of uni_links with app_links and its platform interface is aligned with the PR objective.
🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Unauthorized modification or deletion of sensitive file. Apply 'ignore-sensitive-files-pr' label to override.
Line range hint
1-120: Address pipeline failure for sensitive file modification.The pipeline indicates unauthorized modification of a sensitive file. Please request the 'ignore-sensitive-files-pr' label from repository maintainers to proceed.
✅ Verification successful
Sensitive file modification confirmed - 'ignore-sensitive-files-pr' label required
The verification confirms that pubspec.yaml is indeed a protected sensitive file. The pipeline correctly enforces this protection. Request the 'ignore-sensitive-files-pr' label from repository maintainers to proceed with the changes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if pubspec.yaml is marked as sensitive gh api repos/:owner/:repo/contents/.github/sensitive-files.txt --raw | grep -q "pubspec.yaml" && echo "pubspec.yaml is marked as sensitive"Length of output: 1457
Script:
#!/bin/bash # Check multiple potential locations for sensitive files configuration echo "Checking .github directory for sensitive files configuration..." fd -t f "sensitive.*\.(txt|ya?ml|json)$" .github/ echo -e "\nSearching for files containing 'pubspec.yaml' as sensitive..." rg -l "pubspec\.ya?ml" .github/ || true echo -e "\nChecking workflow files for sensitive file patterns..." fd -e yml -e yaml . .github/workflows/ --exec cat {} \; | rg "sensitive.*files?" -C 2 || trueLength of output: 1774
Script:
#!/bin/bash # Extract the sensitive files check implementation echo "Checking pull-request.yml for sensitive files definition..." rg -A 10 "Check-Sensitive-Files" .github/workflows/pull-request.ymlLength of output: 664
Script:
#!/bin/bash # Get the complete workflow file content echo "Getting complete pull-request.yml content..." cat .github/workflows/pull-request.ymlLength of output: 11994
🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Unauthorized modification or deletion of sensitive file. Apply 'ignore-sensitive-files-pr' label to override.
lib/splash_screen.dart (3)
46-47: LGTM: Clean initialization of AppLinks.The AppLinks instance is properly initialized at the start of the method.
Line range hint
48-65: LGTM: Stream subscription handling looks good.The implementation correctly:
- Subscribes to URI link stream
- Handles mounted state
- Updates state appropriately
- Handles errors
Line range hint
66-75: LGTM: Initial link handling is properly implemented.The migration maintains the same error handling and state management while using the new app_links API.
| video_player_android: ^2.7.16 | ||
| visibility_detector: ^0.4.0+2 | ||
| win32: ^5.10.0 |
There was a problem hiding this comment.
Remove unrelated package additions.
The following packages seem unrelated to the uni_links to app_links migration:
- video_player_android
- win32
Please remove these additions or create a separate PR for them.
🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Unauthorized modification or deletion of sensitive file. Apply 'ignore-sensitive-files-pr' label to override.
| @@ -267,7 +267,7 @@ Future<void> testWidgets3(WidgetTester tester) async { | |||
|
|
|||
| // Mock the uriLinkStream to emit a test URI | |||
| final testUri = Uri.parse("https://example.com"); | |||
| uriLinkStream.any(testUri as bool Function(Uri? element)); | |||
| appLinks.uriLinkStream.any(testUri as bool Function(Uri? element)); | |||
There was a problem hiding this comment.
Fix stream and future mocking in tests.
The tests incorrectly use the real AppLinks instance. Mock the dependencies and their responses.
-appLinks.uriLinkStream.any(testUri as bool Function(Uri? element));
+when(mockAppLinks.uriLinkStream)
+ .thenAnswer((_) => Stream.fromIterable([testUri]));
-when(appLinks.getInitialLink()).thenAnswer((_) async => testUri);
+when(mockAppLinks.getInitialLink())
+ .thenAnswer((_) async => testUri);Also applies to: 302-302
| final appLinks = AppLinks(); | ||
| Future<void> testWidgets1(WidgetTester tester) async { | ||
| await tester.pumpWidget(createSplashScreenLight()); | ||
| await tester.pumpAndSettle(); | ||
|
|
||
| // Mock the uriLinkStream to emit a test URI | ||
| final testUri = Uri.parse("https://example.com"); | ||
| uriLinkStream.any(testUri as bool Function(Uri? element)); | ||
| appLinks.uriLinkStream.any(testUri as bool Function(Uri? element)); |
There was a problem hiding this comment.
Fix incorrect stream testing implementation.
The current implementation has several issues:
appLinksshould be mockedStream.any()is incorrectly used with a URI directly- Test is not properly structured within a test group
-final appLinks = AppLinks();
+// Mock AppLinks
+final mockAppLinks = MockAppLinks();
+
+group('URI Link Handling', () {
+ setUp(() {
+ when(mockAppLinks.uriLinkStream)
+ .thenAnswer((_) => Stream.fromIterable([Uri.parse("https://example.com")]));
+ });
+
+ testWidgets('handles incoming URI correctly', (tester) async {
// ... test implementation
+ });
+});Committable suggestion skipped: line range outside the PR's diff.
What kind of change does this PR introduce?
Issue Number:
uni_linkstoapp_linksdue to Discontinuation #2706Did you add tests for your changes?
Yes
Tests are written for all changes made in this PR.
Test coverage meets or exceeds the current coverage (~90/95%).
Summary
Does this PR introduce a breaking change?
Checklist for Repository Standards
coderaabbitaireview suggestions?Have you read the contributing guide?
Summary by CodeRabbit
Dependencies
uni_linkspackage withapp_linksvideo_player_androidandwin32Deep Link Handling
app_linkspackageTesting