Skip to content

Conversation

@tassoevan
Copy link
Contributor

@tassoevan tassoevan commented Oct 10, 2025

Proposed changes (including videos or screenshots)

Issue(s)

ARCH-1848

Steps to test or reproduce

Further comments

Extracted from #36692

Summary by CodeRabbit

  • Refactor
    • Reorganized module structure and import paths for authentication and startup sequences to improve code organization and maintainability.

@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Oct 10, 2025

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

@changeset-bot
Copy link

changeset-bot bot commented Oct 10, 2025

⚠️ No Changeset found

Latest commit: 00a5468

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
Copy link
Contributor

coderabbitai bot commented Oct 10, 2025

Walkthrough

This PR reorganizes client-side authentication modules by moving login provider implementations into a dedicated apps/meteor/client/meteor/login/ directory structure, updating all corresponding import paths to reflect the new hierarchy, and introducing index files to aggregate these modules for cleaner initialization.

Changes

Cohort / File(s) Summary
Login provider modules
apps/meteor/client/meteor/login/cas.ts, apps/meteor/client/meteor/login/crowd.ts, apps/meteor/client/meteor/login/facebook.ts, apps/meteor/client/meteor/login/github.ts, apps/meteor/client/meteor/login/google.ts, apps/meteor/client/meteor/login/ldap.ts, apps/meteor/client/meteor/login/meteorDeveloperAccount.ts, apps/meteor/client/meteor/login/oauth.ts, apps/meteor/client/meteor/login/password.ts, apps/meteor/client/meteor/login/saml.ts, apps/meteor/client/meteor/login/twitter.ts
Updated relative import paths from ../../../lib/ to ../../lib/ to reflect new directory structure.
Login aggregator
apps/meteor/client/meteor/login/index.ts
New file that imports and aggregates all login provider modules.
CustomOAuth import
apps/meteor/client/lib/customOAuth/CustomOAuth.ts
Updated import path for createOAuthTotpLoginMethod from ../../meteor/overrides/login/oauth to ../../meteor/login/oauth.
Startup organization
apps/meteor/client/meteor/startup/index.ts
New file aggregating startup modules (absoluteUrl and accounts).
Startup modules
apps/meteor/client/meteor/startup/absoluteUrl.ts, apps/meteor/client/meteor/startup/accounts.ts
Updated import paths to reflect directory restructuring.
Overrides reorganization
apps/meteor/client/meteor/overrides/desktopInjection.ts, apps/meteor/client/meteor/overrides/index.ts
Updated and reorganized imports; login providers removed from overrides index, replaced with desktopInjection and settings aggregation.
Main boot sequence
apps/meteor/client/main.ts
Replaced direct imports of ./startup/accounts and ./startup/desktopInjection with aggregated ./meteor/overrides and ./meteor/startup; extended dynamic import chain to load ./meteor/login, ./omnichannel, and additional views.
Legacy startup
apps/meteor/client/startup/index.ts
Removed import of absoluteUrl; module initialization now handled via aggregated startup index.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • All changes follow a consistent pattern: updating relative import paths (from ../../../lib/ to ../../lib/) and reorganizing module aggregation across multiple files
  • No functional logic, error handling, or control flow alterations
  • Primary effort involves verifying path accuracy and that all module references resolve correctly after the restructuring

Possibly related PRs

Suggested labels

stat: ready to merge, stat: QA assured

Suggested reviewers

  • aleksandernsilva
  • jeanfbrito

Poem

🐰 Modules hop to their new nest so neat,
Paths reorganized in rhythmic beat,
Import chains dance with simpler grace,
Each directory finds its perfect place!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title "chore: Move Meteor specific startup code" refers to a real aspect of the changeset, specifically the reorganization of startup-related modules into a new startup/index.ts aggregator and adjustments to startup file import paths. However, the title does not capture the full scope of the PR. The changes are significantly broader and include reorganizing login provider modules (cas, crowd, facebook, github, google, ldap, meteorDeveloperAccount, oauth, password, saml, twitter) into a new login/index.ts aggregator, restructuring directory organization across the client codebase, updating numerous import paths, and reorganizing the overrides/index.ts file. The title focuses narrowly on startup code while missing these other major restructuring efforts that appear to be central to the PR's purpose.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/client-move-meteor-startup

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 and usage tips.

@codecov
Copy link

codecov bot commented Oct 10, 2025

Codecov Report

❌ Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.92%. Comparing base (65fbcbe) to head (00a5468).
⚠️ Report is 4 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #37199      +/-   ##
===========================================
- Coverage    67.92%   67.92%   -0.01%     
===========================================
  Files         3356     3356              
  Lines       114894   114894              
  Branches     20754    20756       +2     
===========================================
- Hits         78040    78038       -2     
+ Misses       34171    34168       -3     
- Partials      2683     2688       +5     
Flag Coverage Δ
e2e 57.43% <50.00%> (-0.03%) ⬇️
unit 71.99% <ø> (+<0.01%) ⬆️

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.

@tassoevan tassoevan force-pushed the chore/client-move-meteor-startup branch 7 times, most recently from 0345e3d to 89b240d Compare October 14, 2025 17:13
@tassoevan tassoevan closed this Oct 14, 2025
@tassoevan tassoevan force-pushed the chore/client-move-meteor-startup branch from 89b240d to e488086 Compare October 14, 2025 18:08
@tassoevan tassoevan reopened this Oct 14, 2025
@tassoevan tassoevan force-pushed the chore/client-move-meteor-startup branch 11 times, most recently from 4e969ac to 04adee8 Compare October 21, 2025 13:28
@tassoevan tassoevan force-pushed the chore/client-move-meteor-startup branch 5 times, most recently from d3a127c to 1020a70 Compare October 27, 2025 20:34
@tassoevan tassoevan force-pushed the chore/client-move-meteor-startup branch from 1020a70 to 00a5468 Compare October 30, 2025 20:11
@tassoevan tassoevan added this to the 7.13.0 milestone Oct 30, 2025
@tassoevan tassoevan marked this pull request as ready for review October 30, 2025 20:46
@tassoevan tassoevan requested a review from a team as a code owner October 30, 2025 20:46
@ggazzo ggazzo added the stat: QA assured Means it has been tested and approved by a company insider label Oct 31, 2025
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Oct 31, 2025
@ggazzo ggazzo merged commit 9434991 into develop Oct 31, 2025
90 of 92 checks passed
@ggazzo ggazzo deleted the chore/client-move-meteor-startup branch October 31, 2025 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants