Files
GGThedandClaude Opus 5 5c87064a17 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]>
2026-08-08 17:14:26 -04:00

126 lines
4.4 KiB
Python

"""Changing a user's role — ARCH-008.
`role` is the polymorphic discriminator, and SQLAlchemy fixes an instance's
class at load time. edit_user therefore writes the column with a
statement-level UPDATE and re-reads the row, which is correct. What it did
next was not: it called db.session.remove(), throwing away the whole
session, and it committed the role on its own before the rest of the edit
had been applied.
The two consequences these tests pin down:
- everything the request still held became detached, current_user
included. The route only survived because it had copied the actor's
name and id into locals beforehand — a workaround, in place of the fix.
- the role change was already committed when the rest of the edit ran, so
a failure afterwards left the account promoted and nothing else
applied.
"""
import pytest
from app.extensions import db
from app.models import Coach, Player, User
def _edit(client, user_id, **overrides):
payload = {
'full_name': 'Promoted Person',
'email': '[email protected]',
'role': 'coach',
'is_active_account': 'on',
}
payload.update(overrides)
return client.post(f'/users/{user_id}/edit', data=payload, follow_redirects=True)
class TestPolymorphicIdentity:
def test_the_account_takes_its_new_class(self, app, client, as_role, make_user):
target_id = make_user('player')
as_role('admin')
_edit(client, target_id)
with app.app_context():
assert isinstance(db.session.get(User, target_id), Coach)
def test_the_new_role_grants_its_pages_right_away(self, app, client, as_role, make_user, login):
"""isinstance() is how this application authorises; a stale class in
the identity map is a stale permission set."""
target_id = make_user('player')
as_role('admin')
_edit(client, target_id)
with app.app_context():
username = db.session.get(User, target_id).username
client.post('/auth/logout')
login(username)
# Coach-only, and it is a coach's own page rather than a redirect.
assert client.get('/users/notes-dashboard').status_code == 200
def test_the_other_fields_of_the_edit_land_too(self, app, client, as_role, make_user):
target_id = make_user('player')
as_role('admin')
_edit(client, target_id, full_name='Renamed Too', phone='555-0100')
with app.app_context():
user = db.session.get(User, target_id)
assert user.full_name == 'Renamed Too'
assert user.phone == '555-0100'
class TestTheActingPresident:
"""db.session.remove() detached current_user mid-request."""
def test_the_president_keeps_their_session(self, client, as_role, make_user):
target_id = make_user('player')
as_role('admin')
_edit(client, target_id)
assert client.get('/users').status_code == 200
def test_the_response_renders(self, client, as_role, make_user):
target_id = make_user('player')
as_role('admin')
response = _edit(client, target_id)
assert response.status_code == 200
class TestAtomicity:
def test_a_failure_after_the_update_leaves_the_role_alone(
self, app, client, as_role, make_user, monkeypatch
):
"""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.
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.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)
assert user.role == 'player'