mirror of
https://github.com/Sea-Haven-Industries/meal-order-manager.git
synced 2026-10-01 11:43:12 +00:00
fix(submit-order): bill from Dynamo menu retail, not client JSON
Load authoritative meal prices from get_menu(week); reject unknown meal names and return 503 when the menu has no priced meals. Use meal_name in the pricing loop to avoid shadowing the employee name. Adds regression tests for tampering, unknown meals, and empty menu meals. Co-authored-by: Adam Moussa <amoussa1229@users.noreply.github.com>
This commit is contained in:
parent
0e26bda83c
commit
b1530c04a2
2 changed files with 244 additions and 11 deletions
|
|
@ -12,7 +12,14 @@ from zoneinfo import ZoneInfo
|
|||
|
||||
import boto3
|
||||
|
||||
from shared.db import current_week, get_form_status, get_roster, get_settings, put_order
|
||||
from shared.db import (
|
||||
current_week,
|
||||
get_form_status,
|
||||
get_menu,
|
||||
get_roster,
|
||||
get_settings,
|
||||
put_order,
|
||||
)
|
||||
from shared.secrets import get_parameter, get_secret
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
|
@ -61,6 +68,19 @@ def _google_auth_configured() -> bool:
|
|||
return bool(os.environ.get("GOOGLE_CLIENT_ID_PARAM", ""))
|
||||
|
||||
|
||||
def _official_menu_retail_by_name(week: str) -> dict[str, Decimal]:
|
||||
"""Map meal name -> retail price from Dynamo menu (authoritative for billing)."""
|
||||
row = get_menu(week)
|
||||
meals = (row or {}).get("meals") or []
|
||||
out: dict[str, Decimal] = {}
|
||||
for m in meals:
|
||||
name = (m.get("name") or "").strip()
|
||||
if not name or m.get("price") is None:
|
||||
continue
|
||||
out[name] = Decimal(str(m["price"]))
|
||||
return out
|
||||
|
||||
|
||||
def _get_google_client_id() -> str:
|
||||
global _google_client_id, _google_client_id_ts
|
||||
now = time.monotonic()
|
||||
|
|
@ -218,6 +238,18 @@ def handle_submit(event):
|
|||
|
||||
filtered_items = [i for i in items if i.get("quantity", 0) > 0]
|
||||
|
||||
official_retail = _official_menu_retail_by_name(week)
|
||||
if not official_retail:
|
||||
logger.error("Week %s: menu has no priced meals; refusing order", week)
|
||||
return response(503, {"error": "Menu temporarily unavailable"})
|
||||
for item in filtered_items:
|
||||
meal_name = (item.get("name") or "").strip()
|
||||
if meal_name not in official_retail:
|
||||
return response(
|
||||
400,
|
||||
{"error": "One or more meals are not on this week's menu"},
|
||||
)
|
||||
|
||||
# --- Price calculation using Decimal for financial precision ---
|
||||
# Rounding approach (two-step intermediate rounding):
|
||||
# 1. bulk_price = retail * bulk_mult, rounded to 2 decimal places
|
||||
|
|
@ -231,7 +263,8 @@ def handle_submit(event):
|
|||
subsidy_mult = Decimal("1") - (subsidy_pct / Decimal("100"))
|
||||
|
||||
for item in filtered_items:
|
||||
retail = Decimal(str(item.get("retail_price", item.get("price", 0)) or 0))
|
||||
meal_name = (item.get("name") or "").strip()
|
||||
retail = official_retail[meal_name]
|
||||
qty = Decimal(str(item.get("quantity", 0)))
|
||||
# Step 1: apply bulk discount and round
|
||||
bulk_price = (retail * bulk_mult).quantize(TWO_PLACES, rounding=ROUND_HALF_UP)
|
||||
|
|
|
|||
|
|
@ -111,6 +111,16 @@ def _make_items(retail_prices_and_qtys):
|
|||
return items
|
||||
|
||||
|
||||
def _menu_doc_from_retail_pairs(pairs):
|
||||
"""Dynamo-style menu document matching _make_items retail pairs."""
|
||||
return {
|
||||
"meals": [
|
||||
{"name": f"Meal {i + 1}", "price": price}
|
||||
for i, (price, _) in enumerate(pairs)
|
||||
]
|
||||
}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Module-level patches that must be active before the handler is imported
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
@ -158,8 +168,16 @@ def _reset_shared_caches():
|
|||
)
|
||||
@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY)
|
||||
@patch("submit_order_handler._get_google_client_id", return_value="")
|
||||
@patch("submit_order_handler.get_menu")
|
||||
def test_discount_two_step_rounding(
|
||||
mock_gcid, mock_secret, mock_settings, mock_week, mock_status, mock_put, mock_lam
|
||||
mock_get_menu,
|
||||
mock_gcid,
|
||||
mock_secret,
|
||||
mock_settings,
|
||||
mock_week,
|
||||
mock_status,
|
||||
mock_put,
|
||||
mock_lam,
|
||||
):
|
||||
"""Retail $10.25, 10% bulk, 50% subsidy.
|
||||
|
||||
|
|
@ -170,6 +188,7 @@ def test_discount_two_step_rounding(
|
|||
from submit_order_handler import lambda_handler
|
||||
|
||||
items = _make_items([(10.25, 3)])
|
||||
mock_get_menu.return_value = _menu_doc_from_retail_pairs([(10.25, 3)])
|
||||
event = _submit_event(items)
|
||||
result = lambda_handler(event, None)
|
||||
status, body = _parse_response(result)
|
||||
|
|
@ -200,8 +219,16 @@ def test_discount_two_step_rounding(
|
|||
)
|
||||
@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY)
|
||||
@patch("submit_order_handler._get_google_client_id", return_value="")
|
||||
@patch("submit_order_handler.get_menu")
|
||||
def test_discount_rounding_half_up_boundary(
|
||||
mock_gcid, mock_secret, mock_settings, mock_week, mock_status, mock_put, mock_lam
|
||||
mock_get_menu,
|
||||
mock_gcid,
|
||||
mock_secret,
|
||||
mock_settings,
|
||||
mock_week,
|
||||
mock_status,
|
||||
mock_put,
|
||||
mock_lam,
|
||||
):
|
||||
"""Retail $10.05, 50% bulk, 0% subsidy.
|
||||
|
||||
|
|
@ -210,6 +237,7 @@ def test_discount_rounding_half_up_boundary(
|
|||
from submit_order_handler import lambda_handler
|
||||
|
||||
items = _make_items([(10.05, 1)])
|
||||
mock_get_menu.return_value = _menu_doc_from_retail_pairs([(10.05, 1)])
|
||||
event = _submit_event(items)
|
||||
result = lambda_handler(event, None)
|
||||
status, body = _parse_response(result)
|
||||
|
|
@ -237,13 +265,22 @@ def test_discount_rounding_half_up_boundary(
|
|||
)
|
||||
@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY)
|
||||
@patch("submit_order_handler._get_google_client_id", return_value="")
|
||||
@patch("submit_order_handler.get_menu")
|
||||
def test_discount_clamping(
|
||||
mock_gcid, mock_secret, mock_settings, mock_week, mock_status, mock_put, mock_lam
|
||||
mock_get_menu,
|
||||
mock_gcid,
|
||||
mock_secret,
|
||||
mock_settings,
|
||||
mock_week,
|
||||
mock_status,
|
||||
mock_put,
|
||||
mock_lam,
|
||||
):
|
||||
"""Negative bulk discount clamped to 0, subsidy >100 clamped to 100 (free)."""
|
||||
from submit_order_handler import lambda_handler
|
||||
|
||||
items = _make_items([(20.00, 1)])
|
||||
mock_get_menu.return_value = _menu_doc_from_retail_pairs([(20.00, 1)])
|
||||
event = _submit_event(items)
|
||||
result = lambda_handler(event, None)
|
||||
status, body = _parse_response(result)
|
||||
|
|
@ -274,13 +311,22 @@ def test_discount_clamping(
|
|||
)
|
||||
@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY)
|
||||
@patch("submit_order_handler._get_google_client_id", return_value="")
|
||||
@patch("submit_order_handler.get_menu")
|
||||
def test_discount_both_zero(
|
||||
mock_gcid, mock_secret, mock_settings, mock_week, mock_status, mock_put, mock_lam
|
||||
mock_get_menu,
|
||||
mock_gcid,
|
||||
mock_secret,
|
||||
mock_settings,
|
||||
mock_week,
|
||||
mock_status,
|
||||
mock_put,
|
||||
mock_lam,
|
||||
):
|
||||
"""When both discounts are 0%, emp_price equals retail price."""
|
||||
from submit_order_handler import lambda_handler
|
||||
|
||||
items = _make_items([(15.99, 2)])
|
||||
mock_get_menu.return_value = _menu_doc_from_retail_pairs([(15.99, 2)])
|
||||
event = _submit_event(items)
|
||||
result = lambda_handler(event, None)
|
||||
status, body = _parse_response(result)
|
||||
|
|
@ -314,8 +360,16 @@ def test_discount_both_zero(
|
|||
)
|
||||
@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY)
|
||||
@patch("submit_order_handler._get_google_client_id", return_value="")
|
||||
@patch("submit_order_handler.get_menu")
|
||||
def test_total_summation_multiple_items(
|
||||
mock_gcid, mock_secret, mock_settings, mock_week, mock_status, mock_put, mock_lam
|
||||
mock_get_menu,
|
||||
mock_gcid,
|
||||
mock_secret,
|
||||
mock_settings,
|
||||
mock_week,
|
||||
mock_status,
|
||||
mock_put,
|
||||
mock_lam,
|
||||
):
|
||||
"""Total is the sum of individually rounded subtotals, not a global multiply.
|
||||
|
||||
|
|
@ -326,6 +380,7 @@ def test_total_summation_multiple_items(
|
|||
from submit_order_handler import lambda_handler
|
||||
|
||||
items = _make_items([(10.00, 2), (7.33, 1)])
|
||||
mock_get_menu.return_value = _menu_doc_from_retail_pairs([(10.00, 2), (7.33, 1)])
|
||||
event = _submit_event(items)
|
||||
result = lambda_handler(event, None)
|
||||
status, body = _parse_response(result)
|
||||
|
|
@ -688,7 +743,9 @@ def test_ssm_failure_fails_closed(
|
|||
)
|
||||
@patch("submit_order_handler.urllib.request.urlopen")
|
||||
@patch("submit_order_handler._google_auth_configured", return_value=True)
|
||||
@patch("submit_order_handler.get_menu")
|
||||
def test_google_token_valid_success(
|
||||
mock_get_menu,
|
||||
mock_gac,
|
||||
mock_urlopen,
|
||||
mock_gcid,
|
||||
|
|
@ -716,6 +773,7 @@ def test_google_token_valid_success(
|
|||
mock_urlopen.return_value = mock_resp
|
||||
|
||||
items = _make_items([(10.00, 1)])
|
||||
mock_get_menu.return_value = _menu_doc_from_retail_pairs([(10.00, 1)])
|
||||
# Body has different name/email — should be overridden by token
|
||||
event = _submit_event(
|
||||
items,
|
||||
|
|
@ -747,13 +805,22 @@ def test_google_token_valid_success(
|
|||
)
|
||||
@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY)
|
||||
@patch("submit_order_handler._get_google_client_id", return_value="")
|
||||
@patch("submit_order_handler.get_menu")
|
||||
def test_manual_fallback_when_google_not_configured(
|
||||
mock_gcid, mock_secret, mock_settings, mock_week, mock_status, mock_put, mock_lam
|
||||
mock_get_menu,
|
||||
mock_gcid,
|
||||
mock_secret,
|
||||
mock_settings,
|
||||
mock_week,
|
||||
mock_status,
|
||||
mock_put,
|
||||
mock_lam,
|
||||
):
|
||||
"""No Google client ID configured -> manual name/email accepted and order saved."""
|
||||
from submit_order_handler import lambda_handler
|
||||
|
||||
items = _make_items([(12.00, 1)])
|
||||
mock_get_menu.return_value = _menu_doc_from_retail_pairs([(12.00, 1)])
|
||||
event = _submit_event(
|
||||
items, employee_name="Manual User", employee_email="manual@seahavenind.com"
|
||||
)
|
||||
|
|
@ -814,13 +881,22 @@ def test_submit_invalid_api_key(
|
|||
)
|
||||
@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY)
|
||||
@patch("submit_order_handler._get_google_client_id", return_value="")
|
||||
@patch("submit_order_handler.get_menu")
|
||||
def test_slug_from_email(
|
||||
mock_gcid, mock_secret, mock_settings, mock_week, mock_status, mock_put, mock_lam
|
||||
mock_get_menu,
|
||||
mock_gcid,
|
||||
mock_secret,
|
||||
mock_settings,
|
||||
mock_week,
|
||||
mock_status,
|
||||
mock_put,
|
||||
mock_lam,
|
||||
):
|
||||
"""'Adam.Moussa@seahavenind.com' -> slug 'adam.moussa@seahavenind.com'."""
|
||||
from submit_order_handler import lambda_handler
|
||||
|
||||
items = _make_items([(10.00, 1)])
|
||||
mock_get_menu.return_value = _menu_doc_from_retail_pairs([(10.00, 1)])
|
||||
event = _submit_event(
|
||||
items, employee_name="Adam Moussa", employee_email="Adam.Moussa@seahavenind.com"
|
||||
)
|
||||
|
|
@ -846,14 +922,23 @@ def test_slug_from_email(
|
|||
)
|
||||
@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY)
|
||||
@patch("submit_order_handler._get_google_client_id", return_value="")
|
||||
@patch("submit_order_handler.get_menu")
|
||||
def test_slug_edge_cases(
|
||||
mock_gcid, mock_secret, mock_settings, mock_week, mock_status, mock_put, mock_lam
|
||||
mock_get_menu,
|
||||
mock_gcid,
|
||||
mock_secret,
|
||||
mock_settings,
|
||||
mock_week,
|
||||
mock_status,
|
||||
mock_put,
|
||||
mock_lam,
|
||||
):
|
||||
"""Slug uses full lowercase email, preserving uniqueness across domains."""
|
||||
from submit_order_handler import lambda_handler
|
||||
|
||||
# Test 1: full email preserved
|
||||
items = _make_items([(10.00, 1)])
|
||||
mock_get_menu.return_value = _menu_doc_from_retail_pairs([(10.00, 1)])
|
||||
event = _submit_event(
|
||||
items,
|
||||
employee_name="First Middle Last",
|
||||
|
|
@ -867,6 +952,7 @@ def test_slug_edge_cases(
|
|||
|
||||
# Test 2: different domains produce different slugs (no collision)
|
||||
mock_put.reset_mock()
|
||||
mock_get_menu.return_value = _menu_doc_from_retail_pairs([(10.00, 1)])
|
||||
event = _submit_event(
|
||||
items, employee_name="Bob Smith", employee_email="Bob@other.com"
|
||||
)
|
||||
|
|
@ -1139,6 +1225,111 @@ def test_submit_no_menu(
|
|||
mock_put.assert_not_called()
|
||||
|
||||
|
||||
@patch("submit_order_handler._lambda")
|
||||
@patch("submit_order_handler.put_order")
|
||||
@patch("submit_order_handler.get_form_status", return_value="open")
|
||||
@patch("submit_order_handler.current_week", return_value="2026-W20")
|
||||
@patch(
|
||||
"submit_order_handler.get_settings",
|
||||
return_value={"bulk_discount_percent": 0, "company_subsidy_percent": 0},
|
||||
)
|
||||
@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY)
|
||||
@patch("submit_order_handler._get_google_client_id", return_value="")
|
||||
@patch("submit_order_handler.get_menu")
|
||||
def test_unknown_meal_returns_400(
|
||||
mock_get_menu,
|
||||
mock_gcid,
|
||||
mock_secret,
|
||||
mock_settings,
|
||||
mock_week,
|
||||
mock_status,
|
||||
mock_put,
|
||||
mock_lam,
|
||||
):
|
||||
"""Line items must match menu meal names (reject client-injected SKUs)."""
|
||||
from submit_order_handler import lambda_handler
|
||||
|
||||
mock_get_menu.return_value = _menu_doc_from_retail_pairs([(10.00, 1)])
|
||||
event = _submit_event(
|
||||
[{"name": "Totally Fake Meal", "retail_price": 1.0, "quantity": 1}]
|
||||
)
|
||||
result = lambda_handler(event, None)
|
||||
status, body = _parse_response(result)
|
||||
|
||||
assert status == 400, f"Expected 400, got {status}: {body}"
|
||||
assert "not on this week's menu" in body["error"].lower(), body["error"]
|
||||
mock_put.assert_not_called()
|
||||
|
||||
|
||||
@patch("submit_order_handler._lambda")
|
||||
@patch("submit_order_handler.put_order")
|
||||
@patch("submit_order_handler.get_form_status", return_value="open")
|
||||
@patch("submit_order_handler.current_week", return_value="2026-W20")
|
||||
@patch(
|
||||
"submit_order_handler.get_settings",
|
||||
return_value={"bulk_discount_percent": 0, "company_subsidy_percent": 0},
|
||||
)
|
||||
@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY)
|
||||
@patch("submit_order_handler._get_google_client_id", return_value="")
|
||||
@patch("submit_order_handler.get_menu")
|
||||
def test_retail_price_from_menu_not_request_body(
|
||||
mock_get_menu,
|
||||
mock_gcid,
|
||||
mock_secret,
|
||||
mock_settings,
|
||||
mock_week,
|
||||
mock_status,
|
||||
mock_put,
|
||||
mock_lam,
|
||||
):
|
||||
"""Billing uses Dynamo menu retail, not client-supplied retail_price."""
|
||||
from submit_order_handler import lambda_handler
|
||||
|
||||
mock_get_menu.return_value = {"meals": [{"name": "Meal 1", "price": 100.0}]}
|
||||
items = [{"name": "Meal 1", "retail_price": 0.01, "quantity": 1}]
|
||||
event = _submit_event(items)
|
||||
result = lambda_handler(event, None)
|
||||
status, body = _parse_response(result)
|
||||
|
||||
assert status == 200, f"Expected 200, got {status}: {body}"
|
||||
saved = mock_put.call_args[0][2]
|
||||
assert saved["items"][0]["retail_price"] == 100.0, saved["items"][0]
|
||||
assert saved["total"] == 100.0, saved["total"]
|
||||
|
||||
|
||||
@patch("submit_order_handler._lambda")
|
||||
@patch("submit_order_handler.put_order")
|
||||
@patch("submit_order_handler.get_form_status", return_value="open")
|
||||
@patch("submit_order_handler.current_week", return_value="2026-W20")
|
||||
@patch(
|
||||
"submit_order_handler.get_settings",
|
||||
return_value={"bulk_discount_percent": 0, "company_subsidy_percent": 0},
|
||||
)
|
||||
@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY)
|
||||
@patch("submit_order_handler._get_google_client_id", return_value="")
|
||||
@patch("submit_order_handler.get_menu")
|
||||
def test_menu_missing_priced_meals_returns_503(
|
||||
mock_get_menu,
|
||||
mock_gcid,
|
||||
mock_secret,
|
||||
mock_settings,
|
||||
mock_week,
|
||||
mock_status,
|
||||
mock_put,
|
||||
mock_lam,
|
||||
):
|
||||
from submit_order_handler import lambda_handler
|
||||
|
||||
mock_get_menu.return_value = {"meals": []}
|
||||
items = _make_items([(10.00, 1)])
|
||||
result = lambda_handler(_submit_event(items), None)
|
||||
status, body = _parse_response(result)
|
||||
|
||||
assert status == 503, f"Expected 503, got {status}: {body}"
|
||||
assert "unavailable" in body["error"].lower(), body["error"]
|
||||
mock_put.assert_not_called()
|
||||
|
||||
|
||||
# ===========================================================================
|
||||
# RELIABILITY (High)
|
||||
# ===========================================================================
|
||||
|
|
@ -1154,8 +1345,16 @@ def test_submit_no_menu(
|
|||
)
|
||||
@patch("submit_order_handler.get_secret", return_value=TEST_API_KEY)
|
||||
@patch("submit_order_handler._get_google_client_id", return_value="")
|
||||
@patch("submit_order_handler.get_menu")
|
||||
def test_slack_failure_does_not_fail_order(
|
||||
mock_gcid, mock_secret, mock_settings, mock_week, mock_status, mock_put, mock_lam
|
||||
mock_get_menu,
|
||||
mock_gcid,
|
||||
mock_secret,
|
||||
mock_settings,
|
||||
mock_week,
|
||||
mock_status,
|
||||
mock_put,
|
||||
mock_lam,
|
||||
):
|
||||
"""Lambda invoke for Slack notification raises, order still saved, returns 200."""
|
||||
from submit_order_handler import lambda_handler
|
||||
|
|
@ -1163,6 +1362,7 @@ def test_slack_failure_does_not_fail_order(
|
|||
mock_lam.invoke.side_effect = Exception("Lambda invoke failed: connection timeout")
|
||||
|
||||
items = _make_items([(10.00, 1)])
|
||||
mock_get_menu.return_value = _menu_doc_from_retail_pairs([(10.00, 1)])
|
||||
event = _submit_event(items)
|
||||
result = lambda_handler(event, None)
|
||||
status, body = _parse_response(result)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue