Merge pull request 'Acknowledge unread updates with a reaction and open the next' (#540) from timmy/539-acknowledge-update-reaction into main
This commit is contained in:
commit
3605c42651
|
|
@ -265,6 +265,7 @@ textarea { resize: vertical; min-height: 120px; }
|
|||
.update-ownership-actions { display:grid; grid-template-columns:repeat(2,minmax(0,1fr)); gap:8px; }
|
||||
.update-ownership-actions button { min-width:0; width:100%; }
|
||||
.update-sheet-actions button, .update-sheet-actions a { min-height:44px; display:flex; align-items:center; justify-content:center; }
|
||||
.update-sheet-actions #acknowledge-update-next { min-height:44px; width:100%; }
|
||||
.update-sheet-actions a { border:1px solid #60a5fa; border-radius:10px; font-weight:700; }
|
||||
.update-retry { min-height:44px; width:100%; margin-top:10px; }
|
||||
.issue-sheet { position:fixed; inset:0; z-index:56; display:none; justify-content:flex-end; background:rgba(5,12,21,.72); backdrop-filter:blur(4px); }
|
||||
|
|
|
|||
|
|
@ -508,6 +508,14 @@
|
|||
return payload;
|
||||
}
|
||||
|
||||
async function acknowledgeNotification(notificationId) {
|
||||
const response = await fetch('api/v1/notifications/' + encodeURIComponent(notificationId) +
|
||||
'/acknowledge', { method: 'POST', headers: { Accept: 'application/json' } });
|
||||
const payload = await response.json().catch(() => ({}));
|
||||
if (!response.ok) throw new Error(payload.error || 'Acknowledging the update failed.');
|
||||
return payload;
|
||||
}
|
||||
|
||||
async function markNotificationsRead(ids) {
|
||||
const response = await fetch('api/v1/notifications/read', {
|
||||
method: 'PATCH',
|
||||
|
|
@ -674,6 +682,7 @@
|
|||
load: fetchNotificationDetail,
|
||||
loadConversation: fetchNotificationConversation,
|
||||
markRead: markNotificationRead,
|
||||
acknowledge: acknowledgeNotification,
|
||||
queueRead: notificationId => notificationReadOutbox.enqueueDurably(notificationId),
|
||||
loadSaved: item => offlineWorkStore.loadDetail(confirmedOwnerLogin, item),
|
||||
onOpen: item => {
|
||||
|
|
@ -696,6 +705,7 @@
|
|||
qs('#update-reply-status').textContent = '';
|
||||
qs('#send-update-reply').disabled = false;
|
||||
qs('#send-update-reply-read-next').disabled = false;
|
||||
qs('#acknowledge-update-next').hidden = true;
|
||||
qs('#update-ownership-action').hidden = true;
|
||||
qs('#update-ownership-start').hidden = true;
|
||||
qs('#retry-update-load').hidden = true;
|
||||
|
|
@ -708,6 +718,7 @@
|
|||
qs('#update-subject-state').textContent = detail.state || '';
|
||||
qs('#update-subject-body').innerHTML = renderMarkdown(detail.subject_body || 'No subject context was provided.');
|
||||
qs('#open-update-gitea').href = detail.url || selectedUpdate?.url || '#';
|
||||
qs('#acknowledge-update-next').hidden = !detail.acknowledge_supported;
|
||||
if (offlineWorkMode) {
|
||||
setOfflineUpdateControls(true);
|
||||
} else {
|
||||
|
|
@ -1676,6 +1687,7 @@
|
|||
function setOfflineUpdateControls(offline) {
|
||||
qs('#mark-update-read-next').disabled = false;
|
||||
qs('#mark-update-read-next').textContent = offline ? 'Queue read & next' : 'Mark read & next';
|
||||
qs('#acknowledge-update-next').disabled = offline;
|
||||
qs('#update-ownership-action').disabled = offline;
|
||||
qs('#update-ownership-start').disabled = offline;
|
||||
qs('#load-older-update-comments').disabled = offline;
|
||||
|
|
@ -4696,6 +4708,16 @@
|
|||
qs('#mark-update-read-next').disabled = false;
|
||||
}
|
||||
});
|
||||
qs('#acknowledge-update-next').addEventListener('click', async () => {
|
||||
const button = qs('#acknowledge-update-next');
|
||||
button.disabled = true;
|
||||
try {
|
||||
const result = await notificationReader.acknowledgeAndNext(lastMyWork);
|
||||
if (result) qs('#my-work-action-status').textContent = 'Update acknowledged with 👍.';
|
||||
} finally {
|
||||
button.disabled = offlineWorkMode;
|
||||
}
|
||||
});
|
||||
qs('#close-review-sheet').addEventListener('click', closeReviewSheet);
|
||||
qs('#retry-review-load').addEventListener('click', () => {
|
||||
if (selectedReview) openReviewSheet(selectedReview, reviewTrigger);
|
||||
|
|
|
|||
|
|
@ -548,6 +548,7 @@
|
|||
<button id="update-ownership-start" type="button" hidden aria-describedby="update-sheet-status">Take ownership & start</button>
|
||||
</div>
|
||||
<button class="share-work-route" type="button">Share</button>
|
||||
<button id="acknowledge-update-next" type="button" hidden aria-label="Acknowledge and open next update">👍 Acknowledge & next</button>
|
||||
<button id="mark-update-read-next" type="button">Mark read & next</button>
|
||||
<a id="open-update-gitea" href="#" target="_blank" rel="noopener noreferrer">Open in Gitea</a>
|
||||
<details class="detail-defer"><summary>Defer</summary><div class="detail-defer-options"><button type="button" data-detail-defer-preset="today" disabled data-planning-disabled>Later today</button><button type="button" data-detail-defer-preset="tomorrow" disabled data-planning-disabled>Tomorrow</button><button type="button" data-detail-defer-custom disabled data-planning-disabled>Choose date & time</button><button type="button" data-detail-defer-cancel>Cancel</button></div></details>
|
||||
|
|
|
|||
|
|
@ -280,6 +280,7 @@ function createWorkPager({ load, onItems, onPagination, onStatus }) {
|
|||
|
||||
function createNotificationReader({
|
||||
load, markRead, onOpen, onDetail, onItems, onStatus, onClose,
|
||||
acknowledge = null,
|
||||
queueRead = null,
|
||||
loadSaved = () => null,
|
||||
loadConversation = null,
|
||||
|
|
@ -382,6 +383,21 @@ function createNotificationReader({
|
|||
marking = false;
|
||||
}
|
||||
},
|
||||
async acknowledgeAndNext(items) {
|
||||
if (!selected || marking || offlineHydrated || !acknowledge) return false;
|
||||
const current = selected;
|
||||
marking = true;
|
||||
onStatus('Adding reaction and marking read…');
|
||||
try {
|
||||
await acknowledge(current.notification_id);
|
||||
return await advanceAfterRead(items, current);
|
||||
} catch (_error) {
|
||||
onStatus('Could not acknowledge update. Retry.');
|
||||
return false;
|
||||
} finally {
|
||||
marking = false;
|
||||
}
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -735,6 +735,60 @@ async def mark_notification_read(thread_id: int) -> None:
|
|||
response.raise_for_status()
|
||||
|
||||
|
||||
async def acknowledge_notification(thread_id: int) -> dict:
|
||||
thread = await fetch(f"notifications/threads/{thread_id}")
|
||||
if not isinstance(thread, dict):
|
||||
raise ValueError("Gitea notification thread response was not an object")
|
||||
repository = thread.get("repository")
|
||||
subject = thread.get("subject")
|
||||
if not isinstance(repository, dict) or not isinstance(subject, dict):
|
||||
raise ValueError("Notification does not identify a conversation")
|
||||
repository_name = repository.get("full_name")
|
||||
subject_path = _gitea_api_path(subject.get("url"))
|
||||
comment_path = _gitea_api_path(subject.get("latest_comment_url"))
|
||||
subject_match = re.fullmatch(
|
||||
r"repos/([^/]+/[^/]+)/(issues|pulls)/(\d+)", subject_path
|
||||
)
|
||||
comment_match = re.fullmatch(
|
||||
r"repos/([^/]+/[^/]+)/issues/comments/(\d+)", comment_path
|
||||
)
|
||||
if (
|
||||
not subject_match
|
||||
or not comment_match
|
||||
or subject_match.group(1) != repository_name
|
||||
or comment_match.group(1) != repository_name
|
||||
or subject.get("type") not in {"Issue", "Pull"}
|
||||
):
|
||||
raise ValueError("Notification has no supported latest comment")
|
||||
|
||||
user = await current_user()
|
||||
login = user.get("login") if isinstance(user, dict) else None
|
||||
if not isinstance(login, str) or not login:
|
||||
raise ValueError("Authenticated Gitea user is unavailable")
|
||||
reaction_path = f"/api/v1/{comment_path}/reactions"
|
||||
response = await _get_client().get(reaction_path, headers=_auth())
|
||||
response.raise_for_status()
|
||||
reactions = response.json()
|
||||
existing = any(
|
||||
isinstance(reaction, dict)
|
||||
and reaction.get("content") == "+1"
|
||||
and isinstance(reaction.get("user"), dict)
|
||||
and reaction["user"].get("login") == login
|
||||
for reaction in (reactions if isinstance(reactions, list) else [])
|
||||
)
|
||||
if not existing:
|
||||
response = await _get_client().post(
|
||||
reaction_path, headers=_auth(), json={"content": "+1"}
|
||||
)
|
||||
response.raise_for_status()
|
||||
await mark_notification_read(thread_id)
|
||||
return {
|
||||
"id": thread_id,
|
||||
"reaction": "existing" if existing else "created",
|
||||
"status": "read",
|
||||
}
|
||||
|
||||
|
||||
def _gitea_api_path(value: Any) -> str:
|
||||
if not isinstance(value, str):
|
||||
return ""
|
||||
|
|
@ -763,6 +817,9 @@ async def notification_detail(thread_id: int) -> dict:
|
|||
conversation_match = re.fullmatch(
|
||||
r"repos/([^/]+/[^/]+)/(issues|pulls)/(\d+)", subject_path
|
||||
)
|
||||
comment_match = re.fullmatch(
|
||||
r"repos/([^/]+/[^/]+)/issues/comments/(\d+)", comment_path
|
||||
)
|
||||
repository_name = repository.get("full_name")
|
||||
supported_conversation = (
|
||||
conversation_match
|
||||
|
|
@ -838,6 +895,11 @@ async def notification_detail(thread_id: int) -> dict:
|
|||
"url": latest_url,
|
||||
},
|
||||
"issue": issue,
|
||||
"acknowledge_supported": bool(
|
||||
supported_conversation
|
||||
and comment_match
|
||||
and comment_match.group(1) == repository_name
|
||||
),
|
||||
"conversation": conversation,
|
||||
}
|
||||
|
||||
|
|
|
|||
28
src/main.py
28
src/main.py
|
|
@ -3066,6 +3066,34 @@ async def read_notification(thread_id: int = PathParam(gt=0)) -> JSONResponse:
|
|||
return JSONResponse({"id": thread_id, "status": "read"})
|
||||
|
||||
|
||||
@app.post("/api/v1/notifications/{thread_id}/acknowledge")
|
||||
async def acknowledge_notification(thread_id: int = PathParam(gt=0)) -> JSONResponse:
|
||||
try:
|
||||
result = await asyncio.wait_for(
|
||||
gitea_proxy.acknowledge_notification(thread_id),
|
||||
timeout=NOTIFICATION_MUTATION_TIMEOUT_SECONDS,
|
||||
)
|
||||
except TimeoutError:
|
||||
return JSONResponse(
|
||||
{"error": "Acknowledging the update timed out. Please retry."},
|
||||
status_code=503,
|
||||
headers={"Retry-After": "1"},
|
||||
)
|
||||
except ValueError:
|
||||
return JSONResponse(
|
||||
{"error": "This update has no comment that can be acknowledged."},
|
||||
status_code=422,
|
||||
)
|
||||
except Exception:
|
||||
return JSONResponse(
|
||||
{"error": "The update could not be acknowledged. Please retry."},
|
||||
status_code=503,
|
||||
headers={"Retry-After": "1"},
|
||||
)
|
||||
await _remove_notifications_from_live_snapshot([thread_id])
|
||||
return JSONResponse(result)
|
||||
|
||||
|
||||
@app.post("/api/v1/notifications/{thread_id}/reply", status_code=201)
|
||||
async def reply_to_notification(
|
||||
reply: NotificationReply,
|
||||
|
|
|
|||
|
|
@ -182,6 +182,7 @@ async def test_notification_detail_loads_subject_and_latest_comment_for_inbox_re
|
|||
"url": "https://forge.example/stackchain/api/issues/7#issuecomment-9",
|
||||
},
|
||||
"issue": {"number": 7, "assignees": [], "claimable": True},
|
||||
"acknowledge_supported": True,
|
||||
"conversation": {
|
||||
"comments": [{
|
||||
"id": 9,
|
||||
|
|
|
|||
|
|
@ -3439,6 +3439,68 @@ reader.open(original[0], original).then(() =>
|
|||
assert output["result"]["next"]["notification_id"] == 43
|
||||
|
||||
|
||||
def test_notification_reader_acknowledges_once_and_opens_next_update():
|
||||
script = f"""
|
||||
const buildMyWork = require({json.dumps(str(MY_WORK))});
|
||||
const items = [
|
||||
{{kind:'update', notification_id:42, has_update:true}},
|
||||
{{kind:'update', notification_id:43, has_update:true}},
|
||||
];
|
||||
let release;
|
||||
const calls = [];
|
||||
const events = [];
|
||||
const reader = buildMyWork.createNotificationReader({{
|
||||
load: async id => ({{id}}), markRead: async () => {{}},
|
||||
acknowledge: id => new Promise(resolve => {{ calls.push(id); release = resolve; }}),
|
||||
onOpen: item => events.push(['open', item.notification_id]), onDetail: () => {{}},
|
||||
onItems: next => events.push(['items', next.map(item => item.notification_id)]),
|
||||
onStatus: status => events.push(['status', status]), onClose: () => events.push(['close']),
|
||||
}});
|
||||
(async () => {{
|
||||
await reader.open(items[0]);
|
||||
events.length = 0;
|
||||
const first = reader.acknowledgeAndNext(items);
|
||||
const duplicate = reader.acknowledgeAndNext(items);
|
||||
await Promise.resolve();
|
||||
release({{reaction:'created', status:'read'}});
|
||||
const results = await Promise.all([first, duplicate]);
|
||||
process.stdout.write(JSON.stringify({{calls, events, results}}));
|
||||
}})();
|
||||
"""
|
||||
output = json.loads(subprocess.run(
|
||||
["node", "-e", script], check=True, capture_output=True, text=True
|
||||
).stdout)
|
||||
|
||||
assert output["calls"] == [42]
|
||||
assert output["events"] == [
|
||||
["status", "Adding reaction and marking read…"],
|
||||
["items", [43]],
|
||||
["open", 43],
|
||||
["status", "Loading update…"],
|
||||
["status", "Update ready."],
|
||||
]
|
||||
assert output["results"][0]["next"]["notification_id"] == 43
|
||||
assert output["results"][1] is False
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_mobile_update_sheet_wires_touch_safe_online_acknowledge_and_next():
|
||||
html = await dashboard()
|
||||
|
||||
assert (
|
||||
'id="acknowledge-update-next" type="button" hidden '
|
||||
'aria-label="Acknowledge and open next update">👍 Acknowledge & next</button>'
|
||||
) in html
|
||||
assert "qs('#acknowledge-update-next').hidden = true;" in html
|
||||
assert "qs('#acknowledge-update-next').hidden = !detail.acknowledge_supported;" in html
|
||||
assert "async function acknowledgeNotification(notificationId)" in html
|
||||
assert "'/acknowledge', { method: 'POST'" in html
|
||||
assert "acknowledge: acknowledgeNotification" in html
|
||||
assert "notificationReader.acknowledgeAndNext(lastMyWork)" in html
|
||||
assert "qs('#acknowledge-update-next').disabled = offline;" in html
|
||||
assert '.update-sheet-actions #acknowledge-update-next { min-height:44px;' in html
|
||||
|
||||
|
||||
def test_notification_reader_keeps_current_update_retryable_when_detail_load_fails():
|
||||
script = f"""
|
||||
const buildMyWork = require({json.dumps(str(MY_WORK))});
|
||||
|
|
|
|||
|
|
@ -66,6 +66,7 @@ async def test_notification_detail_opens_the_newest_conversation_page_in_chronol
|
|||
"assignees": [],
|
||||
"claimable": True,
|
||||
}
|
||||
assert result["acknowledge_supported"] is True
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
|
|
|
|||
|
|
@ -41,6 +41,117 @@ async def test_mark_notification_read_calls_supported_gitea_thread_endpoint(monk
|
|||
]
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_acknowledge_notification_resolves_latest_comment_and_adds_one_reaction(monkeypatch):
|
||||
calls = []
|
||||
|
||||
async def fake_fetch(path):
|
||||
calls.append(("fetch", path))
|
||||
if path == "notifications/threads/42":
|
||||
return {
|
||||
"repository": {"full_name": "stackchain/api"},
|
||||
"subject": {
|
||||
"type": "Issue",
|
||||
"url": "https://forge.example/api/v1/repos/stackchain/api/issues/7",
|
||||
"latest_comment_url": (
|
||||
"https://forge.example/api/v1/repos/stackchain/api/issues/comments/91"
|
||||
),
|
||||
},
|
||||
}
|
||||
if path == "user":
|
||||
return {"login": "timmy"}
|
||||
raise AssertionError(path)
|
||||
|
||||
class Response:
|
||||
def __init__(self, payload=None):
|
||||
self._payload = payload
|
||||
|
||||
def raise_for_status(self):
|
||||
return None
|
||||
|
||||
def json(self):
|
||||
return self._payload
|
||||
|
||||
class Client:
|
||||
async def get(self, path, headers):
|
||||
calls.append(("get", path))
|
||||
return Response([])
|
||||
|
||||
async def post(self, path, headers, json):
|
||||
calls.append(("post", path, json))
|
||||
return Response({"content": "+1"})
|
||||
|
||||
async def patch(self, path, headers):
|
||||
calls.append(("patch", path))
|
||||
return Response()
|
||||
|
||||
monkeypatch.setattr(gitea_proxy, "GITEA_URL", "https://forge.example")
|
||||
monkeypatch.setattr(gitea_proxy, "fetch", fake_fetch)
|
||||
monkeypatch.setattr(gitea_proxy, "_get_client", lambda: Client())
|
||||
monkeypatch.setattr(gitea_proxy, "_auth", lambda: {"Authorization": "token test"})
|
||||
|
||||
result = await gitea_proxy.acknowledge_notification(42)
|
||||
|
||||
reaction_path = "/api/v1/repos/stackchain/api/issues/comments/91/reactions"
|
||||
assert result == {"id": 42, "reaction": "created", "status": "read"}
|
||||
assert calls == [
|
||||
("fetch", "notifications/threads/42"),
|
||||
("fetch", "user"),
|
||||
("get", reaction_path),
|
||||
("post", reaction_path, {"content": "+1"}),
|
||||
("patch", "/api/v1/notifications/threads/42?to-status=read"),
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_acknowledge_notification_reuses_existing_operator_reaction_on_retry(monkeypatch):
|
||||
posts = []
|
||||
|
||||
async def fake_fetch(path):
|
||||
if path == "user":
|
||||
return {"login": "timmy"}
|
||||
return {
|
||||
"repository": {"full_name": "stackchain/api"},
|
||||
"subject": {
|
||||
"type": "Pull",
|
||||
"url": "https://forge.example/api/v1/repos/stackchain/api/pulls/7",
|
||||
"latest_comment_url": (
|
||||
"https://forge.example/api/v1/repos/stackchain/api/issues/comments/91"
|
||||
),
|
||||
},
|
||||
}
|
||||
|
||||
class Response:
|
||||
def __init__(self, payload=None):
|
||||
self.payload = payload
|
||||
|
||||
def raise_for_status(self):
|
||||
return None
|
||||
|
||||
def json(self):
|
||||
return self.payload
|
||||
|
||||
class Client:
|
||||
async def get(self, path, headers):
|
||||
return Response([{"content": "+1", "user": {"login": "timmy"}}])
|
||||
|
||||
async def post(self, path, headers, json):
|
||||
posts.append((path, json))
|
||||
return Response()
|
||||
|
||||
async def patch(self, path, headers):
|
||||
return Response()
|
||||
|
||||
monkeypatch.setattr(gitea_proxy, "GITEA_URL", "https://forge.example")
|
||||
monkeypatch.setattr(gitea_proxy, "fetch", fake_fetch)
|
||||
monkeypatch.setattr(gitea_proxy, "_get_client", lambda: Client())
|
||||
|
||||
result = await gitea_proxy.acknowledge_notification(42)
|
||||
|
||||
assert result == {"id": 42, "reaction": "existing", "status": "read"}
|
||||
assert posts == []
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_mark_notification_read_api_is_bounded_and_never_cacheable(monkeypatch):
|
||||
marked = []
|
||||
|
|
@ -61,6 +172,33 @@ async def test_mark_notification_read_api_is_bounded_and_never_cacheable(monkeyp
|
|||
assert marked == [42]
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_acknowledge_notification_api_confirms_reaction_and_removes_snapshot_item(monkeypatch):
|
||||
calls = []
|
||||
|
||||
async def acknowledge(thread_id):
|
||||
calls.append(thread_id)
|
||||
return {"id": thread_id, "reaction": "created", "status": "read"}
|
||||
|
||||
monkeypatch.setattr(main.gitea_proxy, "acknowledge_notification", acknowledge, raising=False)
|
||||
monkeypatch.setattr(
|
||||
main,
|
||||
"_live_snapshot_value",
|
||||
{"notifications": [{"id": 42}, {"id": 43}]},
|
||||
)
|
||||
transport = httpx.ASGITransport(app=main.app)
|
||||
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
||||
response = await client.post("/api/v1/notifications/42/acknowledge")
|
||||
invalid = await client.post("/api/v1/notifications/0/acknowledge")
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json() == {"id": 42, "reaction": "created", "status": "read"}
|
||||
assert response.headers["cache-control"] == "no-store"
|
||||
assert invalid.status_code == 422
|
||||
assert calls == [42]
|
||||
assert main._live_snapshot_value == {"notifications": [{"id": 43}]}
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_snapshot_maintenance_does_not_block_the_event_loop(monkeypatch):
|
||||
async def mark(_thread_id):
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user