chore: Recolocate Meteor-related client-side modules under client/meteor
#36895
Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #36895 +/- ##
===========================================
- Coverage 66.26% 66.26% -0.01%
===========================================
Files 3339 3339
Lines 113726 113725 -1
Branches 21166 21173 +7
===========================================
- Hits 75363 75357 -6
Misses 35674 35674
- Partials 2689 2694 +5
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
tassoevan
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
WalkthroughRefactors update multiple import paths across the Meteor client, consolidating modules under apps/meteor/client/meteor/overrides and apps/meteor/client/meteor/watch. One re-export (watch) is removed from cachedStores. No logic, control flow, or public API signatures change beyond that export removal. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing Touches
🧪 Generate unit tests
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/meteor/client/meteor/overrides/login/meteorDeveloperAccount.ts (1)
20-21: Build break: Random is used but not importedAdd the missing import.
Apply:
+import { Random } from '@rocket.chat/random';
🧹 Nitpick comments (4)
apps/meteor/client/meteor/overrides/ddpOverREST.ts (1)
5-5: SDKClient path realigned; consider path aliases to avoid brittle ../../../Optional: add TS path aliases to make future relocations safer.
Example tsconfig paths:
{ "compilerOptions": { "baseUrl": "apps/meteor", "paths": { "@app/*": ["app/*"], "@client/*": ["client/*"] } } }apps/meteor/client/meteor/overrides/login/google.ts (1)
8-9: Import depths updated correctly post-relocation.Paths to
overrideLoginMethodandwrapRequestCredentialFnlook right from overrides/login/.To reduce future churn from deep relatives, consider TS path aliases (e.g.,
@client/lib/...) in a follow-up.apps/meteor/client/meteor/overrides/totpOnCall.ts (2)
48-60: Fix double-wrapped return type in async wrapper.The inner function is declared as Promise<ReturnType>, but ReturnType is already a Promise<…>. This yields Promise<Promise<…>> at the type level. Adjust to ReturnType.
- return async function callAsyncWithTOTP(methodName: string, ...args: unknown[]): Promise<ReturnType<T>> { + return async function callAsyncWithTOTP(methodName: string, ...args: unknown[]): ReturnType<T> {
55-56: Prefer original callAsync to avoid re-wrapping via the global.Using the captured callAsync keeps the retry path on the original impl and avoids unnecessary wrapper re-entry.
- onCode: (twoFactorCode, twoFactorMethod) => Meteor.callAsync(methodName, ...args, { twoFactorCode, twoFactorMethod }), + onCode: (twoFactorCode, twoFactorMethod) => callAsync(methodName, ...args, { twoFactorCode, twoFactorMethod }),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (24)
apps/meteor/app/authorization/client/hasPermission.ts(1 hunks)apps/meteor/app/authorization/client/hasRole.ts(1 hunks)apps/meteor/client/lib/cachedStores/CachedStore.ts(1 hunks)apps/meteor/client/lib/cachedStores/index.ts(0 hunks)apps/meteor/client/lib/customOAuth/CustomOAuth.ts(1 hunks)apps/meteor/client/lib/settings/settings.ts(1 hunks)apps/meteor/client/main.ts(1 hunks)apps/meteor/client/meteor/overrides/ddpOverREST.ts(1 hunks)apps/meteor/client/meteor/overrides/login/cas.ts(2 hunks)apps/meteor/client/meteor/overrides/login/crowd.ts(1 hunks)apps/meteor/client/meteor/overrides/login/facebook.ts(1 hunks)apps/meteor/client/meteor/overrides/login/github.ts(1 hunks)apps/meteor/client/meteor/overrides/login/google.ts(1 hunks)apps/meteor/client/meteor/overrides/login/ldap.ts(1 hunks)apps/meteor/client/meteor/overrides/login/meteorDeveloperAccount.ts(1 hunks)apps/meteor/client/meteor/overrides/login/oauth.ts(2 hunks)apps/meteor/client/meteor/overrides/login/password.ts(1 hunks)apps/meteor/client/meteor/overrides/login/saml.ts(2 hunks)apps/meteor/client/meteor/overrides/login/twitter.ts(1 hunks)apps/meteor/client/meteor/overrides/settings.ts(1 hunks)apps/meteor/client/meteor/overrides/totpOnCall.ts(1 hunks)apps/meteor/client/meteor/overrides/unstoreLoginToken.ts(1 hunks)apps/meteor/client/meteor/overrides/userAndUsers.ts(1 hunks)apps/meteor/client/startup/accounts.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/meteor/client/lib/cachedStores/index.ts
🧰 Additional context used
🧬 Code graph analysis (3)
apps/meteor/app/authorization/client/hasRole.ts (1)
apps/meteor/app/lib/server/lib/notifyListener.ts (2)
T(136-138)NotifyUserChange(372-384)
apps/meteor/client/meteor/overrides/userAndUsers.ts (2)
apps/meteor/app/lib/server/lib/notifyListener.ts (2)
NotifyUserChange(372-384)id(405-412)apps/meteor/app/lib/server/functions/setUserActiveStatus.ts (2)
u(38-38)setUserActiveStatus(53-176)
apps/meteor/app/authorization/client/hasPermission.ts (1)
apps/meteor/app/lib/server/lib/notifyListener.ts (5)
permission(100-110)T(287-293)pid(114-121)NotifyUserChange(372-384)T(136-138)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: 🔨 Test Storybook / Test Storybook
- GitHub Check: 🔎 Code Check / Code Lint
- GitHub Check: 🔎 Code Check / TypeScript
- GitHub Check: 🔨 Test Unit / Unit Tests
- GitHub Check: 📦 Meteor Build - coverage
- GitHub Check: Builds matrix rust bindings against alpine
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (22)
apps/meteor/client/lib/settings/settings.ts (1)
3-3: LGTM: import now points to the centralized Meteor watch modulePath looks correct for
apps/meteor/client/lib/settings→../../meteor/watch. No behavior change.apps/meteor/client/meteor/overrides/unstoreLoginToken.ts (1)
3-3: LGTM: updated CachedStoresManager import pathRelocation aligns with the new directory layout; override behavior remains unchanged.
apps/meteor/client/meteor/overrides/userAndUsers.ts (1)
4-4: LGTM: Users store import path adjustedCorrect relative path after move; no functional impact.
apps/meteor/client/meteor/overrides/login/twitter.ts (1)
8-9: Verified consistent import depth
All login override modules under apps/meteor/client/meteor/overrides/login import overrideLoginMethod and wrapRequestCredentialFn from '../../../lib/...'; no stale '../../lib/…' paths remain.apps/meteor/client/meteor/overrides/settings.ts (1)
3-3: LGTM: PublicSettings import path updatedMatches the new colocated stores location; logic untouched.
apps/meteor/client/lib/cachedStores/CachedStore.ts (1)
16-16: LGTM: watch import redirected to client/meteorKeeps reactive readiness wiring intact while isolating Meteor-specific watch logic.
apps/meteor/client/meteor/overrides/login/facebook.ts (1)
8-9: LGTM: path depth corrected for shared lib importsConsistent with twitter override; no functional differences.
apps/meteor/app/authorization/client/hasPermission.ts (1)
6-6: Watch import migration verified
No imports ofwatchremain undercachedStoresand no re-exports inapps/meteor/client/lib/cachedStores/index.tswere found.apps/meteor/client/startup/accounts.ts (1)
10-10: LGTM:watchfully relocated
No imports fromcachedStoresdetected;apps/meteor/client/meteor/watch.tsexists.apps/meteor/client/meteor/overrides/login/crowd.ts (1)
3-3: LGTM: 2FA override import path updatedConsistent with other login overrides.
apps/meteor/client/meteor/overrides/login/oauth.ts (2)
5-6: LGTM: type-only imports now point to client/definitions and client/libNo behavior change; good use of import type.
116-116: Approve dynamic import forprocess2faReturn
File verified atapps/meteor/client/lib/2fa/process2faReturn.ts; no further changes needed.apps/meteor/client/lib/customOAuth/CustomOAuth.ts (1)
10-10: LGTM: createOAuthTotpLoginMethod import switched to meteor/overridesKeeps responsibilities clear between lib and meteor overrides.
apps/meteor/client/meteor/overrides/login/github.ts (1)
8-10: LGTM: lib imports moved one level deeperMatches repo layout changes; no functional diffs.
apps/meteor/client/main.ts (1)
6-7: Approve changes: overrides entry relocated and cleanup confirmed
Confirmedapps/meteor/client/meteor/overrides/index.tsexists and the legacymeteorOverridesdirectory has been removed.apps/meteor/client/meteor/overrides/login/meteorDeveloperAccount.ts (1)
6-7: LGTM: lib imports depth adjustedConsistent with the rest of the login overrides.
apps/meteor/app/authorization/client/hasRole.ts (1)
3-3: Approve import relocation
No remaining imports ofwatchfromclient/lib/cachedStores; newapps/meteor/client/meteor/watch.tsexportswatchas expected.apps/meteor/client/meteor/overrides/login/password.ts (1)
4-4: Import path adjustment is correct.Matches the new directory layout; no behavior change.
apps/meteor/client/meteor/overrides/login/ldap.ts (1)
3-3: Import relocation LGTM.
overrideLoginMethodsymbols now resolve from the correct path.apps/meteor/client/meteor/overrides/login/cas.ts (1)
4-4: All imports verified
Confirmed thatapps/meteor/client/lib/openCASLoginPopup.tsexists and exportsopenCASLoginPopupas a named export.apps/meteor/client/meteor/overrides/login/saml.ts (1)
5-6: Approve imports and exports—verified
Bothsettingsin apps/meteor/client/lib/settings/settings.ts and the named exportsdkin apps/meteor/app/utils/client/lib/SDKClient.ts are present as expected.apps/meteor/client/meteor/overrides/totpOnCall.ts (1)
3-6: Import paths verified and correct. All referenced modules exist at their new locations, so no further changes needed.
Proposed changes (including videos or screenshots)
It colocates code related to Meteor APIs under
client/meteor.Issue(s)
Steps to test or reproduce
Further comments
This PR is part of our “demeteorization” initiative.
It's a slice of #36858.
Summary by CodeRabbit