Skip to content

Commit a9f578d

Browse files
fix(billing): address PR review before stripe branch landing
Restore .env.example, drop BILLING_ENABLED, optimize nav middleware, fix webhook retry semantics, use custom Stripe exceptions, standardize has_permission checks, and revert unrelated footer redesign.
1 parent 19da3ec commit a9f578d

21 files changed

Lines changed: 276 additions & 243 deletions

.env.example

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,45 @@
1-
# Billing (set to 0 to disable Stripe and hide billing routes/UI)
2-
BILLING_ENABLED=1
1+
# Secret key for JWT
2+
SECRET_KEY=
33

4-
# Stripe billing (required when BILLING_ENABLED=1)
4+
# Base URL
5+
BASE_URL=http://localhost:8000
6+
7+
# Database connection mode (0 = direct, 1 = pooled)
8+
USE_POOL=0
9+
10+
# Shared database settings
11+
DB_HOST=127.0.0.1
12+
DB_SSLMODE=prefer
13+
14+
# Direct connection settings (when USE_POOL=0)
15+
DB_USER=postgres
16+
DB_PASSWORD=postgres
17+
DB_PORT=5432
18+
DB_NAME=
19+
20+
# Pooled connection settings (when USE_POOL=1)
21+
# DB_APPUSER=
22+
# DB_APPUSER_PASSWORD=
23+
# DB_POOL_PORT=6543
24+
# DB_POOL_NAME=
25+
26+
# Domain for production (used by Caddy for TLS)
27+
DOMAIN=
28+
29+
# Rate limit storage: memory (single process) or postgres (multi-worker)
30+
# RATE_LIMIT_BACKEND=memory
31+
32+
# Comma-separated reverse-proxy peer IPs for X-Forwarded-For (e.g. 127.0.0.1,::1)
33+
# TRUSTED_PROXY_IPS=
34+
35+
# Set to 0 to disable CSRF checks (not recommended in production)
36+
# CSRF_ENABLED=1
37+
38+
# Resend
39+
RESEND_API_KEY=
40+
EMAIL_FROM=
41+
42+
# Stripe billing (required on the stripe branch)
543
STRIPE_SECRET_KEY=
644
STRIPE_WEBHOOK_SECRET=
745
STRIPE_PRICE_ID=

exceptions/http_exceptions.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,30 @@ def __init__(self) -> None:
259259
)
260260

261261

262+
class ActiveSubscriptionError(HTTPException):
263+
def __init__(self) -> None:
264+
super().__init__(
265+
status_code=409,
266+
detail="This organization already has an active subscription.",
267+
)
268+
269+
270+
class StripeServiceUnavailableError(HTTPException):
271+
def __init__(self, action: str) -> None:
272+
super().__init__(
273+
status_code=503,
274+
detail=f"Unable to {action}. Please try again later.",
275+
)
276+
277+
278+
class StripeSessionError(HTTPException):
279+
def __init__(self, session_kind: str) -> None:
280+
super().__init__(
281+
status_code=502,
282+
detail=f"Stripe did not return a {session_kind} URL.",
283+
)
284+
285+
262286
class CsrfError(HTTPException):
263287
"""Raised when CSRF validation fails."""
264288

main.py

Lines changed: 9 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,6 @@
22
from contextlib import asynccontextmanager
33

44
from dotenv import load_dotenv
5-
6-
load_dotenv()
7-
85
from fastapi import FastAPI, Request, Depends, status
96
from fastapi.responses import RedirectResponse, Response
107
from fastapi.staticfiles import StaticFiles
@@ -51,8 +48,8 @@
5148
)
5249
from exceptions.exceptions import NeedsNewTokens
5350
from routers.app import billing as billing_router
54-
from utils.app.credentials import billing_enabled, validate_billing_environment
55-
from utils.app.billing import billing_nav_href
51+
from utils.app.credentials import validate_billing_environment
52+
from utils.app.billing import resolve_billing_nav_href, should_resolve_billing_nav
5653
from utils.core.db import set_up_db
5754

5855
logger = logging.getLogger("uvicorn.error")
@@ -61,6 +58,7 @@
6158

