chore: reorganize backend folder structure — Phase 5 (meteor methods) - #41225
Conversation
|
Looks like this PR is ready to merge! 🎉 |
⏭️ Hacktron Security Check — SkippedReason: This PR exceeds Hacktron's 200-file review cap and will not be scanned. Split the PR into smaller changes for review coverage.
|
|
|
Important Review skippedToo many files! This PR contains 212 files, which is 62 over the limit of 150. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (212)
You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Reviewer guide — what changed beyond moves + import rewritesEverything not listed here is a 1. Method registration moved to a single aggregator (the one semantic change)server/meteor-methods/index.ts now holds all side-effect registration imports, grouped by domain. They were removed from app/lib/server/index.ts and 9 feature aggregators (threads' pure-aggregator Warning This consolidation is exactly where a method can silently deregister — and it happened: the 5b rewrite carried over 33 of 34 registrations, dropping Three registrations deliberately did NOT move to the aggregator (they'd double-register or lose gating):
2. Hand-rewritten proxyquire strings (lint/tsc can't validate these)Mock keys /
3. Test-mirror relocations
4. Placement judgment calls (files the plan's tables didn't list)
5. Docs: Phase 6/7 tables corrected (MIGRATION_PLAN.md)Ten features moving in Phases 6/7 still carry 6. Mechanical but hand-triggered
7. Why some
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #41225 +/- ##
===========================================
- Coverage 69.39% 68.98% -0.42%
===========================================
Files 3680 3755 +75
Lines 145528 147149 +1621
Branches 25853 26275 +422
===========================================
+ Hits 100987 101507 +520
- Misses 40241 41138 +897
- Partials 4300 4504 +204
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
1 issue found across 215 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/meteor/server/meteor-methods/index.ts">
<violation number="1" location="apps/meteor/server/meteor-methods/index.ts:103">
P1: **Missing import for `unblockUser` in the new aggregator (method won't be registered)**
The old import `import './methods/unblockUser'` from `app/lib/server/index.ts` was removed as part of this migration. The file was moved to `server/meteor-methods/users/unblockUser.ts` (per the phase5b manifest), but the corresponding `import './users/unblockUser'` line was not added to `server/meteor-methods/index.ts`. The `blockUser` import is present directly above it, so `unblockUser` appears to have been accidentally skipped.
Without this import, the Meteor method for unblocking users is never registered — calling the `unblockUser` method will fail with a method-not-found error. Add `import './users/unblockUser';` between the `blockUser` and `deleteUser` imports.</violation>
</file>
Note: This PR contains a large number of files. cubic only reviews up to 200 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review.
Re-trigger cubic
a028c8a
a028c8a to
346d7b2
Compare
346d7b2 to
a24220d
Compare
Meteor-specific RPC handlers get an explicit folder name, distinguishing them from REST handlers and service methods, per MIGRATION_PLAN.md Phase 5. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… domain subfolders) Moves the flat files in server/meteor-methods/ into domain subfolders (users/, rooms/, messages/, auth/, settings/, platform/) and regroups index.ts by domain. Mirrored specs follow: deleteFileMessage.spec.ts to the messages/ mirror, banUserFromRoom.spec.ts to tests/unit/server/lib/ (it tests server/lib/banUserFromRoom.ts, not a method). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Moves app/lib/server/methods/* into server/meteor-methods/<domain>/. Method registrations previously aggregated by app/lib/server/index.ts now live in server/meteor-methods/index.ts. Stale proxyquire keys updated in roomConverter, publications/messages and sendFileMessage specs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Moves Meteor methods from authorization, 2fa, channel-settings, threads, discussion, livechat, integrations, importer, autotranslate and e2e into server/meteor-methods/<domain>/. Registrations consolidate into server/meteor-methods/index.ts; feature server/index.ts aggregators drop their method imports. updateGroupKey.spec.ts moves to the meteor-methods platform/ mirror; stale proxyquire keys rewritten. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Renames ee/server/methods → ee/server/meteor-methods and moves EE method files there: canned-responses methods (still registered via the License.onLicense dynamic imports), license methods.ts (as license.ts) and livechat-enterprise removeBusinessHour into omnichannel/. All EE sources stay under ee/ per the license boundary. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… Phase 5a The spec loaded '../../methods/requestDataDownload.ts', a string literal the move script does not rewrite; the module now lives under meteor-methods/platform/ and its settings/dataExport specifiers changed depth, so the mock keys are updated to match. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Ten features whose methods/ dirs Phase 5 deliberately left in place move wholesale in Phases 6/7 — as written, their tables would sweep Meteor methods into server/lib/, breaking the meteor-methods consolidation. Adds explicit rows sending each methods/ dir to server/meteor-methods/<domain>/ (new media/ domain for custom-sounds, emoji-custom and file-upload admin methods), routes the integrations webhook REST API to server/api/webhooks.ts, and notes the co-located sendFileMessage.spec.ts mocha-glob move. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The import domain was the only one with an index re-exporting its method files; register them from server/meteor-methods/index.ts like every other domain and import the execute* functions directly in api/v1/import.ts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Phase 5b consolidated app/lib/server/index.ts's 34 method registrations into server/meteor-methods/index.ts but only carried 33 over — unblockUser was dropped, deregistering the Meteor method (used by external DDP clients; the web client goes through REST im.blockUser, so no in-repo test exercises it). A registration audit over every meteor-methods file found no other missing import; the plan now mandates that audit for Phases 6/7. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
a24220d to
398ee26
Compare
|
/jira ARCH-1539 |
|
The PR description was sent as plain text because Jira rejected the formatted body. |
The restructure (#40259, #41115, #41126, #41155, #41225, #41315, #41381) moved API, settings, auth, notifications, integrations and slash-command server code out of app/ into apps/meteor/server/ while this PR was open. Update every path reference and fix related factual drift: - app/api/server -> server/api (rest-endpoints, add-a-rest-endpoint, critical-flows, realtime-and-ddp, monorepo-layout, glossary) - app/settings/server -> server/settings; fix settings import example - sendMessage entry -> server/lib/messages/sendMessage.ts - notifications streams -> server/modules/notifications; auth entry -> server/hooks/auth + server/lib/auth-providers - slash commands: two registries (server/lib/utils + app/utils/client), server handlers in server/slashcommands/<name>/ - integrations -> server/lib/integrations + server/api/webhooks.ts - ee/apps count 8 -> 6: stream-hub-service and federation-service do not exist; federation runs in-process (server/services/), noted - module/package counts: ~100 -> ~37 app modules, 68 -> ~65 packages; drop nonexistent apps/docs workspace and string-helpers - Yarn 4.12.0 -> 4.17.1 (getting-started, troubleshooting) - Mongo troubleshooting: align with change-streams story; MONGO_OPLOG_URL is prod/ms-only; env vars must be shell-exported (no .env loader in dev) - drop stale apps/meteor/.env.example pointer - glossary proxify: models throw immediately; only core-services proxify waits/hangs - useTranslation: teach react-i18next form ({ t }), ui-contexts one is deprecated; useUniqueId does not exist (use React useId); theming via useThemeMode in ui-client (no ui-theming package) - remove Blaze bridge section: Blaze is fully removed - add-a-rest-endpoint: add the mandatory side-effect import step in server/api/index.ts (endpoint silently never mounts without it) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Proposed changes
Phase 5 of the backend folder-structure migration: rename
server/methods/→server/meteor-methods/and consolidate every Meteor method fromapp/*/server/methods/into domain subfolders. The new name makes explicit that these are Meteor-specific RPC handlers — deprecated and slated for removal — not REST handlers or service methods. Files move as-is — only import paths change, no refactoring.Please read comment with detailed information on the changes: #41225 (comment)
Folder structure
Community — before
Community — after
EE mirror — before / after (stays inside
ee/)Before
After
Test mirrors —
tests/unit/**follows its sourcesNo runner-glob changes needed:
tests/unit/server/**already covers the new mirrors.Eleven commits for reviewability: the rename, 5a (flat files → domain subfolders), 5b (app/lib methods), 5c (feature methods), EE mirror, then review follow-ups: a proxyquire fixup caught by the mocha run, Phase 6/7 docs amendments, dropping the import/ domain index in favor of direct registrations, restoring the dropped
unblockUserregistration, and deleting two 0-byte business-hour stubs left behind by #37772/#37819.Beyond file moves (see reviewer comment)
server/meteor-methods/index.ts— removed fromapp/lib/server/index.tsand 9 feature aggregators (threads' pure-aggregatormethods/index.tsdeleted). Three deliberate exceptions keep their previous registration path:createToken,removeUserFromRole, and the license-gated EE canned-responses methods..load()targets / mock keys (string literals lint & tsc can't validate).methods/dirs toserver/meteor-methods/<domain>/(newmedia/domain) andapp/integrations/server/api/api.tstoserver/api/webhooks.ts, so later phases can't sweep methods intoserver/lib/.Verification
yarn lint --quiet— clean (after each batch)yarn typecheck(meteor lint +tsc --noEmit --skipLibCheck) — 0 errorsverify-no-old-imports.mjs— only the 11 intentional references to methods dirs of features deferred to Phases 6/7 (file-upload, user-status, mentions, emoji-custom, push-notifications)Note
Not run here (need a running server/DB): integration suites and manual DDP smoke tests of the moved methods (send message, create channel, save settings, 2FA enable/disable, livechat message).
🤖 Generated with Claude Code
Task: ARCH-2241