Merge pull request #805 from christianlouis/copilot/fix-image-build-failure
fix(build): remove --omit=dev from npm ci in Dockerfile frontend-builder stage
This commit is contained in:
+1
-1
@@ -34,7 +34,7 @@ WORKDIR /frontend
|
||||
|
||||
# Install dependencies first (layer-cached unless package.json/lockfile changes)
|
||||
COPY frontend/package.json frontend/package-lock.json ./
|
||||
RUN npm ci --omit=dev
|
||||
RUN npm ci
|
||||
|
||||
# Copy source files and compile Tailwind CSS
|
||||
COPY frontend/ ./
|
||||
|
||||
+1
-1
@@ -260,7 +260,7 @@ async def stripe_webhook(request: Request, db: Session = Depends(get_db)) -> dic
|
||||
@require_login
|
||||
async def billing_success(request: Request) -> Any:
|
||||
"""Show a success page after a completed Stripe Checkout."""
|
||||
return _templates.TemplateResponse("billing_success.html", {"request": request})
|
||||
return _templates.TemplateResponse(request, "billing_success.html")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -101,9 +101,9 @@ async def signup_page(request: Request) -> Any:
|
||||
if not settings.allow_local_signup:
|
||||
return RedirectResponse(url="/login?error=Registration+is+not+enabled", status_code=302)
|
||||
return templates.TemplateResponse(
|
||||
request,
|
||||
"signup.html",
|
||||
{
|
||||
"request": request,
|
||||
context={
|
||||
"csrf_token": getattr(request.state, "csrf_token", ""),
|
||||
"app_version": settings.version,
|
||||
},
|
||||
@@ -113,16 +113,16 @@ async def signup_page(request: Request) -> Any:
|
||||
@router.get("/verify-email-sent", include_in_schema=False)
|
||||
async def verify_email_sent_page(request: Request) -> Any:
|
||||
"""Render the verify-email-sent confirmation page."""
|
||||
return templates.TemplateResponse("verify_email_sent.html", {"request": request})
|
||||
return templates.TemplateResponse(request, "verify_email_sent.html")
|
||||
|
||||
|
||||
@router.get("/forgot-username", include_in_schema=False)
|
||||
async def forgot_username_page(request: Request) -> Any:
|
||||
"""Render the forgot-username page where users can request a username reminder email."""
|
||||
return templates.TemplateResponse(
|
||||
request,
|
||||
"forgot_username.html",
|
||||
{
|
||||
"request": request,
|
||||
context={
|
||||
"csrf_token": getattr(request.state, "csrf_token", ""),
|
||||
"app_version": settings.version,
|
||||
},
|
||||
@@ -133,9 +133,9 @@ async def forgot_username_page(request: Request) -> Any:
|
||||
async def forgot_password_page(request: Request) -> Any:
|
||||
"""Render the forgot-password page where users can request a reset email."""
|
||||
return templates.TemplateResponse(
|
||||
request,
|
||||
"forgot_password.html",
|
||||
{
|
||||
"request": request,
|
||||
context={
|
||||
"csrf_token": getattr(request.state, "csrf_token", ""),
|
||||
"app_version": settings.version,
|
||||
},
|
||||
@@ -147,9 +147,9 @@ async def reset_password_page(request: Request) -> Any:
|
||||
"""Render the password reset form page."""
|
||||
token = request.query_params.get("token", "")
|
||||
return templates.TemplateResponse(
|
||||
request,
|
||||
"password_reset_form.html",
|
||||
{
|
||||
"request": request,
|
||||
context={
|
||||
"token": token,
|
||||
"csrf_token": getattr(request.state, "csrf_token", ""),
|
||||
"app_version": settings.version,
|
||||
|
||||
+2
-2
@@ -536,9 +536,9 @@ async def login(request: Request):
|
||||
return RedirectResponse(url="/oauth-login", status_code=status.HTTP_302_FOUND)
|
||||
|
||||
return templates.TemplateResponse(
|
||||
request,
|
||||
"login.html",
|
||||
{
|
||||
"request": request,
|
||||
context={
|
||||
"error": error,
|
||||
"message": message,
|
||||
"show_oauth": show_oauth,
|
||||
|
||||
+4
-5
@@ -424,15 +424,13 @@ async def http_exception_handler(request: Request, exc: HTTPException):
|
||||
# For frontend routes, return appropriate HTML templates
|
||||
# Handle 404 errors with a custom template
|
||||
if exc.status_code == 404:
|
||||
return _error_templates.TemplateResponse(
|
||||
"404.html", {"request": request}, status_code=status.HTTP_404_NOT_FOUND
|
||||
)
|
||||
return _error_templates.TemplateResponse(request, "404.html", status_code=status.HTTP_404_NOT_FOUND)
|
||||
|
||||
# For other HTTP errors, we could create specific templates or use a generic one
|
||||
# For now, return a simple error page
|
||||
return _error_templates.TemplateResponse(
|
||||
request,
|
||||
"404.html", # Reuse 404 template for other errors, or create a generic error template
|
||||
{"request": request},
|
||||
status_code=exc.status_code,
|
||||
)
|
||||
|
||||
@@ -452,8 +450,9 @@ async def custom_500_handler(request: Request, exc: Exception):
|
||||
|
||||
# Serve the 500 template for non-API routes
|
||||
return _error_templates.TemplateResponse(
|
||||
request,
|
||||
"500.html",
|
||||
{"request": request, "exc": exc},
|
||||
context={"exc": exc},
|
||||
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||
)
|
||||
|
||||
|
||||
+29
-5
@@ -162,12 +162,36 @@ def _inject_global_context(ctx: dict) -> None:
|
||||
|
||||
|
||||
def template_response_with_version(*args, **kwargs):
|
||||
"""Wrapper for TemplateResponse to include version and CSRF token in all templates"""
|
||||
# If context dict is provided, add version to it
|
||||
if len(args) >= 2 and isinstance(args[1], dict):
|
||||
_inject_global_context(args[1])
|
||||
elif "context" in kwargs and isinstance(kwargs["context"], dict):
|
||||
"""Wrapper for TemplateResponse to include version and CSRF token in all templates.
|
||||
|
||||
Handles both old-style and new-style Starlette TemplateResponse calls:
|
||||
- Old-style (Starlette <1.0): TemplateResponse(name, {"request": req, ...}, ...)
|
||||
- New-style (Starlette 1.0+): TemplateResponse(request, name, context={...}, ...)
|
||||
"""
|
||||
if len(args) >= 1 and isinstance(args[0], str):
|
||||
# Old-style call: first positional arg is the template name (string).
|
||||
# Convert to new-style: (request, name, context=..., ...)
|
||||
name = args[0]
|
||||
if len(args) >= 2 and isinstance(args[1], dict):
|
||||
context = args[1]
|
||||
# Old-style may have status_code as 3rd positional arg
|
||||
if len(args) >= 3 and "status_code" not in kwargs:
|
||||
kwargs["status_code"] = args[2]
|
||||
else:
|
||||
context = kwargs.pop("context", {})
|
||||
request_obj = context.pop("request", None)
|
||||
if request_obj is not None:
|
||||
context["request"] = request_obj
|
||||
_inject_global_context(context)
|
||||
if request_obj is not None:
|
||||
return original_template_response(request_obj, name, context=context, **kwargs)
|
||||
return original_template_response(name, context=context, **kwargs)
|
||||
|
||||
# New-style call: (request, name, context=..., ...)
|
||||
if "context" in kwargs and isinstance(kwargs["context"], dict):
|
||||
_inject_global_context(kwargs["context"])
|
||||
elif len(args) >= 3 and isinstance(args[2], dict):
|
||||
_inject_global_context(args[2])
|
||||
return original_template_response(*args, **kwargs)
|
||||
|
||||
|
||||
|
||||
+2
-1
@@ -23,6 +23,7 @@ templates = Jinja2Templates(directory=str(_templates_dir))
|
||||
async def shared_link_view(request: Request, token: str):
|
||||
"""Render the public share landing page for a given token."""
|
||||
return templates.TemplateResponse(
|
||||
request,
|
||||
"shared_link_view.html",
|
||||
{"request": request, "token": token},
|
||||
context={"token": token},
|
||||
)
|
||||
|
||||
+3
-3
@@ -430,8 +430,8 @@ class TestLoginFunction:
|
||||
# Verify TemplateResponse was called with correct context
|
||||
mock_templates.TemplateResponse.assert_called_once()
|
||||
call_args = mock_templates.TemplateResponse.call_args
|
||||
assert call_args[0][0] == "login.html"
|
||||
context = call_args[0][1]
|
||||
assert call_args[0][1] == "login.html"
|
||||
context = call_args.kwargs["context"]
|
||||
assert context["error"] == "Test error"
|
||||
assert context["message"] == "Test message"
|
||||
|
||||
@@ -450,7 +450,7 @@ class TestLoginFunction:
|
||||
|
||||
mock_templates.TemplateResponse.assert_called_once()
|
||||
call_args = mock_templates.TemplateResponse.call_args
|
||||
context = call_args[0][1]
|
||||
context = call_args.kwargs["context"]
|
||||
assert context["error"] is None
|
||||
assert context["message"] is None
|
||||
|
||||
|
||||
@@ -281,7 +281,7 @@ class TestLoginEndpoint:
|
||||
# Verify template was rendered with OAuth enabled
|
||||
mock_templates.TemplateResponse.assert_called_once()
|
||||
call_args = mock_templates.TemplateResponse.call_args
|
||||
context = call_args[0][1]
|
||||
context = call_args.kwargs["context"]
|
||||
assert context["show_oauth"] is True
|
||||
assert context["oauth_provider_name"] == "Test SSO"
|
||||
|
||||
|
||||
@@ -69,8 +69,9 @@ class TestViewsBase:
|
||||
context = {"request": req}
|
||||
template_response_with_version("template.html", context)
|
||||
|
||||
args, _ = mock_orig.call_args
|
||||
assert args[1].get("csrf_token") == "my-csrf"
|
||||
args, kwargs = mock_orig.call_args
|
||||
context = kwargs.get("context", {})
|
||||
assert context.get("csrf_token") == "my-csrf"
|
||||
|
||||
def test_kwargs_context_no_request(self):
|
||||
"""Test kwargs context path when request is not in context."""
|
||||
|
||||
@@ -55,8 +55,8 @@ class TestDarkModeTemplateInjection:
|
||||
|
||||
captured = {}
|
||||
|
||||
def fake_original(name, ctx, **kw):
|
||||
captured.update(ctx)
|
||||
def fake_original(request_obj, name, context=None, **kw):
|
||||
captured.update(context or {})
|
||||
|
||||
with patch("app.views.base.original_template_response", side_effect=fake_original):
|
||||
mock_request = MagicMock()
|
||||
@@ -73,8 +73,8 @@ class TestDarkModeTemplateInjection:
|
||||
|
||||
captured = {}
|
||||
|
||||
def fake_original(name, ctx, **kw):
|
||||
captured.update(ctx)
|
||||
def fake_original(request_obj, name, context=None, **kw):
|
||||
captured.update(context or {})
|
||||
|
||||
with patch("app.views.base.original_template_response", side_effect=fake_original):
|
||||
mock_request = MagicMock()
|
||||
|
||||
@@ -0,0 +1,145 @@
|
||||
"""Tests for frontend build configuration and Docker build consistency.
|
||||
|
||||
Validates that the frontend build toolchain (Tailwind CSS) is correctly
|
||||
configured in package.json and that the Dockerfile installs all required
|
||||
dependencies for the build step.
|
||||
"""
|
||||
|
||||
import json
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
# Resolve the project root from the test file location
|
||||
PROJECT_ROOT = Path(__file__).resolve().parent.parent
|
||||
FRONTEND_DIR = PROJECT_ROOT / "frontend"
|
||||
DOCKERFILE_PATH = PROJECT_ROOT / "Dockerfile"
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestFrontendPackageJson:
|
||||
"""Validate frontend/package.json structure and scripts."""
|
||||
|
||||
def test_package_json_exists(self) -> None:
|
||||
"""package.json must exist in the frontend directory."""
|
||||
pkg_path = FRONTEND_DIR / "package.json"
|
||||
assert pkg_path.exists(), "frontend/package.json not found"
|
||||
|
||||
def test_package_json_is_valid_json(self) -> None:
|
||||
"""package.json must be parseable JSON."""
|
||||
pkg_path = FRONTEND_DIR / "package.json"
|
||||
data = json.loads(pkg_path.read_text(encoding="utf-8"))
|
||||
assert isinstance(data, dict), "package.json must be a JSON object"
|
||||
|
||||
def test_build_script_defined(self) -> None:
|
||||
"""A 'build' script must be defined in package.json."""
|
||||
pkg_path = FRONTEND_DIR / "package.json"
|
||||
data = json.loads(pkg_path.read_text(encoding="utf-8"))
|
||||
scripts = data.get("scripts", {})
|
||||
assert "build" in scripts, "Missing 'build' script in package.json"
|
||||
|
||||
def test_build_script_uses_tailwindcss(self) -> None:
|
||||
"""The build script must invoke the tailwindcss CLI."""
|
||||
pkg_path = FRONTEND_DIR / "package.json"
|
||||
data = json.loads(pkg_path.read_text(encoding="utf-8"))
|
||||
build_cmd = data["scripts"]["build"]
|
||||
assert "tailwindcss" in build_cmd, f"Build script does not reference tailwindcss: {build_cmd}"
|
||||
|
||||
def test_tailwindcss_listed_as_dependency(self) -> None:
|
||||
"""tailwindcss must be listed in dependencies or devDependencies."""
|
||||
pkg_path = FRONTEND_DIR / "package.json"
|
||||
data = json.loads(pkg_path.read_text(encoding="utf-8"))
|
||||
deps = data.get("dependencies", {})
|
||||
dev_deps = data.get("devDependencies", {})
|
||||
all_deps = {**deps, **dev_deps}
|
||||
assert "tailwindcss" in all_deps, "tailwindcss is not listed in dependencies or devDependencies"
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestFrontendBuildAssets:
|
||||
"""Validate that required frontend build source files exist."""
|
||||
|
||||
def test_input_css_exists(self) -> None:
|
||||
"""The Tailwind CSS input file must exist."""
|
||||
input_css = FRONTEND_DIR / "input.css"
|
||||
assert input_css.exists(), "frontend/input.css not found"
|
||||
|
||||
def test_input_css_has_tailwind_directives(self) -> None:
|
||||
"""input.css must include Tailwind CSS directives."""
|
||||
input_css = FRONTEND_DIR / "input.css"
|
||||
content = input_css.read_text(encoding="utf-8")
|
||||
assert "@tailwind base" in content, "Missing @tailwind base directive"
|
||||
assert "@tailwind components" in content, "Missing @tailwind components directive"
|
||||
assert "@tailwind utilities" in content, "Missing @tailwind utilities directive"
|
||||
|
||||
def test_tailwind_config_exists(self) -> None:
|
||||
"""tailwind.config.js must exist in the frontend directory."""
|
||||
config_path = FRONTEND_DIR / "tailwind.config.js"
|
||||
assert config_path.exists(), "frontend/tailwind.config.js not found"
|
||||
|
||||
def test_package_lock_exists(self) -> None:
|
||||
"""package-lock.json must exist for reproducible installs."""
|
||||
lock_path = FRONTEND_DIR / "package-lock.json"
|
||||
assert lock_path.exists(), "frontend/package-lock.json not found"
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestDockerfileFrontendBuilder:
|
||||
"""Validate the Dockerfile frontend-builder stage installs build dependencies."""
|
||||
|
||||
def test_dockerfile_exists(self) -> None:
|
||||
"""Production Dockerfile must exist at the project root."""
|
||||
assert DOCKERFILE_PATH.exists(), "Dockerfile not found at project root"
|
||||
|
||||
def test_dockerfile_has_frontend_builder_stage(self) -> None:
|
||||
"""Dockerfile must define a frontend-builder stage."""
|
||||
content = DOCKERFILE_PATH.read_text(encoding="utf-8")
|
||||
assert "AS frontend-builder" in content, "Dockerfile does not define a frontend-builder stage"
|
||||
|
||||
def test_dockerfile_npm_ci_does_not_omit_dev(self) -> None:
|
||||
"""npm ci must NOT use --omit=dev in the frontend-builder stage.
|
||||
|
||||
The tailwindcss CLI is a devDependency required at build time.
|
||||
Using --omit=dev would skip installing it, causing the build to
|
||||
fail with 'tailwindcss: not found'.
|
||||
"""
|
||||
content = DOCKERFILE_PATH.read_text(encoding="utf-8")
|
||||
|
||||
# Extract the frontend-builder stage content
|
||||
# Look for the stage start and the next stage (or end of file)
|
||||
stage_pattern = re.compile(
|
||||
r"FROM\s+\S+\s+AS\s+frontend-builder\b(.*?)(?=FROM\s|\Z)",
|
||||
re.DOTALL,
|
||||
)
|
||||
match = stage_pattern.search(content)
|
||||
assert match is not None, "Could not find frontend-builder stage in Dockerfile"
|
||||
|
||||
stage_content = match.group(1)
|
||||
assert "--omit=dev" not in stage_content, (
|
||||
"Dockerfile frontend-builder stage uses 'npm ci --omit=dev' which "
|
||||
"excludes tailwindcss (a devDependency) needed for the build step. "
|
||||
"Use 'npm ci' instead to install all dependencies."
|
||||
)
|
||||
|
||||
def test_dockerfile_runs_npm_build(self) -> None:
|
||||
"""Dockerfile frontend-builder stage must run npm run build."""
|
||||
content = DOCKERFILE_PATH.read_text(encoding="utf-8")
|
||||
|
||||
stage_pattern = re.compile(
|
||||
r"FROM\s+\S+\s+AS\s+frontend-builder\b(.*?)(?=FROM\s|\Z)",
|
||||
re.DOTALL,
|
||||
)
|
||||
match = stage_pattern.search(content)
|
||||
assert match is not None, "Could not find frontend-builder stage in Dockerfile"
|
||||
|
||||
stage_content = match.group(1)
|
||||
assert "npm run build" in stage_content, "Dockerfile frontend-builder stage does not run 'npm run build'"
|
||||
|
||||
def test_dockerfile_copies_compiled_css(self) -> None:
|
||||
"""Dockerfile must copy the compiled styles.css from the frontend-builder stage."""
|
||||
content = DOCKERFILE_PATH.read_text(encoding="utf-8")
|
||||
assert "COPY --from=frontend-builder" in content, (
|
||||
"Dockerfile does not copy assets from the frontend-builder stage"
|
||||
)
|
||||
assert "styles.css" in content, "Dockerfile does not reference the compiled styles.css"
|
||||
@@ -345,7 +345,7 @@ class TestLoginPageSocialProviders:
|
||||
|
||||
mock_templates.TemplateResponse.assert_called_once()
|
||||
call_args = mock_templates.TemplateResponse.call_args
|
||||
context = call_args[0][1]
|
||||
context = call_args.kwargs.get("context", {})
|
||||
assert context["social_providers"] == mock_providers
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -371,7 +371,7 @@ class TestLoginPageSocialProviders:
|
||||
|
||||
mock_templates.TemplateResponse.assert_called_once()
|
||||
call_args = mock_templates.TemplateResponse.call_args
|
||||
context = call_args[0][1]
|
||||
context = call_args.kwargs.get("context", {})
|
||||
assert context["social_providers"] == {}
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user