6259
@asynccontextmanager
6360
async def lifespan(app: FastAPI):
61+
load_dotenv()
6462
validate_billing_environment()
6563
set_up_db()
6664
yield
@@ -88,32 +86,12 @@ async def lifespan(app: FastAPI):
8886
@app.middleware("http")
8987
async def nav_billing_middleware(request: Request, call_next):
9088
request.state.billing_nav_href = None
91-
if billing_enabled():
89+
if should_resolve_billing_nav(request):
9290
access_token = request.cookies.get("access_token")
9391
if access_token:
94-
from sqlmodel import Session, select
95-
from sqlalchemy.orm import selectinload
96-
97-
from utils.core.db import get_engine
98-
from utils.core.dependencies import get_optional_user_from_access_token
99-
from utils.core.models import Role, User
100-
101-
engine = get_engine()
102-
with Session(engine) as session:
103-
user = get_optional_user_from_access_token(access_token, session)
104-
if user and user.id is not None:
105-
eager_user = session.exec(
106-
select(User)
107-
.where(User.id == user.id)
108-
.options(
109-
selectinload(User.roles).selectinload(Role.organization),
110-
selectinload(User.roles).selectinload(Role.permissions),
111-
)
112-
).first()
113-
if eager_user is not None:
114-
request.state.billing_nav_href = billing_nav_href(
115-
request, eager_user
116-
)
92+
request.state.billing_nav_href = resolve_billing_nav_href(
93+
request, access_token
94+
)
11795
return await call_next(request)
11896

11997

@@ -154,9 +132,8 @@ async def csrf_middleware(request: Request, call_next):
154132
app.include_router(role.router)
155133
app.include_router(static_pages.router)
156134
app.include_router(user.router)
157-
if billing_enabled():
158-
app.include_router(billing_router.router)
159-
app.include_router(billing_router.webhook_router)
135+
app.include_router(billing_router.router)
136+
app.include_router(billing_router.webhook_router)
160137

161138

162139
# --- Exception Handling Middlewares ---

pyproject.toml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,7 @@ dev = [
4141
]
4242

4343
[tool.ruff.lint.per-file-ignores]
44-
# load_dotenv() must run before app imports that read environment variables
45-
"main.py" = ["E402"]
44+
# load_dotenv() runs before app import in tests/conftest.py
4645
"tests/conftest.py" = ["E402"]
4746

4847
[tool.ty.rules]

routers/app/billing.py

Lines changed: 12 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,17 @@
66
import uuid
77

88
import stripe
9-
from fastapi import APIRouter, Depends, HTTPException, Query, Request, Response
9+
from fastapi import APIRouter, Depends, Query, Request, Response
1010
from fastapi.responses import RedirectResponse
1111
from fastapi.templating import Jinja2Templates
1212
from sqlmodel import Session
1313

