Skip to content

Commit 3cdcf7f

Browse files
authored
refactor(users): split into admin/auth_local/oauth feature folders (#153)
* refactor(users): split into admin/auth_local/oauth feature folders Reorganize the users module intra-package from a type-folder layout (endpoints/, service.py, oauth.py at top level) to a feature-folder layout where each sub-feature owns its routes, service, views, and components. modules/users/users/ admin/ # service, REST API, Inertia views, React components auth_local/ # login/register/reset/verify/profile + accept-invite oauth/ # provider builders + per-provider routes (__init__.py # re-exports the prior `users.oauth.{X}` surface) Shared infrastructure stays at top level (deps, manager, backend, models, contracts, mailer, middleware, bootstrap, pages, locales). UsersModule.register_routes is now the single point of cross-feature wiring; no feature imports from a sibling feature in either the API or views layer. The login page's OAuth-provider list is cached once at boot on app.state.users.oauth_providers so auth_local/views.py doesn't reach into the oauth feature. Framework filesystem conventions are preserved unchanged: pages/ stays a single tree at the package root (required by simple_module_hosting.manifest.compute_module_pages) and locales/ stays in place. Scope: the users module only. The scaffold templates, framework-conventions docs, and other modules are deliberately not touched — this is a POC to validate the layout before propagating it. Verified locally: make lint, make doctor, full pytest (1010 passed), JS tests (8 passed), and a live uvicorn+Vite run exercising the refactored auth_local/api login wrapper, auth_local/views login page, admin/api list, admin/views Inertia index, and the moved admin/components/*.tsx imports. * fix(users): read login_redirect_url lazily on OAuth callback Address two issues raised in PR review: 1. `oauth/api.py` captured `settings.login_redirect_url` as a string at route-registration time, then closed over it. `UsersModule.on_startup` mutates `app.state.users.settings.login_redirect_url` (dashboard fallback), and admins can change it at runtime via the settings UI, so OAuth callbacks could redirect to a stale URL. Now the callback reads it from `request.app.state.users.settings` at request time. `_build_provider_router` no longer takes a `login_redirect_url` parameter. 2. The `register_routes` comment claimed constructing `UsersSettings()` "re-reads env_str-bound fields like OAuth client ids/secrets" — this is wrong: `env_str()` is evaluated when `UsersSettings` is defined (class-attribute defaults), so re-instantiating doesn't re-read env. Updated to accurately describe why a fresh instance is acceptable here (only `build_clients` reads it, and only static fields) and to point readers at `app.state.users.settings` for mutable fields. Both issues were pre-existing latent behaviour surfaced by the review of this PR. Tests still pass (1010); doctor clean.
1 parent 52f47a0 commit 3cdcf7f

26 files changed

Lines changed: 319 additions & 320 deletions

‎docs/testing/fixtures.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ The admin has `*` permission (via `DEFAULT_ROLE_PERMISSIONS["admin"]`), so it by
105105
```python
106106
@pytest.mark.asyncio
107107
async def test_non_admin_denied(client, db_session):
108-
from users.service import UserService
108+
from users.admin.service import UserService
109109
svc = UserService(db_session)
110110
await svc.create(email="u@e.com", password="x", roles=["viewer"])
111111
await db_session.commit()

‎modules/users/tests/test_api_auth.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ async def test_forgot_password_rate_limited_after_threshold(
198198
self, anon_client, users_app, users_db
199199
):
200200
"""After the configured attempt budget, /forgot-password returns 429."""
201-
from users.rate_limit import ThroughputLimiter
201+
from users.auth_local.rate_limit import ThroughputLimiter
202202

203203
# Tighten the limit for the test so we don't need to hit 10 real endpoints
204204
users_app.state.users.auth_throughput_limiter = ThroughputLimiter(

‎modules/users/tests/test_rate_limit.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
from __future__ import annotations
44

55
import pytest
6-
from users.rate_limit import LoginRateLimiter, ThroughputLimiter
6+
from users.auth_local.rate_limit import LoginRateLimiter, ThroughputLimiter
77

88

99
@pytest.fixture

‎modules/users/tests/test_service_admin.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@
1010

1111
def _build_service(session, users_app):
1212
"""Build a UserService directly (bypass FastAPI Depends)."""
13+
from users.admin.service import UserService
1314
from users.db_adapter import UserDatabaseWithRoles
1415
from users.manager import UserManager
1516
from users.models import User
16-
from users.service import UserService
1717

1818
user_db = UserDatabaseWithRoles(session, User)
1919
manager = UserManager(

‎modules/users/tests/test_user_service.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,10 @@
99

1010
def _build_service(session, users_app):
1111
"""Build a UserService directly (bypass FastAPI Depends)."""
12+
from users.admin.service import UserService
1213
from users.db_adapter import UserDatabaseWithRoles
1314
from users.manager import UserManager
1415
from users.models import User
15-
from users.service import UserService
1616

1717
user_db = UserDatabaseWithRoles(session, User)
1818
manager = UserManager(
@@ -77,10 +77,10 @@ async def test_get_list_item_unknown_user_raises_user_not_found(users_app):
7777
async def test_to_list_item_includes_created_at(users_app):
7878
"""`UserListItem` carries `created_at` sourced from AuditMixin."""
7979
from fastapi_users.password import PasswordHelper
80+
from users.admin.service import UserService
8081
from users.db_adapter import UserDatabaseWithRoles
8182
from users.manager import UserManager
8283
from users.models import User
83-
from users.service import UserService
8484

8585
async with users_app.state.sm.db.session_factory() as session:
8686
user = User(

‎modules/users/tests/test_views.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ async def test_admin_edit_page_unknown_user_returns_404(admin_client):
187187
@pytest.mark.anyio
188188
async def test_roles_payload_returns_id_name_dicts(users_app):
189189
"""Helper reads the roles cache and returns id/name dicts in cache order."""
190-
from users.endpoints.views import _roles_payload
190+
from users.admin.views import _roles_payload
191191

192192
payload = await _roles_payload(users_app)
193193

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
"""Admin user management. Intentionally empty — import via fully-qualified paths."""

modules/users/users/endpoints/api_admin.py renamed to modules/users/users/admin/api.py

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,4 @@
1-
"""Admin REST endpoints for the users module.
2-
3-
Split out of :mod:`.api` to keep per-file complexity manageable. Mounted
4-
into the main ``router`` via ``include_router`` at the bottom of ``api.py``.
5-
"""
1+
"""Admin REST endpoints for the users module."""
62

73
from __future__ import annotations
84

@@ -13,6 +9,7 @@
139
from simple_module_core.events import EventBus
1410
from simple_module_hosting.permissions import RequiresPermission
1511

12+
from users.admin.service import UserService
1613
from users.constants import PERM_USERS_MANAGE, sanitize_list_filters
1714
from users.contracts.events import RoleAssigned, UserDisabled, UserInvited
1815
from users.contracts.schemas import (
@@ -23,7 +20,6 @@
2320
)
2421
from users.deps import get_event_bus, get_mailer, get_user_service
2522
from users.exceptions import UserNotFoundError
26-
from users.service import UserService
2723

2824
admin_router = APIRouter(
2925
prefix="/admin",
File renamed without changes.
File renamed without changes.

0 commit comments

Comments
 (0)