Merge pull request 'Take ownership from unread issue updates' (#322) from timmy/321-take-ownership-unread-update into main
This commit is contained in:
commit
160753caf8
|
|
@ -371,6 +371,33 @@
|
|||
authoredOutbox,
|
||||
onStatus: message => { qs('#update-reply-status').textContent = message; },
|
||||
});
|
||||
const updateOwnership = createUpdateOwnership({
|
||||
claim: item => fetchReviewJson(
|
||||
'api/v1/repos/' + item.repository.split('/').map(encodeURIComponent).join('/') +
|
||||
'/issues/' + encodeURIComponent(item.number) + '/claim',
|
||||
{ method:'PATCH', headers:{ Accept:'application/json' } }
|
||||
),
|
||||
addToday: item => {
|
||||
const result = todayWork.add(item);
|
||||
refreshMyWorkView();
|
||||
return result;
|
||||
},
|
||||
onClaimed: item => {
|
||||
if (!lastContextSnapshot) return;
|
||||
const issues = (lastContextSnapshot.issues || []).filter(candidate =>
|
||||
candidate.repository !== item.repository || candidate.number !== item.number
|
||||
);
|
||||
lastContextSnapshot = { ...lastContextSnapshot, issues:issues.concat(item) };
|
||||
paintMyWork(lastContextSnapshot);
|
||||
},
|
||||
onState: state => {
|
||||
const button = qs('#update-ownership-action');
|
||||
button.hidden = state.action === 'hidden';
|
||||
button.disabled = state.busy;
|
||||
button.textContent = state.action === 'today' ? 'Add to Today' : 'Take ownership';
|
||||
if (state.message) qs('#update-sheet-status').textContent = state.message;
|
||||
},
|
||||
});
|
||||
const notificationReader = createNotificationReader({
|
||||
load: fetchNotificationDetail,
|
||||
loadConversation: fetchNotificationConversation,
|
||||
|
|
@ -390,6 +417,7 @@
|
|||
qs('#update-reply').value = notificationReplier.loadDraft(item);
|
||||
qs('#update-reply-status').textContent = '';
|
||||
qs('#send-update-reply').disabled = false;
|
||||
qs('#update-ownership-action').hidden = true;
|
||||
qs('#retry-update-load').hidden = true;
|
||||
qs('#keep-update-unread').focus();
|
||||
},
|
||||
|
|
@ -399,6 +427,7 @@
|
|||
qs('#update-subject-state').textContent = detail.state || '';
|
||||
qs('#update-subject-body').textContent = detail.subject_body || 'No subject context was provided.';
|
||||
qs('#open-update-gitea').href = detail.url || selectedUpdate?.url || '#';
|
||||
updateOwnership.open(detail, selectedUpdate);
|
||||
qs('#retry-update-load').hidden = true;
|
||||
},
|
||||
onConversation: renderUpdateConversation,
|
||||
|
|
@ -2700,6 +2729,7 @@
|
|||
}
|
||||
});
|
||||
qs('#keep-update-unread').addEventListener('click', () => closeUpdateSheet(true));
|
||||
qs('#update-ownership-action').addEventListener('click', () => updateOwnership.act());
|
||||
qs('#retry-update-load').addEventListener('click', () => {
|
||||
if (selectedUpdate) notificationReader.open(selectedUpdate, lastMyWork);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -378,6 +378,7 @@
|
|||
<div id="update-reply-status" class="small" aria-live="assertive"></div>
|
||||
</section>
|
||||
<div class="update-sheet-actions">
|
||||
<button id="update-ownership-action" type="button" hidden aria-describedby="update-sheet-status">Take ownership</button>
|
||||
<button class="share-work-route" type="button">Share</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>
|
||||
|
|
@ -519,6 +520,7 @@
|
|||
<script src="static/offline-work.js"></script>
|
||||
<script src="static/my-work.js"></script>
|
||||
<script src="static/today-work.js"></script>
|
||||
<script src="static/update-ownership.js"></script>
|
||||
<script src="static/later-work.js"></script>
|
||||
<script src="static/detail-defer.js"></script>
|
||||
<script src="static/pick-work.js"></script>
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
const BASE = new URL('./', self.location.href).pathname;
|
||||
importScripts(BASE + 'static/background-issue-sync.js');
|
||||
const CACHE = 'stackchain-dashboard-shell-v34';
|
||||
const CACHE = 'stackchain-dashboard-shell-v35';
|
||||
const OUTAGE_STATUSES = new Set([500, 502, 503, 504]);
|
||||
const SHELL = [
|
||||
BASE,
|
||||
|
|
@ -22,6 +22,7 @@ const SHELL = [
|
|||
BASE + 'static/offline-work.js',
|
||||
BASE + 'static/my-work.js',
|
||||
BASE + 'static/today-work.js',
|
||||
BASE + 'static/update-ownership.js',
|
||||
BASE + 'static/later-work.js',
|
||||
BASE + 'static/detail-defer.js',
|
||||
BASE + 'static/pick-work.js',
|
||||
|
|
|
|||
78
frontend/update-ownership.js
Normal file
78
frontend/update-ownership.js
Normal file
|
|
@ -0,0 +1,78 @@
|
|||
function createUpdateOwnership({ claim, addToday, onClaimed, onState }) {
|
||||
let detail = null;
|
||||
let update = null;
|
||||
let claimedItem = null;
|
||||
let request = null;
|
||||
|
||||
function state(value) {
|
||||
onState?.(value);
|
||||
}
|
||||
|
||||
function open(nextDetail, nextUpdate) {
|
||||
detail = nextDetail;
|
||||
update = nextUpdate;
|
||||
claimedItem = null;
|
||||
request = null;
|
||||
state({
|
||||
action: nextDetail?.issue?.claimable ? 'claim' : 'hidden',
|
||||
busy: false,
|
||||
message: '',
|
||||
});
|
||||
}
|
||||
|
||||
function act() {
|
||||
if (request) return request;
|
||||
if (claimedItem) {
|
||||
const result = addToday(claimedItem);
|
||||
const messages = {
|
||||
added: 'Added to Today.',
|
||||
exists: 'This issue is already in Today.',
|
||||
full: 'Today is limited to 5 items. Remove one before adding more.',
|
||||
unavailable: 'Could not save Today on this device.',
|
||||
};
|
||||
state({ action: 'today', busy: false, message: messages[result] || messages.unavailable });
|
||||
return Promise.resolve(result);
|
||||
}
|
||||
if (!detail?.issue?.claimable) return Promise.resolve('hidden');
|
||||
|
||||
state({ action: 'claim', busy: true, message: 'Assigning…' });
|
||||
request = Promise.resolve(claim({
|
||||
repository: detail.repository,
|
||||
number: detail.issue.number,
|
||||
})).then(result => {
|
||||
claimedItem = {
|
||||
...result,
|
||||
repository: detail.repository,
|
||||
kind: 'issue',
|
||||
notification_id: update?.notification_id,
|
||||
updated_at: update?.updated_at || result.updated_at,
|
||||
has_update: true,
|
||||
};
|
||||
detail.issue.claimable = false;
|
||||
onClaimed?.(claimedItem);
|
||||
state({ action: 'today', busy: false, message: 'Assigned to you. The update is still unread.' });
|
||||
return claimedItem;
|
||||
}).catch(error => {
|
||||
if (error?.status === 409) {
|
||||
detail.issue.claimable = false;
|
||||
state({
|
||||
action: 'hidden',
|
||||
busy: false,
|
||||
message: 'Someone else claimed or closed this issue. The update is still unread.',
|
||||
});
|
||||
return 'conflict';
|
||||
}
|
||||
state({
|
||||
action: 'claim',
|
||||
busy: false,
|
||||
message: (error?.message || 'The issue could not be assigned.') + ' The update and your reply draft are safe; retry.',
|
||||
});
|
||||
return 'retry';
|
||||
}).finally(() => { request = null; });
|
||||
return request;
|
||||
}
|
||||
|
||||
return { open, act };
|
||||
}
|
||||
|
||||
if (typeof module !== 'undefined' && module.exports) module.exports = createUpdateOwnership;
|
||||
|
|
@ -680,6 +680,21 @@ async def notification_detail(thread_id: int) -> dict:
|
|||
latest_url = _safe_web_url(comment.get("html_url")) or _safe_web_url(
|
||||
subject.get("latest_comment_html_url")
|
||||
)
|
||||
assignee_values = subject_detail.get("assignees")
|
||||
assignees = [
|
||||
value.get("login")
|
||||
for value in (assignee_values if isinstance(assignee_values, list) else [])
|
||||
if isinstance(value, dict) and isinstance(value.get("login"), str)
|
||||
]
|
||||
issue = {
|
||||
"number": subject_detail.get("number"),
|
||||
"assignees": assignees,
|
||||
"claimable": (
|
||||
subject.get("type") == "Issue"
|
||||
and subject_detail.get("state", subject.get("state")) == "open"
|
||||
and not assignees
|
||||
),
|
||||
} if supported_conversation and subject.get("type") == "Issue" else None
|
||||
return {
|
||||
"id": thread_id,
|
||||
"repository": repository.get("full_name", "")
|
||||
|
|
@ -710,6 +725,7 @@ async def notification_detail(thread_id: int) -> dict:
|
|||
else "",
|
||||
"url": latest_url,
|
||||
},
|
||||
"issue": issue,
|
||||
"conversation": conversation,
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1587,7 +1587,7 @@ async def claim_available_issue(
|
|||
)
|
||||
except gitea_proxy.IssueNotAvailableError:
|
||||
return JSONResponse(
|
||||
{"error": "This issue was already claimed or is no longer open. Refresh Find Work."},
|
||||
{"error": "This issue was already claimed or is no longer open."},
|
||||
status_code=409,
|
||||
)
|
||||
except Exception:
|
||||
|
|
|
|||
|
|
@ -133,7 +133,10 @@ async def test_notification_detail_loads_subject_and_latest_comment_for_inbox_re
|
|||
},
|
||||
})
|
||||
if request.url.path.endswith("/issues/7"):
|
||||
return httpx.Response(200, json={"body": "Deploy fails after **three** retries."})
|
||||
return httpx.Response(200, json={
|
||||
"number": 7, "state": "open", "assignees": [],
|
||||
"body": "Deploy fails after **three** retries.",
|
||||
})
|
||||
if request.url.path.endswith("/issues/comments/9"):
|
||||
return httpx.Response(200, json={
|
||||
"id": 9,
|
||||
|
|
@ -178,6 +181,7 @@ async def test_notification_detail_loads_subject_and_latest_comment_for_inbox_re
|
|||
"created_at": "2026-08-06T12:30:00Z",
|
||||
"url": "https://forge.example/stackchain/api/issues/7#issuecomment-9",
|
||||
},
|
||||
"issue": {"number": 7, "assignees": [], "claimable": True},
|
||||
"conversation": {
|
||||
"comments": [{
|
||||
"id": 9,
|
||||
|
|
|
|||
|
|
@ -915,7 +915,7 @@ async def test_claim_available_issue_endpoint_reports_assignment_race_as_conflic
|
|||
assert response.status_code == 409
|
||||
assert response.headers["cache-control"] == "no-store"
|
||||
assert response.json() == {
|
||||
"error": "This issue was already claimed or is no longer open. Refresh Find Work."
|
||||
"error": "This issue was already claimed or is no longer open."
|
||||
}
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -18,6 +18,94 @@ PULL_SHEET = Path(__file__).parents[1] / "frontend" / "pull-sheet.js"
|
|||
CONVERSATION = Path(__file__).parents[1] / "frontend" / "conversation.js"
|
||||
PICK_WORK = Path(__file__).parents[1] / "frontend" / "pick-work.js"
|
||||
WORK_ROUTE = Path(__file__).parents[1] / "frontend" / "work-route.js"
|
||||
UPDATE_OWNERSHIP = Path(__file__).parents[1] / "frontend" / "update-ownership.js"
|
||||
|
||||
|
||||
def test_unassigned_issue_update_claims_once_and_becomes_today_ready():
|
||||
script = f"""
|
||||
const createUpdateOwnership = require({json.dumps(str(UPDATE_OWNERSHIP))});
|
||||
let claims = 0;
|
||||
let finishClaim;
|
||||
const states = [];
|
||||
const controller = createUpdateOwnership({{
|
||||
claim: () => {{ claims += 1; return new Promise(resolve => {{ finishClaim = resolve; }}); }},
|
||||
addToday: () => 'added',
|
||||
onClaimed: item => states.push({{status:'reconciled', item}}),
|
||||
onState: state => states.push(state),
|
||||
}});
|
||||
controller.open({{
|
||||
repository:'stackchain/api', title:'Retry deploy',
|
||||
issue:{{number:7, assignees:[], claimable:true}},
|
||||
}}, {{notification_id:42, updated_at:'2026-08-08T12:00:00Z'}});
|
||||
const first = controller.act();
|
||||
const second = controller.act();
|
||||
if (first !== second || claims !== 1) throw new Error('claim was not single-flight');
|
||||
finishClaim({{number:7, title:'Retry deploy', assignees:['timmy'], state:'open'}});
|
||||
(async () => {{
|
||||
await first;
|
||||
const added = await controller.act();
|
||||
process.stdout.write(JSON.stringify({{claims, added, states}}));
|
||||
}})().catch(error => {{ console.error(error); process.exit(1); }});
|
||||
"""
|
||||
result = subprocess.run(
|
||||
["node", "-e", script], check=True, capture_output=True, text=True
|
||||
)
|
||||
output = json.loads(result.stdout)
|
||||
assert output["claims"] == 1
|
||||
assert output["added"] == "added"
|
||||
assert output["states"][0] == {"action": "claim", "busy": False, "message": ""}
|
||||
assert {"action": "claim", "busy": True, "message": "Assigning…"} in output["states"]
|
||||
reconciled = next(state for state in output["states"] if state.get("status") == "reconciled")
|
||||
assert reconciled["item"]["notification_id"] == 42
|
||||
assert reconciled["item"]["repository"] == "stackchain/api"
|
||||
assert reconciled["item"]["kind"] == "issue"
|
||||
assert output["states"][-1] == {
|
||||
"action": "today", "busy": False, "message": "Added to Today."
|
||||
}
|
||||
|
||||
|
||||
def test_update_ownership_conflict_removes_stale_action_without_reconciling():
|
||||
script = f"""
|
||||
const createUpdateOwnership = require({json.dumps(str(UPDATE_OWNERSHIP))});
|
||||
const states = [];
|
||||
let reconciled = 0;
|
||||
const conflict = new Error('server wording'); conflict.status = 409;
|
||||
const controller = createUpdateOwnership({{
|
||||
claim: () => Promise.reject(conflict), addToday: () => 'added',
|
||||
onClaimed: () => {{ reconciled += 1; }}, onState: state => states.push(state),
|
||||
}});
|
||||
controller.open({{repository:'stackchain/api', issue:{{number:7, claimable:true}}}}, {{notification_id:42}});
|
||||
(async () => {{
|
||||
const result = await controller.act();
|
||||
process.stdout.write(JSON.stringify({{result, reconciled, states}}));
|
||||
}})().catch(error => {{ console.error(error); process.exit(1); }});
|
||||
"""
|
||||
result = subprocess.run(
|
||||
["node", "-e", script], check=True, capture_output=True, text=True
|
||||
)
|
||||
output = json.loads(result.stdout)
|
||||
assert output["result"] == "conflict"
|
||||
assert output["reconciled"] == 0
|
||||
assert output["states"][-1] == {
|
||||
"action": "hidden",
|
||||
"busy": False,
|
||||
"message": "Someone else claimed or closed this issue. The update is still unread.",
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_mobile_update_sheet_wires_phone_safe_ownership_to_my_work_and_today():
|
||||
html = await dashboard()
|
||||
|
||||
assert '<script src="static/update-ownership.js"></script>' in html
|
||||
assert 'id="update-ownership-action"' in html
|
||||
assert 'hidden aria-describedby="update-sheet-status"' in html
|
||||
assert 'const updateOwnership = createUpdateOwnership({' in html
|
||||
assert 'updateOwnership.open(detail, selectedUpdate)' in html
|
||||
assert "qs('#update-ownership-action').addEventListener('click'" in html
|
||||
assert "lastContextSnapshot.issues" in html
|
||||
assert "todayWork.add(item)" in html
|
||||
assert '.update-sheet-actions button, .update-sheet-actions a { min-height:44px;' in html
|
||||
|
||||
|
||||
def test_work_routes_round_trip_all_sheet_kinds_and_reject_unsafe_fragments():
|
||||
|
|
|
|||
|
|
@ -36,7 +36,12 @@ async def test_notification_detail_opens_the_newest_conversation_page_in_chronol
|
|||
comments = [comment(i) for i in (range(1, 21) if page == 1 else range(41, 48))]
|
||||
return httpx.Response(200, json=comments, headers={"X-Total-Count": "47"})
|
||||
if request.url.path.endswith("/repos/stackchain/api/issues/7"):
|
||||
return httpx.Response(200, json={"body": "Deploy fails after retries."})
|
||||
return httpx.Response(200, json={
|
||||
"number": 7,
|
||||
"state": "open",
|
||||
"assignees": [],
|
||||
"body": "Deploy fails after retries.",
|
||||
})
|
||||
if request.url.path.endswith("/repos/stackchain/api/issues/comments/47"):
|
||||
return httpx.Response(200, json=comment(47))
|
||||
raise AssertionError(f"unexpected request: {request.url}")
|
||||
|
|
@ -56,6 +61,11 @@ async def test_notification_detail_opens_the_newest_conversation_page_in_chronol
|
|||
"older_page": 2,
|
||||
"total": 47,
|
||||
}
|
||||
assert result["issue"] == {
|
||||
"number": 7,
|
||||
"assignees": [],
|
||||
"claimable": True,
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
|
|
|
|||
|
|
@ -97,7 +97,7 @@ async function dispatchNotificationClick(route) {{
|
|||
def test_share_target_sign_in_fix_ships_in_a_new_shell_cache():
|
||||
source = WORKER.read_text()
|
||||
|
||||
assert "stackchain-dashboard-shell-v34" in source
|
||||
assert "stackchain-dashboard-shell-v35" in source
|
||||
assert "BASE + 'static/dashboard.css'" in source
|
||||
assert "BASE + 'static/dashboard.js'" in source
|
||||
assert "BASE + 'static/install-app.js'" in source
|
||||
|
|
@ -106,10 +106,17 @@ def test_share_target_sign_in_fix_ships_in_a_new_shell_cache():
|
|||
def test_mobile_search_viewport_ships_in_a_new_offline_shell():
|
||||
source = WORKER.read_text()
|
||||
|
||||
assert "stackchain-dashboard-shell-v34" in source
|
||||
assert "stackchain-dashboard-shell-v35" in source
|
||||
assert "BASE + 'static/mobile-search-viewport.js'" in source
|
||||
|
||||
|
||||
def test_update_ownership_flow_ships_atomically_in_a_new_offline_shell():
|
||||
source = WORKER.read_text()
|
||||
|
||||
assert "stackchain-dashboard-shell-v35" in source
|
||||
assert "BASE + 'static/update-ownership.js'" in source
|
||||
|
||||
|
||||
def test_background_sync_event_flushes_closed_app_issue_outbox_only_for_its_tag():
|
||||
result = run_worker_scenario(
|
||||
"""
|
||||
|
|
@ -279,6 +286,7 @@ def test_install_precaches_complete_subpath_scoped_app_shell():
|
|||
"/dashboard/static/offline-work.js",
|
||||
"/dashboard/static/my-work.js",
|
||||
"/dashboard/static/today-work.js",
|
||||
"/dashboard/static/update-ownership.js",
|
||||
"/dashboard/static/later-work.js",
|
||||
"/dashboard/static/detail-defer.js",
|
||||
"/dashboard/static/pick-work.js",
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user