feat: resolve shared My Work routes
This commit is contained in:
parent
2fecccfbde
commit
b5545e380e
|
|
@ -76,6 +76,8 @@ textarea { resize: vertical; min-height: 120px; }
|
|||
.load-more-notifications[hidden] { display:none; }
|
||||
.load-more-work { min-height:44px; width:100%; margin-top:10px; }
|
||||
.load-more-work[hidden] { display:none; }
|
||||
.retry-work-route { min-height:44px; width:100%; margin-top:10px; }
|
||||
.retry-work-route[hidden] { display:none; }
|
||||
.my-work-bulk { position:sticky; bottom:0; z-index:4; margin:10px -4px -12px; padding:10px 4px; padding-bottom:calc(10px + env(safe-area-inset-bottom)); background:rgba(11,21,38,.98); border-top:1px solid #2a496e; }
|
||||
.my-work-bulk button { min-height:44px; width:100%; }
|
||||
.my-work[data-stale="true"] { border-color:#fcd34d; }
|
||||
|
|
@ -287,6 +289,7 @@ textarea { resize: vertical; min-height: 120px; }
|
|||
<div class="small" id="notification-page-status" aria-live="polite"></div>
|
||||
<button class="load-more-notifications" id="load-more-notifications" type="button" hidden>Load older updates</button>
|
||||
<div class="small" id="my-work-action-status" aria-live="assertive"></div>
|
||||
<button class="retry-work-route" id="retry-work-route" type="button" hidden>Retry shared work item</button>
|
||||
<div class="small" id="work-route-share-status" aria-live="polite"></div>
|
||||
<div class="my-work-bulk" id="bulk-mark-read-bar" hidden>
|
||||
<button id="bulk-mark-read" type="button"></button>
|
||||
|
|
@ -733,6 +736,17 @@ textarea { resize: vertical; min-height: 120px; }
|
|||
if (!response.ok) throw new Error(payload.error || 'Review request failed.');
|
||||
return payload;
|
||||
}
|
||||
|
||||
async function api(url) {
|
||||
const response = await fetch(url, { headers: { Accept: 'application/json' } });
|
||||
const payload = await response.json().catch(() => ({}));
|
||||
if (!response.ok) {
|
||||
const error = new Error(payload.error || payload.detail || 'Shared work request failed.');
|
||||
error.unavailable = response.status === 404;
|
||||
throw error;
|
||||
}
|
||||
return payload;
|
||||
}
|
||||
const reviewController = createReviewController({ fetchJson: fetchReviewJson, storage: localStorage });
|
||||
const issueController = createIssueSheet({ fetchJson: fetchReviewJson, storage: localStorage });
|
||||
const issueCapture = createIssueCapture({ fetchJson: fetchReviewJson, storage: localStorage });
|
||||
|
|
@ -942,7 +956,22 @@ textarea { resize: vertical; min-height: 120px; }
|
|||
location: window.location,
|
||||
history: window.history,
|
||||
eventTarget: window,
|
||||
resolve: route => {
|
||||
const params = new URLSearchParams({ kind:route.kind });
|
||||
if (route.kind === 'update') params.set('notification_id', route.notification_id);
|
||||
else {
|
||||
params.set('repository', route.repository);
|
||||
params.set('number', route.number);
|
||||
}
|
||||
return api('api/v1/work-route?' + params.toString());
|
||||
},
|
||||
onResolving: () => {
|
||||
qs('#retry-work-route').hidden = true;
|
||||
qs('#my-work-action-status').textContent = 'Loading shared work item…';
|
||||
},
|
||||
onOpen: item => {
|
||||
qs('#retry-work-route').hidden = true;
|
||||
qs('#my-work-action-status').textContent = '';
|
||||
closeOpenWorkSheets();
|
||||
if (item.kind === 'update') notificationReader.open(item, lastMyWork);
|
||||
else if (item.kind === 'review') openReviewSheet(item, reviewTrigger);
|
||||
|
|
@ -956,13 +985,19 @@ textarea { resize: vertical; min-height: 120px; }
|
|||
if (selectedUpdate) closeUpdateSheet(true, false);
|
||||
},
|
||||
onInvalid: () => {
|
||||
qs('#retry-work-route').hidden = true;
|
||||
window.history.replaceState(null, '', window.location.pathname + window.location.search);
|
||||
closeOpenWorkSheets();
|
||||
qs('#my-work-action-status').textContent = 'Route unavailable · this item is no longer in My Work.';
|
||||
qs('#my-work').scrollIntoView({ block:'start' });
|
||||
},
|
||||
onError: () => {
|
||||
qs('#my-work-action-status').textContent = 'Could not load shared work item. The link is preserved; retry when connected.';
|
||||
qs('#retry-work-route').hidden = false;
|
||||
},
|
||||
});
|
||||
workRoute.start();
|
||||
qs('#retry-work-route').addEventListener('click', () => workRoute.sync());
|
||||
|
||||
function closeOpenWorkSheets() {
|
||||
['#issue-sheet', '#pull-sheet', '#review-sheet', '#update-sheet'].forEach(selector =>
|
||||
|
|
|
|||
|
|
@ -46,31 +46,68 @@
|
|||
Number(item.number) === route.number;
|
||||
}
|
||||
|
||||
function createController({ location, history, eventTarget, onOpen, onClose, onInvalid }) {
|
||||
function createController({
|
||||
location, history, eventTarget, onOpen, onClose, onInvalid,
|
||||
resolve, onResolving = function () {}, onError = function () {},
|
||||
}) {
|
||||
let items = [];
|
||||
let started = false;
|
||||
let active = '';
|
||||
let ready = false;
|
||||
let resolving = '';
|
||||
let resolution = 0;
|
||||
|
||||
function resolveMissing(fragment, route) {
|
||||
if (typeof resolve !== 'function') {
|
||||
onInvalid();
|
||||
return;
|
||||
}
|
||||
if (resolving === fragment) return;
|
||||
resolving = fragment;
|
||||
const request = ++resolution;
|
||||
onResolving(route);
|
||||
Promise.resolve(resolve(route)).then(item => {
|
||||
if (request !== resolution || String(location.hash || '') !== fragment) return;
|
||||
resolving = '';
|
||||
if (!item || !sameRoute(item, route)) {
|
||||
onInvalid();
|
||||
return;
|
||||
}
|
||||
active = fragment;
|
||||
onOpen({ ...item, kind: route.kind });
|
||||
}).catch(error => {
|
||||
if (request !== resolution || String(location.hash || '') !== fragment) return;
|
||||
resolving = '';
|
||||
if (error?.unavailable) onInvalid();
|
||||
else onError(error, route);
|
||||
});
|
||||
}
|
||||
|
||||
function sync() {
|
||||
const fragment = String(location.hash || '');
|
||||
if (!fragment) {
|
||||
resolution += 1;
|
||||
resolving = '';
|
||||
if (active) onClose();
|
||||
active = '';
|
||||
return;
|
||||
}
|
||||
const route = parse(fragment);
|
||||
if (!route) {
|
||||
resolution += 1;
|
||||
resolving = '';
|
||||
if (fragment.startsWith('#/my-work/')) onInvalid();
|
||||
active = '';
|
||||
return;
|
||||
}
|
||||
const item = items.find(candidate => sameRoute(candidate, route));
|
||||
if (!item) {
|
||||
if (ready) onInvalid();
|
||||
if (ready) resolveMissing(fragment, route);
|
||||
return;
|
||||
}
|
||||
if (active === fragment) return;
|
||||
resolution += 1;
|
||||
resolving = '';
|
||||
active = fragment;
|
||||
onOpen({ ...item, kind: route.kind });
|
||||
}
|
||||
|
|
|
|||
|
|
@ -22,6 +22,10 @@ class WorkItems(list[dict]):
|
|||
self.pagination = pagination
|
||||
|
||||
|
||||
class WorkRouteUnavailableError(ValueError):
|
||||
"""Raised when a shared route no longer belongs in the current user's queue."""
|
||||
|
||||
|
||||
class StaleReviewError(ValueError):
|
||||
"""Raised before mutation when a pull request head changed during review."""
|
||||
|
||||
|
|
@ -1040,6 +1044,57 @@ def _login_in_users(login: str, value: object) -> bool:
|
|||
)
|
||||
|
||||
|
||||
async def resolve_work_route(
|
||||
kind: str,
|
||||
repository: str | None,
|
||||
number: int | None,
|
||||
notification_id: int | None,
|
||||
) -> dict:
|
||||
if kind == "update":
|
||||
if notification_id is None:
|
||||
raise WorkRouteUnavailableError("Notification identity is missing")
|
||||
thread = await fetch(f"notifications/threads/{notification_id}")
|
||||
if not isinstance(thread, dict) or thread.get("unread") is not True:
|
||||
raise WorkRouteUnavailableError("Notification is no longer unread")
|
||||
detail = await notification_detail(notification_id)
|
||||
return {
|
||||
"kind": "update",
|
||||
"notification_id": notification_id,
|
||||
"has_update": True,
|
||||
"repository": detail.get("repository", ""),
|
||||
"title": detail.get("title", ""),
|
||||
"url": detail.get("url", ""),
|
||||
}
|
||||
if repository is None or number is None or kind not in {"issue", "pull", "review"}:
|
||||
raise WorkRouteUnavailableError("Work identity is missing")
|
||||
|
||||
target_path = "issues" if kind == "issue" else "pulls"
|
||||
login, target = await _current_login_and_target(
|
||||
f"repos/{repository}/{target_path}/{number}"
|
||||
)
|
||||
assigned = _login_in_users(login, target.get("assignees"))
|
||||
requested = _login_in_users(login, target.get("requested_reviewers"))
|
||||
eligible = (
|
||||
target.get("state") == "open"
|
||||
and (
|
||||
(kind == "issue" and not isinstance(target.get("pull_request"), dict) and assigned)
|
||||
or (kind == "pull" and assigned)
|
||||
or (kind == "review" and requested)
|
||||
)
|
||||
)
|
||||
if not eligible:
|
||||
raise WorkRouteUnavailableError("Work item is no longer in My Work")
|
||||
return {
|
||||
"kind": kind,
|
||||
"repository": repository,
|
||||
"number": number,
|
||||
"title": target.get("title", "") if isinstance(target.get("title"), str) else "",
|
||||
"state": "open",
|
||||
"url": _safe_web_url(target.get("html_url")),
|
||||
**({"is_review": True, "work_reasons": ["review_requested"]} if kind == "review" else {}),
|
||||
}
|
||||
|
||||
|
||||
async def is_assigned_issue(repository: str, number: int) -> bool:
|
||||
login, issue = await _current_login_and_target(
|
||||
f"repos/{repository}/issues/{number}"
|
||||
|
|
|
|||
29
src/main.py
29
src/main.py
|
|
@ -397,7 +397,7 @@ app.include_router(frontend_router)
|
|||
@app.middleware("http")
|
||||
async def prevent_live_api_caching(request, call_next):
|
||||
response = await call_next(request)
|
||||
if request.url.path in {"/api/v1/context", "/api/v1/events", "/api/v1/live", "/api/v1/available-issues", "/api/v1/search"} or request.url.path.startswith("/api/v1/work/") or (
|
||||
if request.url.path in {"/api/v1/context", "/api/v1/events", "/api/v1/live", "/api/v1/available-issues", "/api/v1/search", "/api/v1/work-route"} or request.url.path.startswith("/api/v1/work/") or (
|
||||
request.url.path.startswith("/api/v1/repos/")
|
||||
and request.url.path.endswith("/review")
|
||||
) or request.url.path.startswith("/api/v1/notifications") or (
|
||||
|
|
@ -547,6 +547,33 @@ async def global_search_preview(
|
|||
return JSONResponse(preview)
|
||||
|
||||
|
||||
@app.get("/api/v1/work-route")
|
||||
async def resolve_work_route(
|
||||
kind: Literal["issue", "pull", "review", "update"] = Query(),
|
||||
repository: str | None = Query(default=None, pattern=r"^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$"),
|
||||
number: int | None = Query(default=None, gt=0),
|
||||
notification_id: int | None = Query(default=None, gt=0),
|
||||
) -> JSONResponse:
|
||||
repository_route = repository is not None and number is not None and notification_id is None
|
||||
update_route = kind == "update" and notification_id is not None and repository is None and number is None
|
||||
if (kind == "update" and not update_route) or (kind != "update" and not repository_route):
|
||||
raise HTTPException(status_code=422, detail="Invalid work route identity")
|
||||
try:
|
||||
result = await asyncio.wait_for(
|
||||
gitea_proxy.resolve_work_route(kind, repository, number, notification_id),
|
||||
timeout=WORK_PAGE_TIMEOUT_SECONDS,
|
||||
)
|
||||
except gitea_proxy.WorkRouteUnavailableError:
|
||||
raise HTTPException(status_code=404, detail="Work item is no longer in My Work")
|
||||
except Exception:
|
||||
return JSONResponse(
|
||||
{"error": "The shared work item is temporarily unavailable. Please retry."},
|
||||
status_code=503,
|
||||
headers={"Retry-After": "1"},
|
||||
)
|
||||
return JSONResponse(result)
|
||||
|
||||
|
||||
@app.get("/api/v1/work/{stream}")
|
||||
async def paged_work(
|
||||
stream: Literal["issue", "pull", "review"],
|
||||
|
|
|
|||
|
|
@ -100,6 +100,74 @@ process.stdout.write(JSON.stringify({{calls, hash:location.hash}}));
|
|||
assert output["hash"] == "#/my-work/review/stackchain/dashboard/10"
|
||||
|
||||
|
||||
def test_work_route_controller_resolves_cold_routes_without_erasing_the_fragment():
|
||||
script = f"""
|
||||
const routes = require({json.dumps(str(WORK_ROUTE))});
|
||||
const location = {{hash:'#/my-work/issue/stackchain/api/87'}};
|
||||
const calls = [];
|
||||
const controller = routes.createController({{
|
||||
location,
|
||||
history: {{pushState() {{}}, replaceState() {{}}, back() {{}}}},
|
||||
eventTarget: {{addEventListener() {{}}}},
|
||||
resolve: async route => {{
|
||||
calls.push(['resolve', route.kind, route.repository, route.number]);
|
||||
return {{kind:'issue', repository:'stackchain/api', number:87, title:'Older assigned issue'}};
|
||||
}},
|
||||
onResolving: route => calls.push(['resolving', route.number]),
|
||||
onOpen: item => calls.push(['open', item.number, item.title]),
|
||||
onClose() {{}},
|
||||
onInvalid: () => calls.push(['invalid']),
|
||||
onError: () => calls.push(['error']),
|
||||
}});
|
||||
(async () => {{
|
||||
controller.start();
|
||||
controller.setItems([]);
|
||||
await new Promise(resolve => setTimeout(resolve, 0));
|
||||
process.stdout.write(JSON.stringify({{calls, hash:location.hash}}));
|
||||
}})();
|
||||
"""
|
||||
result = subprocess.run(
|
||||
["node", "-e", script], check=True, capture_output=True, text=True
|
||||
)
|
||||
|
||||
assert json.loads(result.stdout) == {
|
||||
"calls": [
|
||||
["resolving", 87],
|
||||
["resolve", "issue", "stackchain/api", 87],
|
||||
["open", 87, "Older assigned issue"],
|
||||
],
|
||||
"hash": "#/my-work/issue/stackchain/api/87",
|
||||
}
|
||||
|
||||
|
||||
def test_work_route_controller_dismisses_only_confirmed_unavailable_routes():
|
||||
script = f"""
|
||||
const routes = require({json.dumps(str(WORK_ROUTE))});
|
||||
const location = {{hash:'#/my-work/update/913'}};
|
||||
const calls = [];
|
||||
const unavailable = new Error('gone'); unavailable.unavailable = true;
|
||||
const controller = routes.createController({{
|
||||
location,
|
||||
history: {{pushState() {{}}, replaceState() {{}}, back() {{}}}},
|
||||
eventTarget: {{addEventListener() {{}}}},
|
||||
resolve: async () => {{ throw unavailable; }},
|
||||
onResolving() {{}}, onOpen() {{}}, onClose() {{}},
|
||||
onInvalid: () => calls.push('invalid'),
|
||||
onError: () => calls.push('error'),
|
||||
}});
|
||||
(async () => {{
|
||||
controller.start(); controller.setItems([]);
|
||||
await new Promise(resolve => setTimeout(resolve, 0));
|
||||
process.stdout.write(JSON.stringify(calls));
|
||||
}})();
|
||||
"""
|
||||
result = subprocess.run(
|
||||
["node", "-e", script], check=True, capture_output=True, text=True
|
||||
)
|
||||
|
||||
assert json.loads(result.stdout) == ["invalid"]
|
||||
|
||||
|
||||
def test_work_route_share_prefers_native_share_and_falls_back_to_clipboard():
|
||||
script = f"""
|
||||
const routes = require({json.dumps(str(WORK_ROUTE))});
|
||||
|
|
@ -135,6 +203,9 @@ async def test_dashboard_wires_addressable_work_sheets_back_navigation_and_share
|
|||
assert '<script src="static/work-route.js"></script>' in html
|
||||
assert 'const workRoute = createWorkRoute.createController({' in html
|
||||
assert 'workRoute.setItems(lastMyWork);' in html
|
||||
assert "api('api/v1/work-route?' + params.toString())" in html
|
||||
assert "Loading shared work item…" in html
|
||||
assert 'id="retry-work-route"' in html
|
||||
assert 'href="' + "' + escAttr(createWorkRoute.serialize(" in html
|
||||
assert '.read-update { min-height:44px; width:100%; display:flex;' in html
|
||||
assert 'workRoute.close();' in html
|
||||
|
|
|
|||
100
tests/test_work_route_resolver.py
Normal file
100
tests/test_work_route_resolver.py
Normal file
|
|
@ -0,0 +1,100 @@
|
|||
import httpx
|
||||
import pytest
|
||||
|
||||
from src import gitea_proxy, main
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_work_route_endpoint_resolves_one_authorized_item(monkeypatch):
|
||||
requested = []
|
||||
|
||||
async def resolve(kind, repository, number, notification_id):
|
||||
requested.append((kind, repository, number, notification_id))
|
||||
return {
|
||||
"kind": "review",
|
||||
"repository": "stackchain/dashboard",
|
||||
"number": 87,
|
||||
"title": "Review the mobile resolver",
|
||||
"is_review": True,
|
||||
"url": "https://forge.example/stackchain/dashboard/pulls/87",
|
||||
}
|
||||
|
||||
monkeypatch.setattr(main.gitea_proxy, "resolve_work_route", resolve, raising=False)
|
||||
transport = httpx.ASGITransport(app=main.app)
|
||||
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
||||
response = await client.get(
|
||||
"/api/v1/work-route?kind=review&repository=stackchain/dashboard&number=87"
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.headers["cache-control"] == "no-store"
|
||||
assert requested == [("review", "stackchain/dashboard", 87, None)]
|
||||
assert response.json() == {
|
||||
"kind": "review",
|
||||
"repository": "stackchain/dashboard",
|
||||
"number": 87,
|
||||
"title": "Review the mobile resolver",
|
||||
"is_review": True,
|
||||
"url": "https://forge.example/stackchain/dashboard/pulls/87",
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_resolve_work_route_returns_only_a_requested_review():
|
||||
requests = []
|
||||
|
||||
async def upstream(request):
|
||||
requests.append(request.url.path)
|
||||
if request.url.path.endswith("/user"):
|
||||
return httpx.Response(200, json={"login": "timmy"})
|
||||
return httpx.Response(200, json={
|
||||
"id": 700,
|
||||
"number": 87,
|
||||
"title": "Review the resolver",
|
||||
"state": "open",
|
||||
"html_url": gitea_proxy.GITEA_URL + "/stackchain/dashboard/pulls/87",
|
||||
"repository": {"full_name": "stackchain/dashboard"},
|
||||
"requested_reviewers": [{"login": "timmy"}],
|
||||
"assignees": [],
|
||||
"user": {"login": "alex"},
|
||||
})
|
||||
|
||||
gitea_proxy.start_client(transport=httpx.MockTransport(upstream))
|
||||
try:
|
||||
result = await gitea_proxy.resolve_work_route(
|
||||
"review", "stackchain/dashboard", 87, None
|
||||
)
|
||||
finally:
|
||||
await gitea_proxy.stop_client()
|
||||
|
||||
assert len(requests) == 2
|
||||
assert result == {
|
||||
"kind": "review",
|
||||
"repository": "stackchain/dashboard",
|
||||
"number": 87,
|
||||
"title": "Review the resolver",
|
||||
"state": "open",
|
||||
"url": gitea_proxy.GITEA_URL + "/stackchain/dashboard/pulls/87",
|
||||
"is_review": True,
|
||||
"work_reasons": ["review_requested"],
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_resolve_work_route_rejects_a_notification_that_is_already_read():
|
||||
async def upstream(request):
|
||||
if request.url.path.endswith("/notifications/threads/913"):
|
||||
return httpx.Response(200, json={
|
||||
"id": 913,
|
||||
"unread": False,
|
||||
"repository": {"full_name": "stackchain/dashboard"},
|
||||
"subject": {"title": "Already handled", "type": "Issue"},
|
||||
})
|
||||
return httpx.Response(404)
|
||||
|
||||
gitea_proxy.start_client(transport=httpx.MockTransport(upstream))
|
||||
try:
|
||||
with pytest.raises(gitea_proxy.WorkRouteUnavailableError):
|
||||
await gitea_proxy.resolve_work_route("update", None, None, 913)
|
||||
finally:
|
||||
await gitea_proxy.stop_client()
|
||||
Loading…
Reference in New Issue
Block a user