TW-2947: Handle invitation link web page and routing#2948
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughA new route handler and corresponding page component were introduced to support an invitation link feature. The route configuration maps the path /chat/:matrixId to a new InvitationLinkWeb page, extracting the matrixId parameter from the URL. The InvitationLinkWeb widget is a stateful component that accepts an optional matrixId, launches a URL after the first frame renders (or navigates to /rooms if matrixId is missing), and displays a centered loading indicator while launching. Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
lib/pages/invitation_link_web/invitation_link_web.dart (1)
28-29: Minor:animating: trueis the default value.This is redundant and can be removed for cleaner code.
Suggested fix
child: CupertinoActivityIndicator( - animating: true, color: LinagoraSysColors.material().onSurfaceVariant, ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/pages/invitation_link_web/invitation_link_web.dart` around lines 28 - 29, The CupertinoActivityIndicator in invitation_link_web.dart explicitly sets animating: true which is redundant because true is the default; remove the animating parameter from the CupertinoActivityIndicator instantiation (locate the CupertinoActivityIndicator widget in the InvitationLinkWeb widget / build method) so the code relies on the default behavior and is cleaner.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/config/go_routes/go_router.dart`:
- Around line 123-132: The GoRoute for path '/chat/:matrixId' is missing an
authentication redirect guard causing unauthenticated users to reach
InvitationLinkWeb and UrlLauncher.openMatrixToUrl() which call matrix.client;
update the GoRoute definition (the GoRoute with path '/chat/:matrixId' that
builds InvitationLinkWeb) to include redirect: loggedOutRedirect so
unauthenticated users are redirected consistently like other routes.
In `@lib/pages/invitation_link_web/invitation_link_web.dart`:
- Around line 17-22: initState currently calls UrlLauncher(context, url:
widget.matrixId).launchUrl() without null-safety or awaiting its result, which
can throw (url! assertion) or leave the loading UI stuck; update initState to
null-check widget.matrixId and provide a fallback navigation (e.g.,
Navigator.pushReplacementNamed to '/rooms' or '/home') if null, and change the
call to await UrlLauncher(...).launchUrl() (or handle its Future with
then/catchError) so you can navigate away on both success and failure; ensure
you reference UrlLauncher and its launchUrl() completion to trigger a
Navigator.replace/pop to dismiss the loading screen in all early-return
branches.
---
Nitpick comments:
In `@lib/pages/invitation_link_web/invitation_link_web.dart`:
- Around line 28-29: The CupertinoActivityIndicator in invitation_link_web.dart
explicitly sets animating: true which is redundant because true is the default;
remove the animating parameter from the CupertinoActivityIndicator instantiation
(locate the CupertinoActivityIndicator widget in the InvitationLinkWeb widget /
build method) so the code relies on the default behavior and is cleaner.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 02ff494d-1309-4ec4-b9d5-66f3154a0d07
📒 Files selected for processing (2)
lib/config/go_routes/go_router.dartlib/pages/invitation_link_web/invitation_link_web.dart
|
This PR has been deployed to https://linagora.github.io/twake-on-matrix/2948 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/utils/url_launcher.dart (1)
34-39:⚠️ Potential issue | 🟡 MinorDon't force invitation routing for every
@userlink.
launchUrl()drops the caller'sisInvitationLinkvalue in the early Matrix branch, andopenMatrixToUrl()then hardcodestruefor all@userlinks. SinceGoToDraftChatMixin.goToDraftChat()uses that flag to redirect self-chats to/rooms, non-invitation self links will now take the invitation fallback too.🧭 Proposed fix
void launchUrl({bool isInvitationLink = false}) { if (url!.toLowerCase().startsWith(AppConfig.deepLinkPrefix) || url!.toLowerCase().startsWith(AppConfig.inviteLinkPrefix) || {'#', '@', '!', '+', '\$'}.contains(url![0]) || url!.toLowerCase().startsWith(AppConfig.schemePrefix)) { - return openMatrixToUrl(); + return openMatrixToUrl(isInvitationLink: isInvitationLink); } @@ onContactTap( context: context, path: 'rooms', - isInvitationLink: true, + isInvitationLink: isInvitationLink, contactPresentationSearch: ContactPresentationSearch(Also applies to: 117-120, 224-227
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/utils/url_launcher.dart` around lines 34 - 39, The Matrix branch in launchUrl() discards the caller's isInvitationLink and calls openMatrixToUrl() which then hardcodes invitation=true for `@user` links; update openMatrixToUrl to accept a bool isInvitationLink parameter and thread the original isInvitationLink through each place that calls openMatrixToUrl (including the other occurrences referenced) so the invite flag is preserved, and remove the hardcoded true inside openMatrixToUrl/goToDraftChat paths so routing uses the passed-in flag.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@lib/utils/url_launcher.dart`:
- Around line 34-39: The Matrix branch in launchUrl() discards the caller's
isInvitationLink and calls openMatrixToUrl() which then hardcodes
invitation=true for `@user` links; update openMatrixToUrl to accept a bool
isInvitationLink parameter and thread the original isInvitationLink through each
place that calls openMatrixToUrl (including the other occurrences referenced) so
the invite flag is preserved, and remove the hardcoded true inside
openMatrixToUrl/goToDraftChat paths so routing uses the passed-in flag.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 43c1d5e8-9031-444d-994e-f4b319298ae0
📒 Files selected for processing (5)
lib/config/go_routes/app_route_paths.dartlib/config/go_routes/go_router.dartlib/pages/invitation_link_web/invitation_link_web.dartlib/presentation/mixins/go_to_direct_chat_mixin.dartlib/utils/url_launcher.dart
✅ Files skipped from review due to trivial changes (2)
- lib/config/go_routes/app_route_paths.dart
- lib/pages/invitation_link_web/invitation_link_web.dart
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/config/go_routes/go_router.dart
Ticket
Related issue
Resolved
Attach screenshots or videos demonstrating the changes
demo.mov
Summary by CodeRabbit
New Features
Bug Fixes