1414
from exceptions.http_exceptions import (
15+
ActiveSubscriptionError,
1516
InsufficientPermissionsError,
1617
OrganizationNotFoundError,
18+
StripeServiceUnavailableError,
19+
StripeSessionError,
1720
)
1821
from utils.app.billing import (
1922
billing_status,
@@ -54,15 +57,6 @@ def _require_org_member(user: User, org_id: int):
5457
return org
5558

5659

57-
def _user_permissions_for_org(user: User, org_id: int) -> set:
58-
permissions: set = set()
59-
for role in user.roles:
60-
if role.organization_id == org_id:
61-
for permission in role.permissions:
62-
permissions.add(permission.name)
63-
return permissions
64-
65-
6660
@router.get("/{org_id}/billing")
6761
async def read_billing(
6862
org_id: int,
@@ -71,14 +65,13 @@ async def read_billing(
7165
session: Session = Depends(get_session),
7266
):
7367
org = _require_org_member(user, org_id)
74-
permissions = _user_permissions_for_org(user, org_id)
75-
if AppPermissions.VIEW_BILLING not in permissions:
68+
if not user.has_permission(AppPermissions.VIEW_BILLING, org_id):
7669
raise InsufficientPermissionsError()
7770

7871
billing = get_org_billing(session, org_id)
7972
status = billing_status(billing)
8073
settings = stripe_settings()
81-
can_manage = AppPermissions.MANAGE_BILLING in permissions
74+
can_manage = user.has_permission(AppPermissions.MANAGE_BILLING, org_id)
8275

8376
return templates.TemplateResponse(
8477
request,
@@ -140,9 +133,7 @@ async def start_checkout(
140133
"Checkout blocked for org_id=%s: organization already has an active subscription",
141134
org_id,
142135
)
143-
raise InsufficientPermissionsError(
144-
"This organization already has an active subscription."
145-
)
136+
raise ActiveSubscriptionError()
146137

147138
assert user.account is not None
148139
billing = get_org_billing(session, org_id)
@@ -158,14 +149,11 @@ async def start_checkout(
158149
logger.exception(
159150
"Stripe Checkout session creation failed for org_id=%s", org_id
160151
)
161-
raise HTTPException(
162-
status_code=503,
163-
detail="Unable to start checkout. Please try again later.",
164-
) from None
152+
raise StripeServiceUnavailableError("start checkout") from None
165153

166154
checkout_url = checkout.url
167155
if not checkout_url:
168-
raise InsufficientPermissionsError("Stripe did not return a checkout URL.")
156+
raise StripeSessionError("checkout")
169157
return _redirect_to_stripe(request, checkout_url)
170158

171159

@@ -193,14 +181,11 @@ async def start_customer_portal(
193181
)
194182
except stripe.StripeError:
195183
logger.exception("Stripe Customer Portal session failed for org_id=%s", org_id)
196-
raise HTTPException(
197-
status_code=503,
198-
detail="Unable to open billing portal. Please try again later.",
199-
) from None
184+
raise StripeServiceUnavailableError("open billing portal") from None
200185

201186
portal_url = portal.url
202187
if not portal_url:
203-
raise InsufficientPermissionsError("Stripe did not return a portal URL.")
188+
raise StripeSessionError("portal")
204189
return _redirect_to_stripe(request, portal_url)
205190

206191

@@ -313,7 +298,7 @@ async def stripe_webhook(
313298
return Response(status_code=200, content='{"ok": true}')
314299

315300
result = handle_stripe_webhook_event(session, event)
316-
if result is WebhookHandleResult.FAILED:
301+
if result in {WebhookHandleResult.FAILED, WebhookHandleResult.RETRYABLE}:
317302
release_webhook_event_claim(session, event.id)
318303
return Response(status_code=500, content="Webhook handler failed")
319304

routers/core/organization.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
DataIntegrityError,
2525
)
2626
from utils.app.billing import cancel_org_stripe_subscription
27-
from utils.app.credentials import billing_enabled
2827
from pydantic import EmailStr
2928
from utils.core.htmx import is_htmx_request, set_flash_cookie
3029

@@ -86,7 +85,6 @@ async def read_organization(
8685
"AppPermissions": AppPermissions,
8786
"all_permissions": list(ValidPermissions) + list(AppPermissions),
8887
"pending_invitations": pending_invitations,
89-
"billing_enabled": billing_enabled(),
9088
},
9189
)
9290

static/css/extras.css

Lines changed: 0 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -434,68 +434,6 @@ input:invalid:not(:placeholder-shown) + .invalid-feedback {
434434
}
435435
}
436436

437-
/* Site footer: align horizontal padding with navbar; spread copyright and contact. */
438-
.site-footer-inner {
439-
padding-inline: 1.25rem;
440-
}
441-
442-
.site-footer-row {
443-
display: grid;
444-
gap: 1.25rem;
445-
text-align: center;
446-
}
447-
448-
.site-footer-copyright,
449-
.site-footer-links,
450-
.site-footer-contact {
451-
min-width: 0;
452-
}
453-
454-
.site-footer-heading {
455-
font-family: var(--font-display);
456-
font-weight: 600;
457-
font-size: 1rem;
458-
line-height: 1.15;
459-
letter-spacing: -0.015em;
460-
color: var(--muted);
461-
margin: 0 0 0.5rem;
462-
}
463-
464-
.site-footer-body,
465-
.site-footer-body a {
466-
font-family: var(--font-body);
467-
font-size: 1rem;
468-
line-height: 1.55;
469-
color: var(--muted);
470-
}
471-
472-
.site-footer-body a:hover {
473-
color: var(--primary);
474-
}
475-
476-
@media (min-width: 992px) {
477-
.site-footer-row {
478-
grid-template-columns: 1fr auto 1fr;
479-
align-items: end;
480-
gap: 2rem;
481-
}
482-
483-
.site-footer-copyright {
484-
justify-self: start;
485-
text-align: start;
486-
}
487-
488-
.site-footer-links {
489-
justify-self: center;
490-
text-align: center;
491-
}
492-
493-
.site-footer-contact {
494-
justify-self: end;
495-
text-align: end;
496-
}
497-
}
498-
499437
/* Pending invitations in organization members card. */
500438
.invitation-list-item-body {
501439
display: flex;

0 commit comments

Comments
 (0)