Skip to content

refactor: replace the app's links module with the one in ui-client - #43070

Open
tassoevan wants to merge 3 commits into
developfrom
chore/dedupe-links
Open

tassoevan wants to merge 3 commits into
developfrom
chore/dedupe-links

Conversation

@tassoevan

@tassoevan tassoevan commented Oct 23, 2026 •

Copy link
Copy Markdown
Member

Proposed changes (including videos or screenshots)

apps/meteor/client/lib/links.ts and packages/ui-client/src/lib/links.ts were two copies of the same table of go.rocket.chat URLs, and they had drifted: the app copy gained abacDocs, abacLDAPDocs and logsDocs after the copy into ui-client (#37757).

  • Adds the three missing keys to ui-client's links.
  • Deletes the app copy; its 46 importers now take links (or GO_ROCKET_CHAT_PREFIX) from @rocket.chat/ui-client, merged into an existing ui-client import where the file had one.
  • tests/e2e/calendar.spec.ts used links.rocketChat only as a sample meeting URL. It now uses the literal 'https://rocket.chat/', so Playwright doesn't load the ui-client barrel.

No behavior change, so no changeset.

Issue(s)

ARCH-2591

Steps to test or reproduce

Nothing to test manually: the URLs are unchanged. Typecheck, lint and the unit specs of the touched components cover it.

Further comments

This is the first of a few enabling refactors for moving views out of apps/meteor/client/views into @rocket.chat/ui-* packages. A package can't import from the app, and links is one of the app modules those views import the most.

🤖 Generated with Claude Code

View guided diff

Summary by CodeRabbit

  • New Features
    • Added direct links to documentation for ABAC, LDAP integration with ABAC, and logs.
  • Updates
    • Help, product information, download, and external-resource links continue to use their existing destinations. Navigation across workspace setup, administration, omnichannel, and rooms remains unchanged, preserving familiar workflows.

@tassoevan
tassoevan requested a review from a team as a code owner October 23, 2026 15:28
@tassoevan tassoevan added this to the 9.0.0 milestone Oct 23, 2026
@dionisio-bot

dionisio-bot Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Looks like this PR is ready to merge! 🎉
If you have any trouble, please check the PR guidelines

@changeset-bot

changeset-bot Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: eaee8be

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a6c27f30-a811-42d4-9e85-878e3a456601
Commits

Reviewing files that changed from the base of the PR and between 468526d and eaee8be.

Files ignored due to path filters: 1
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
Files selected for processing: 5
  • apps/meteor/client/components/GenericUpsellModal/hooks/useUpsellActions.ts
  • apps/meteor/client/views/admin/ABAC/ABACSettingTab/SettingsPage.tsx
  • apps/meteor/client/views/admin/customUserStatus/CustomUserStatusService.tsx
  • apps/meteor/client/views/omnichannel/contactInfo/AdvancedContactModal.tsx
  • packages/ui-setup-wizard/package.json

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Recent review details
Context from checks skipped due to timeout: 8
  • GitHub Check: 🔨 Test Storybook / Test Storybook
  • GitHub Check: 🔎 Code Check / Code Lint
  • GitHub Check: 🔨 Test Unit / Unit Tests
  • GitHub Check: 🔨 Test Visual Regression / Detect Fuselage Changes
  • GitHub Check: 🔎 Code Check / TypeScript
  • GitHub Check: 📦 Meteor Build (coverage)
  • GitHub Check: CodeRabbit
  • GitHub Check: CodeQL-Build
Additional comments: 5
packages/ui-setup-wizard/package.json: 1

38-38: LGTM!

Also applies to: 49-49

apps/meteor/client/components/GenericUpsellModal/hooks/useUpsellActions.ts: 1

1-1: LGTM!

apps/meteor/client/views/admin/ABAC/ABACSettingTab/SettingsPage.tsx: 1

2-2: LGTM!

apps/meteor/client/views/admin/customUserStatus/CustomUserStatusService.tsx: 1

14-14: LGTM!

apps/meteor/client/views/omnichannel/contactInfo/AdvancedContactModal.tsx: 1

1-1: LGTM!

Walkthrough

Client code now imports link constants from @rocket.chat/ui-client instead of the deleted local links module. The shared link catalog adds three documentation URLs. The calendar test uses a literal meeting URL.

Changes

Client link catalog migration

Layer / File(s) Summary
Update the shared link catalog
packages/ui-client/src/lib/links.ts, apps/meteor/client/lib/links.ts
The shared links.go object adds URLs for ABAC, ABAC LDAP, and logs documentation. The local links module is deleted.
Migrate client link consumers
apps/meteor/client/components/*, apps/meteor/client/lib/*, apps/meteor/client/sidebar/*, apps/meteor/client/views/admin/*, apps/meteor/client/views/e2e/*, apps/meteor/client/views/home/cards/*, apps/meteor/client/views/marketplace/*, apps/meteor/client/views/omnichannel/*, apps/meteor/client/views/room/*, apps/meteor/tests/e2e/calendar.spec.ts
Client call sites now import link constants from @rocket.chat/ui-client. The calendar test uses https://rocket.chat/ as its meeting URL. The E2E test mock includes the actual package exports alongside its mocked imperativeModal.
Adjust Storybook version ranges
packages/ui-setup-wizard/package.json
The @storybook/react and storybook dev dependency ranges change from ^9.1.21 to ~9.1.21.

Priority: Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Suggested labels: type: chore, area: authentication

Suggested reviewers: ggazzo

Merge Risk: Minimal · up to eaee8

The shared link migration is internally consistent, with no established behavior or deployment risk requiring resolution before merge.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: replacing the app-level links module with the implementation from @rocket.chat/ui-client.
Docstring Coverage Passed Docstring coverage is inconclusive: files from earlier reviews still need to be rechecked within the check's analysis limits. Not blocking.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Errors were encountered while retrieving linked issues.

Errors: 1
  • JIRA integration encountered authorization issues. Please disconnect and reconnect the integration in the CodeRabbit UI.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 49 files

Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic

Comment thread apps/meteor/client/lib/links.ts
@codecov

codecov Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.26%. Comparing base (7a174fd) to head (eaee8be).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #43070      +/-   ##
===========================================
- Coverage    72.28%   72.26%   -0.02%     
===========================================
  Files         5387     5386       -1     
  Lines       231781   231715      -66     
  Branches     44478    44404      -74     
===========================================
- Hits        167538   167450      -88     
- Misses       59708    59738      +30     
+ Partials      4535     4527       -8     
Flag Coverage Δ
e2e 61.91% <ø> (-0.04%) ⬇️
e2e-api 49.23% <ø> (+0.29%) ⬆️
unit 72.16% <100.00%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@rc-layne

rc-layne Bot commented Oct 23, 2026

Copy link
Copy Markdown

⚠️ Layne — scan incomplete

Layne could not analyze all changed content. Review the Check Run summary before merging.

dougfabris
dougfabris previously approved these changes Oct 28, 2026
@tassoevan tassoevan added the stat: QA assured Means it has been tested and approved by a company insider label Oct 29, 2026
@tassoevan tassoevan modified the milestones: 9.0.0, 9.1.0 Oct 29, 2026
@dionisio-bot dionisio-bot Bot added the stat: ready to merge PR tested and approved waiting for merge label Oct 29, 2026
@dionisio-bot
dionisio-bot Bot added this pull request to the merge queue Oct 29, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Oct 29, 2026
tassoevan and others added 3 commits October 29, 2026 00:15

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: authentication stat: QA assured Means it has been tested and approved by a company insider stat: ready to merge PR tested and approved waiting for merge type: chore

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants