Skip to content

Commit ff80141

Browse files
jssmithjssmith
committed
ui cleanup
1 parent 231a8de commit ff80141

File tree

7 files changed

+68
-41
lines changed

7 files changed

+68
-41
lines changed

‎expense/activities.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,9 @@ async def payment_activity(expense_id: str) -> None:
9898

9999
client = get_http_client()
100100
try:
101-
response = await client.get(
101+
response = await client.post(
102102
f"{EXPENSE_SERVER_HOST_PORT}/action",
103-
params={"is_api_call": "true", "type": "payment", "id": expense_id},
103+
data={"is_api_call": "true", "type": "payment", "id": expense_id},
104104
)
105105
response.raise_for_status()
106106
except httpx.HTTPStatusError as e:

‎expense/ui.py‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
import uvicorn
66
from fastapi import FastAPI, Form, Query
7-
from fastapi.responses import HTMLResponse, PlainTextResponse
7+
from fastapi.responses import HTMLResponse, PlainTextResponse, RedirectResponse
88
from temporalio.client import Client
99

1010
from expense import EXPENSE_SERVER_HOST, EXPENSE_SERVER_PORT
@@ -43,24 +43,28 @@ async def list_handler():
4343
state = all_expenses[expense_id]
4444
action_link = ""
4545
if state == ExpenseState.CREATED:
46-
action_link = f"""
47-
<a href="/action?type=approve&id={expense_id}">
48-
<button style="background-color:#4CAF50;">APPROVE</button>
49-
</a>
50-
&nbsp;&nbsp;
51-
<a href="/action?type=reject&id={expense_id}">
52-
<button style="background-color:#f44336;">REJECT</button>
53-
</a>
54-
"""
46+
action_link = (
47+
f'<form method="post" action="/action" style="display:inline;">'
48+
f'<input type="hidden" name="type" value="approve">'
49+
f'<input type="hidden" name="id" value="{expense_id}">'
50+
'<button type="submit" style="background-color:#4CAF50;">APPROVE</button>'
51+
"</form>"
52+
"&nbsp;&nbsp;"
53+
f'<form method="post" action="/action" style="display:inline;">'
54+
f'<input type="hidden" name="type" value="reject">'
55+
f'<input type="hidden" name="id" value="{expense_id}">'
56+
'<button type="submit" style="background-color:#f44336;">REJECT</button>'
57+
"</form>"
58+
)
5559
html += f"<tr><td>{expense_id}</td><td>{state}</td><td>{action_link}</td></tr>"
5660

5761
html += "</table>"
5862
return html
5963

6064

61-
@app.get("/action", response_class=HTMLResponse)
65+
@app.post("/action")
6266
async def action_handler(
63-
type: str = Query(...), id: str = Query(...), is_api_call: str = Query("false")
67+
type: str = Form(...), id: str = Form(...), is_api_call: str = Form("false")
6468
):
6569
if id not in all_expenses:
6670
if is_api_call == "true":
@@ -102,7 +106,7 @@ async def action_handler(
102106
await notify_expense_state_change(id, all_expenses[id])
103107

104108
print(f"Set state for {id} from {old_state} to {all_expenses[id]}")
105-
return await list_handler()
109+
return RedirectResponse(url="/list", status_code=303)
106110

107111

108112
@app.get("/create")

‎tests/expense/UI_SPECIFICATION.md‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,13 @@ All endpoints use FastAPI's automatic parameter validation:
3838
- Only CREATED expenses show action buttons
3939
- Expenses are displayed in sorted order by ID
4040

41-
### 2. Action Handler (`GET /action`)
41+
### 2. Action Handler (`POST /action`)
4242
**Purpose**: Process expense state changes (approve/reject/payment)
4343

4444
**Parameters**:
45-
- `type` (required): Action type - "approve", "reject", or "payment"
46-
- `id` (required): Expense ID
47-
- `is_api_call` (optional): "true" for API calls, "false" for UI calls
45+
- `type` (required): Action type - "approve", "reject", or "payment" (form data)
46+
- `id` (required): Expense ID (form data)
47+
- `is_api_call` (optional): "true" for API calls, "false" for UI calls (form data)
4848

4949
**Business Rules**:
5050
- `approve`: Changes CREATED → APPROVED
@@ -54,7 +54,7 @@ All endpoints use FastAPI's automatic parameter validation:
5454
- Invalid action types return HTTP 200 with error message in response body
5555
- State changes from CREATED to APPROVED/REJECTED trigger workflow notifications
5656
- API calls return "SUCCEED" on success
57-
- UI calls redirect to list view after success
57+
- UI calls redirect to list view after success (HTTP 303 redirect)
5858

5959
**Error Handling**:
6060
- API calls return HTTP 200 with "ERROR:INVALID_ID" or "ERROR:INVALID_TYPE" in response body

‎tests/expense/WORKFLOW_SPECIFICATION.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ The activity demonstrates a signal-based human-in-the-loop pattern. It simply re
123123
**Business Rules**:
124124
- Only called for approved expenses
125125
- Validate expense_id is not empty
126-
- HTTP GET to `/action?is_api_call=true&type=payment&id={expense_id}`
126+
- HTTP POST to `/action` with form data: `is_api_call=true`, `type=payment`, `id={expense_id}`
127127
- Success condition: Response body equals "SUCCEED"
128128

129129
**Error Handling**:

‎tests/expense/test_expense_activities.py‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -144,16 +144,16 @@ async def test_payment_activity_success(self, activity_env):
144144
mock_response.raise_for_status = AsyncMock()
145145

146146
mock_client_instance = AsyncMock()
147-
mock_client_instance.get.return_value = mock_response
147+
mock_client_instance.post.return_value = mock_response
148148
mock_get_client.return_value = mock_client_instance
149149

150150
# Execute activity
151151
result = await activity_env.run(payment_activity, "test-expense-123")
152152

153153
# Verify HTTP call
154-
mock_client_instance.get.assert_called_once_with(
154+
mock_client_instance.post.assert_called_once_with(
155155
f"{EXPENSE_SERVER_HOST_PORT}/action",
156-
params={
156+
data={
157157
"is_api_call": "true",
158158
"type": "payment",
159159
"id": "test-expense-123",
@@ -177,7 +177,7 @@ async def test_payment_activity_payment_failure(self, activity_env):
177177
mock_response.raise_for_status = AsyncMock()
178178

179179
mock_client_instance = AsyncMock()
180-
mock_client_instance.get.return_value = mock_response
180+
mock_client_instance.post.return_value = mock_response
181181
mock_get_client.return_value = mock_client_instance
182182

183183
with pytest.raises(Exception, match="ERROR:INSUFFICIENT_FUNDS"):

‎tests/expense/test_http_client_lifecycle.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
"""
33
Simple test script to verify HTTP client lifecycle management.
44
"""
5+
56
import asyncio
67
import os
78
import sys

‎tests/expense/test_ui.py‎

Lines changed: 38 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,10 @@ def test_list_view_action_buttons_only_for_created(self, client):
6262

6363
# Count actual button elements - should only be for the CREATED expense
6464
approve_count = html.count(
65-
'<button style="background-color:#4CAF50;">APPROVE</button>'
65+
'<button type="submit" style="background-color:#4CAF50;">APPROVE</button>'
6666
)
6767
reject_count = html.count(
68-
'<button style="background-color:#f44336;">REJECT</button>'
68+
'<button type="submit" style="background-color:#f44336;">REJECT</button>'
6969
)
7070
assert approve_count == 1
7171
assert reject_count == 1
@@ -119,19 +119,23 @@ def test_action_approve_ui(self, client):
119119
all_expenses["test-expense"] = ExpenseState.CREATED
120120

121121
with patch("expense.ui.notify_expense_state_change") as mock_notify:
122-
response = client.get("/action?type=approve&id=test-expense")
122+
response = client.post(
123+
"/action", data={"type": "approve", "id": "test-expense"}
124+
)
123125
assert response.status_code == 200
124126
assert all_expenses["test-expense"] == ExpenseState.APPROVED
125-
assert "SAMPLE EXPENSE SYSTEM" in response.text # Should show list view
127+
# Should redirect to list view
128+
assert response.url.path == "/list"
126129
mock_notify.assert_called_once_with("test-expense", ExpenseState.APPROVED)
127130

128131
def test_action_approve_api(self, client):
129132
"""Test approve action via API"""
130133
all_expenses["test-expense"] = ExpenseState.CREATED
131134

132135
with patch("expense.ui.notify_expense_state_change") as mock_notify:
133-
response = client.get(
134-
"/action?type=approve&id=test-expense&is_api_call=true"
136+
response = client.post(
137+
"/action",
138+
data={"type": "approve", "id": "test-expense", "is_api_call": "true"},
135139
)
136140
assert response.status_code == 200
137141
assert response.text == "SUCCEED"
@@ -143,45 +147,60 @@ def test_action_reject_ui(self, client):
143147
all_expenses["test-expense"] = ExpenseState.CREATED
144148

145149
with patch("expense.ui.notify_expense_state_change") as mock_notify:
146-
response = client.get("/action?type=reject&id=test-expense")
150+
response = client.post(
151+
"/action", data={"type": "reject", "id": "test-expense"}
152+
)
147153
assert response.status_code == 200
148154
assert all_expenses["test-expense"] == ExpenseState.REJECTED
155+
# Should redirect to list view
156+
assert response.url.path == "/list"
149157
mock_notify.assert_called_once_with("test-expense", ExpenseState.REJECTED)
150158

151159
def test_action_payment(self, client):
152160
"""Test payment action"""
153161
all_expenses["test-expense"] = ExpenseState.APPROVED
154162

155-
response = client.get("/action?type=payment&id=test-expense&is_api_call=true")
163+
response = client.post(
164+
"/action",
165+
data={"type": "payment", "id": "test-expense", "is_api_call": "true"},
166+
)
156167
assert response.status_code == 200
157168
assert response.text == "SUCCEED"
158169
assert all_expenses["test-expense"] == ExpenseState.COMPLETED
159170

160171
def test_action_invalid_id_ui(self, client):
161172
"""Test action with invalid ID via UI"""
162-
response = client.get("/action?type=approve&id=nonexistent")
173+
response = client.post("/action", data={"type": "approve", "id": "nonexistent"})
163174
assert response.status_code == 200
164175
assert response.text == "Invalid ID"
165176

166177
def test_action_invalid_id_api(self, client):
167178
"""Test action with invalid ID via API"""
168-
response = client.get("/action?type=approve&id=nonexistent&is_api_call=true")
179+
response = client.post(
180+
"/action",
181+
data={"type": "approve", "id": "nonexistent", "is_api_call": "true"},
182+
)
169183
assert response.status_code == 200
170184
assert response.text == "ERROR:INVALID_ID"
171185

172186
def test_action_invalid_type_ui(self, client):
173187
"""Test action with invalid type via UI"""
174188
all_expenses["test-expense"] = ExpenseState.CREATED
175189

176-
response = client.get("/action?type=invalid&id=test-expense")
190+
response = client.post(
191+
"/action", data={"type": "invalid", "id": "test-expense"}
192+
)
177193
assert response.status_code == 200
178194
assert response.text == "Invalid action type"
179195

180196
def test_action_invalid_type_api(self, client):
181197
"""Test action with invalid type via API"""
182198
all_expenses["test-expense"] = ExpenseState.CREATED
183199

184-
response = client.get("/action?type=invalid&id=test-expense&is_api_call=true")
200+
response = client.post(
201+
"/action",
202+
data={"type": "invalid", "id": "test-expense", "is_api_call": "true"},
203+
)
185204
assert response.status_code == 200
186205
assert response.text == "ERROR:INVALID_TYPE"
187206

@@ -280,15 +299,18 @@ def test_state_transitions_complete_workflow(self, client):
280299

281300
# 3. Approve expense
282301
with patch("expense.ui.notify_expense_state_change") as mock_notify:
283-
response = client.get(
284-
f"/action?type=approve&id={expense_id}&is_api_call=true"
302+
response = client.post(
303+
"/action",
304+
data={"type": "approve", "id": expense_id, "is_api_call": "true"},
285305
)
286306
assert response.text == "SUCCEED"
287307
assert all_expenses[expense_id] == ExpenseState.APPROVED
288308
mock_notify.assert_called_once_with(expense_id, ExpenseState.APPROVED)
289309

290310
# 4. Process payment
291-
response = client.get(f"/action?type=payment&id={expense_id}&is_api_call=true")
311+
response = client.post(
312+
"/action", data={"type": "payment", "id": expense_id, "is_api_call": "true"}
313+
)
292314
assert response.text == "SUCCEED"
293315
assert all_expenses[expense_id] == ExpenseState.COMPLETED
294316

@@ -345,7 +367,7 @@ def test_parameter_validation(self, client):
345367
response = client.get("/create") # Missing id
346368
assert response.status_code == 422 # FastAPI validation error
347369

348-
response = client.get("/action") # Missing type and id
370+
response = client.post("/action") # Missing type and id
349371
assert response.status_code == 422
350372

351373
response = client.get("/status") # Missing id

0 commit comments

Comments
 (0)