refactor(routes): decouper users.py en paquet, extraire les notifications
ARCH-004 et la moitie NotificationService d ARCH-003. app/routes/users.py faisait 1 699 lignes et couvrait six sujets qui ne partageaient rien d autre qu un prefixe d URL. Il devient un paquet : blueprint.py l objet Blueprint, seul _shared.py helpers de formulaire, gamertags, validation de PDF accounts.py 348 l. liste, creation, edition, suppression, fiche availability.py 233 l. disponibilites joueur et creneaux coach contracts.py 206 l. depot, signature, telechargement notes.py 376 l. notes d equipe et notes nominatives one_on_one.py 259 l. demandes de seance individuelle profile.py 132 l. profil de la personne connectee Aucun fichier ne depasse 400 lignes -- le critere d acceptation de l audit. **Un seul blueprint, pas six.** Les endpoints restent `users.*`. Les renommer aurait touche 137 appels `url_for` dans les gabarits, pour un benefice nul : l objectif est un fichier qu on peut lire, pas une carte d URL a reapprendre. Les 30 endpoints sont identiques avant et apres, verifie sur url_map. send_discord_notification part dans app/services/notifications.py. Elle tirait `requests`, `logging` et le bot Discord dans un module dont le sujet est le traitement HTTP, et se trouvait coincee entre deux definitions de route. Son `except Exception` est conserve et documente : une notification qui n arrive pas ne doit pas annuler la transaction qu elle annoncait. tests/test_route_map.py, nouveau : il parcourt gabarits et code, releve tout endpoint nomme litteralement dans un url_for, et verifie qu il existe dans la carte. C est le mode de defaillance de ce genre de decoupage -- pas une erreur a l import, mais un BuildError chez la premiere personne qui ouvre la page concernee. Un test de garde verifie aussi que le scan trouve quelque chose, sinon le reste serait vide de sens. Piege rencontre, et corrige : test_role_change patchait `app.routes.users.update_user_gamertags`. Apres le decoupage ce nom est un reexport, pas celui qu accounts.py resout -- le patch aurait pu laisser la route appeler la vraie fonction et le test passer sans rien verifier. Ici monkeypatch a echoue bruyamment, mais la cible est desormais explicite et le test enregistre que la doublure a bien ete appelee. 347 tests passent (263 + 84, dont 82 parametres par la carte des routes). Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
GGThed
@@ -97,18 +97,28 @@ class TestAtomicity:
|
||||
):
|
||||
"""The role used to be committed on its own, before the rest of the
|
||||
edit ran. A failure afterwards left an account promoted, and no
|
||||
interface reported it."""
|
||||
interface reported it.
|
||||
|
||||
The patch target is the name *as accounts.py resolved it*. Patching
|
||||
a re-export instead would leave the route calling the real function,
|
||||
the request would succeed, and the assertions below would pass
|
||||
without testing anything — so `called` records that the stand-in
|
||||
actually ran.
|
||||
"""
|
||||
target_id = make_user('player')
|
||||
as_role('admin')
|
||||
called = []
|
||||
|
||||
def _explode(user, selected_games):
|
||||
called.append(True)
|
||||
raise RuntimeError('storage unavailable')
|
||||
|
||||
monkeypatch.setattr('app.routes.users.update_user_gamertags', _explode)
|
||||
monkeypatch.setattr('app.routes.users.accounts.update_user_gamertags', _explode)
|
||||
|
||||
with pytest.raises(RuntimeError):
|
||||
_edit(client, target_id)
|
||||
|
||||
assert called, 'the patched function was never reached'
|
||||
with app.app_context():
|
||||
user = db.session.get(User, target_id)
|
||||
assert isinstance(user, Player)
|
||||
|
||||
@@ -0,0 +1,110 @@
|
||||
"""Every endpoint the templates name still exists — ARCH-004.
|
||||
|
||||
Splitting app/routes/users.py into a package moved thirty route functions
|
||||
between modules. The failure mode of that kind of change is not a crash at
|
||||
import time: it is a `url_for('users.something')` in a template that nobody
|
||||
opens during the test run, raising BuildError for the one person who does.
|
||||
|
||||
So this walks the templates, collects every endpoint spelled out as a
|
||||
literal, and checks it against the real URL map. It costs one test and
|
||||
covers all seven blueprints, not just the one that moved.
|
||||
"""
|
||||
|
||||
import os
|
||||
import re
|
||||
|
||||
import pytest
|
||||
|
||||
#: url_for('blueprint.endpoint' — literal first argument only. Calls that
|
||||
#: compute the endpoint are rare here and cannot be checked statically.
|
||||
_URL_FOR = re.compile(r"""url_for\(\s*['"]([a-zA-Z_][\w.]*)['"]""")
|
||||
|
||||
_TEMPLATE_ROOT = os.path.join(
|
||||
os.path.dirname(os.path.dirname(os.path.abspath(__file__))), 'app', 'templates'
|
||||
)
|
||||
_CODE_ROOT = os.path.join(os.path.dirname(os.path.dirname(os.path.abspath(__file__))), 'app')
|
||||
|
||||
|
||||
def _referenced_endpoints(root, suffixes):
|
||||
"""Endpoints named in files under `root`, mapped to where they appear."""
|
||||
found = {}
|
||||
for directory, _dirs, files in os.walk(root):
|
||||
if '__pycache__' in directory or 'translations' in directory:
|
||||
continue
|
||||
for name in files:
|
||||
if not name.endswith(suffixes):
|
||||
continue
|
||||
path = os.path.join(directory, name)
|
||||
with open(path, encoding='utf-8') as handle:
|
||||
for endpoint in _URL_FOR.findall(handle.read()):
|
||||
found.setdefault(endpoint, set()).add(os.path.relpath(path, root))
|
||||
return found
|
||||
|
||||
|
||||
TEMPLATE_ENDPOINTS = _referenced_endpoints(_TEMPLATE_ROOT, ('.html',))
|
||||
CODE_ENDPOINTS = _referenced_endpoints(_CODE_ROOT, ('.py',))
|
||||
|
||||
|
||||
def test_the_scan_found_something():
|
||||
"""A regex that silently matches nothing would make the rest vacuous."""
|
||||
assert len(TEMPLATE_ENDPOINTS) > 30, TEMPLATE_ENDPOINTS
|
||||
|
||||
|
||||
@pytest.mark.parametrize('endpoint', sorted(TEMPLATE_ENDPOINTS))
|
||||
def test_a_template_endpoint_exists(app, endpoint):
|
||||
known = {rule.endpoint for rule in app.url_map.iter_rules()}
|
||||
assert endpoint in known, (
|
||||
f'{endpoint} is named in {sorted(TEMPLATE_ENDPOINTS[endpoint])} but no route provides it'
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize('endpoint', sorted(CODE_ENDPOINTS))
|
||||
def test_a_redirect_target_exists(app, endpoint):
|
||||
known = {rule.endpoint for rule in app.url_map.iter_rules()}
|
||||
assert endpoint in known, (
|
||||
f'{endpoint} is named in {sorted(CODE_ENDPOINTS[endpoint])} but no route provides it'
|
||||
)
|
||||
|
||||
|
||||
class TestTheUsersPackage:
|
||||
"""The split had to preserve the endpoint names exactly: 137 url_for
|
||||
calls spell them out, and a renamed blueprint would break every one."""
|
||||
|
||||
EXPECTED = {
|
||||
'users.accept_one_on_one',
|
||||
'users.add_disponibilities_bulk',
|
||||
'users.add_disponibility',
|
||||
'users.add_note_from_match',
|
||||
'users.add_note_from_tryout',
|
||||
'users.add_personal_note',
|
||||
'users.clear_coach_availability',
|
||||
'users.clear_disponibilities',
|
||||
'users.create_user',
|
||||
'users.delete_disponibility',
|
||||
'users.delete_user',
|
||||
'users.download_contract',
|
||||
'users.download_signed_contract',
|
||||
'users.edit_profile',
|
||||
'users.edit_user',
|
||||
'users.get_disponibilities',
|
||||
'users.get_my_disponibilities',
|
||||
'users.list_contracts',
|
||||
'users.list_users',
|
||||
'users.manage_coach_availability',
|
||||
'users.manage_personal_notes',
|
||||
'users.manage_team_notes',
|
||||
'users.my_notes',
|
||||
'users.notes_dashboard',
|
||||
'users.one_on_one',
|
||||
'users.profile',
|
||||
'users.reject_one_on_one',
|
||||
'users.upload_contract',
|
||||
'users.upload_signed_contract',
|
||||
'users.view_user',
|
||||
}
|
||||
|
||||
def test_the_same_thirty_routes_are_registered(self, app):
|
||||
registered = {
|
||||
rule.endpoint for rule in app.url_map.iter_rules() if rule.endpoint.startswith('users.')
|
||||
}
|
||||
assert registered == self.EXPECTED
|
||||
Reference in New Issue
Block a user