refactor: use shared ProfileUpdate validation in admin routes
This commit is contained in:
parent
5fd63d61ff
commit
7bfea306ab
2 changed files with 56 additions and 16 deletions
|
|
@ -1,11 +1,12 @@
|
|||
from base64 import urlsafe_b64decode
|
||||
from urllib.parse import urlparse
|
||||
|
||||
from fastapi import APIRouter, Form, Request, Response
|
||||
from fastapi.responses import HTMLResponse, RedirectResponse
|
||||
from pydantic import ValidationError
|
||||
|
||||
from porchlight.dependencies import get_session_user
|
||||
from porchlight.models import User
|
||||
from porchlight.validation import ProfileUpdate
|
||||
|
||||
router = APIRouter(prefix="/admin", tags=["admin"])
|
||||
|
||||
|
|
@ -143,12 +144,35 @@ async def update_user_profile(
|
|||
return HTMLResponse("Forbidden", status_code=403)
|
||||
|
||||
# Validate
|
||||
if email and "@" not in email:
|
||||
return HTMLResponse('<div role="alert">Invalid email address</div>')
|
||||
if picture:
|
||||
parsed = urlparse(picture)
|
||||
if parsed.scheme not in ("http", "https") or not parsed.netloc:
|
||||
return HTMLResponse('<div role="alert">Picture URL must be a valid HTTP or HTTPS URL</div>')
|
||||
try:
|
||||
profile = ProfileUpdate(
|
||||
given_name=given_name,
|
||||
family_name=family_name,
|
||||
preferred_username=preferred_username,
|
||||
email=email,
|
||||
phone_number=phone_number,
|
||||
picture=picture,
|
||||
locale=locale,
|
||||
)
|
||||
except ValidationError as exc:
|
||||
error = exc.errors()[0]
|
||||
field = error["loc"][-1] if error["loc"] else "input"
|
||||
msg = error["msg"]
|
||||
labels = {
|
||||
"given_name": "Given name",
|
||||
"family_name": "Family name",
|
||||
"preferred_username": "Display name",
|
||||
"email": "Email",
|
||||
"phone_number": "Phone number",
|
||||
"picture": "Picture URL",
|
||||
"locale": "Locale",
|
||||
}
|
||||
label = labels.get(str(field), str(field))
|
||||
if error["type"] == "value_error":
|
||||
display_msg = msg.removeprefix("Value error, ")
|
||||
else:
|
||||
display_msg = f"{label}: {msg}"
|
||||
return HTMLResponse(f'<div role="alert">{display_msg}</div>')
|
||||
|
||||
user_repo = request.app.state.user_repo
|
||||
user = await user_repo.get_by_userid(userid)
|
||||
|
|
@ -157,13 +181,13 @@ async def update_user_profile(
|
|||
|
||||
updated = user.model_copy(
|
||||
update={
|
||||
"given_name": given_name or None,
|
||||
"family_name": family_name or None,
|
||||
"preferred_username": preferred_username or None,
|
||||
"email": email or None,
|
||||
"phone_number": phone_number or None,
|
||||
"picture": picture or None,
|
||||
"locale": locale or None,
|
||||
"given_name": profile.given_name or None,
|
||||
"family_name": profile.family_name or None,
|
||||
"preferred_username": profile.preferred_username or None,
|
||||
"email": profile.email,
|
||||
"phone_number": profile.phone_number,
|
||||
"picture": profile.picture,
|
||||
"locale": profile.locale or None,
|
||||
}
|
||||
)
|
||||
await user_repo.update(updated)
|
||||
|
|
|
|||
|
|
@ -153,7 +153,7 @@ async def test_update_profile(client: AsyncClient) -> None:
|
|||
"family_name": "Smith",
|
||||
"preferred_username": "bobby",
|
||||
"email": "bob@example.com",
|
||||
"phone_number": "+1234567890",
|
||||
"phone_number": "+46701234567",
|
||||
"picture": "https://example.com/bob.jpg",
|
||||
"locale": "en-US",
|
||||
},
|
||||
|
|
@ -183,7 +183,23 @@ async def test_update_profile_invalid_email(client: AsyncClient) -> None:
|
|||
headers={"X-CSRF-Token": token},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
assert "Invalid email" in response.text
|
||||
assert "alert" in response.text
|
||||
assert "email" in response.text.lower()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_profile_invalid_phone(client: AsyncClient) -> None:
|
||||
await _login(client)
|
||||
target = await _create_target_user(client, username="bob")
|
||||
token = await get_csrf_token(client)
|
||||
response = await client.post(
|
||||
f"/admin/users/{target.userid}/profile",
|
||||
data={"phone_number": "not-a-phone"},
|
||||
headers={"X-CSRF-Token": token},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
assert "alert" in response.text
|
||||
assert "phone" in response.text.lower()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue