Merge pull request 'Add review requests to the mobile My Work inbox' (#118) from timmy/117-mobile-review-inbox into main
This commit is contained in:
commit
b5830cf86f
|
|
@ -99,6 +99,7 @@ textarea { resize: vertical; min-height: 120px; }
|
||||||
<button class="work-filter" data-work-filter="all" aria-pressed="true">All</button>
|
<button class="work-filter" data-work-filter="all" aria-pressed="true">All</button>
|
||||||
<button class="work-filter" data-work-filter="issue" aria-pressed="false">Issues</button>
|
<button class="work-filter" data-work-filter="issue" aria-pressed="false">Issues</button>
|
||||||
<button class="work-filter" data-work-filter="pull" aria-pressed="false">PRs</button>
|
<button class="work-filter" data-work-filter="pull" aria-pressed="false">PRs</button>
|
||||||
|
<button class="work-filter" data-work-filter="review" aria-pressed="false">Reviews</button>
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
<div class="my-work-list" id="my-work-list"></div>
|
<div class="my-work-list" id="my-work-list"></div>
|
||||||
|
|
@ -272,14 +273,12 @@ textarea { resize: vertical; min-height: 120px; }
|
||||||
lastMyWork = buildMyWork(data);
|
lastMyWork = buildMyWork(data);
|
||||||
qs('#my-work').removeAttribute('data-stale');
|
qs('#my-work').removeAttribute('data-stale');
|
||||||
qs('#my-work-status').textContent = lastMyWork.length ?
|
qs('#my-work-status').textContent = lastMyWork.length ?
|
||||||
lastMyWork.length + ' assigned item' + (lastMyWork.length === 1 ? '' : 's') :
|
summarizeMyWork(lastMyWork) : 'No assigned work or review requests.';
|
||||||
'No assigned work.';
|
|
||||||
renderMyWork();
|
renderMyWork();
|
||||||
}
|
}
|
||||||
|
|
||||||
function renderMyWork() {
|
function renderMyWork() {
|
||||||
const visible = selectedWorkFilter === 'all' ? lastMyWork :
|
const visible = filterMyWork(lastMyWork, selectedWorkFilter);
|
||||||
lastMyWork.filter(item => item.kind === selectedWorkFilter);
|
|
||||||
qs('#my-work-list').innerHTML = visible.length ? visible.map(item =>
|
qs('#my-work-list').innerHTML = visible.length ? visible.map(item =>
|
||||||
'<a class="my-work-card" href="' + escAttr(item.url) + '" target="_blank" rel="noopener noreferrer">' +
|
'<a class="my-work-card" href="' + escAttr(item.url) + '" target="_blank" rel="noopener noreferrer">' +
|
||||||
'<span class="small">' + escapeHtml(item.key) + ' · ' + escapeHtml(item.kind === 'pull' ? 'PR' : 'Issue') + '</span>' +
|
'<span class="small">' + escapeHtml(item.key) + ' · ' + escapeHtml(item.kind === 'pull' ? 'PR' : 'Issue') + '</span>' +
|
||||||
|
|
@ -287,13 +286,13 @@ textarea { resize: vertical; min-height: 120px; }
|
||||||
'<span class="pill">' + escapeHtml(item.reason) + '</span>' +
|
'<span class="pill">' + escapeHtml(item.reason) + '</span>' +
|
||||||
(item.updated_at ? '<span class="small"> · Updated ' + escapeHtml(fmt(item.updated_at)) + '</span>' : '') +
|
(item.updated_at ? '<span class="small"> · Updated ' + escapeHtml(fmt(item.updated_at)) + '</span>' : '') +
|
||||||
'</a>'
|
'</a>'
|
||||||
).join('') : '<div class="muted">No ' + (selectedWorkFilter === 'all' ? '' : selectedWorkFilter + ' ') + 'items.</div>';
|
).join('') : '<div class="muted">No ' + (selectedWorkFilter === 'review' ? 'reviews' : (selectedWorkFilter === 'all' ? 'work' : selectedWorkFilter + ' items')) + '.</div>';
|
||||||
}
|
}
|
||||||
|
|
||||||
function markMyWorkStale() {
|
function markMyWorkStale() {
|
||||||
qs('#my-work').setAttribute('data-stale', 'true');
|
qs('#my-work').setAttribute('data-stale', 'true');
|
||||||
qs('#my-work-status').textContent = lastMyWork.length ?
|
qs('#my-work-status').textContent = lastMyWork.length ?
|
||||||
'Update failed · showing last assigned work' : 'Assigned work unavailable.';
|
'Update failed · showing last known work' : 'Work inbox unavailable.';
|
||||||
}
|
}
|
||||||
|
|
||||||
function paintDeltas(deltas) {
|
function paintDeltas(deltas) {
|
||||||
|
|
|
||||||
|
|
@ -10,11 +10,15 @@ function buildMyWork(data) {
|
||||||
priorityLabels.includes(String(label).toLowerCase())
|
priorityLabels.includes(String(label).toLowerCase())
|
||||||
);
|
);
|
||||||
const assigned = (item.assignees || []).includes(login);
|
const assigned = (item.assignees || []).includes(login);
|
||||||
|
const isReview = (item.work_reasons || []).includes('review_requested');
|
||||||
return {
|
return {
|
||||||
...item,
|
...item,
|
||||||
key: (item.repository || 'unknown') + '#' + item.number,
|
key: (item.repository || 'unknown') + '#' + item.number,
|
||||||
reason: priorityLabel ? priorityLabel + ' priority' : (assigned ? 'Assigned to you' : 'Open work'),
|
is_review: isReview,
|
||||||
_priority: priorityLabel ? 0 : (assigned ? 1 : 2),
|
is_assigned: assigned,
|
||||||
|
reason: priorityLabel ? priorityLabel + ' priority' :
|
||||||
|
(isReview ? 'Needs your review' : (assigned ? 'Assigned to you' : 'Open work')),
|
||||||
|
_priority: priorityLabel ? 0 : (isReview ? 1 : (assigned ? 2 : 3)),
|
||||||
};
|
};
|
||||||
}).sort((left, right) =>
|
}).sort((left, right) =>
|
||||||
left._priority - right._priority ||
|
left._priority - right._priority ||
|
||||||
|
|
@ -23,6 +27,22 @@ function buildMyWork(data) {
|
||||||
).map(({ _priority, ...item }) => item);
|
).map(({ _priority, ...item }) => item);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function filterMyWork(items, selectedFilter) {
|
||||||
|
if (selectedFilter === 'all') return items;
|
||||||
|
if (selectedFilter === 'review') return items.filter((item) => item.is_review);
|
||||||
|
return items.filter((item) => item.kind === selectedFilter);
|
||||||
|
}
|
||||||
|
|
||||||
|
function summarizeMyWork(items) {
|
||||||
|
const reviews = items.filter((item) => item.is_review).length;
|
||||||
|
const assigned = items.filter((item) => item.is_assigned).length;
|
||||||
|
const reviewLabel = reviews + ' review' + (reviews === 1 ? '' : 's');
|
||||||
|
const assignedLabel = assigned + ' assigned';
|
||||||
|
return reviewLabel + ' · ' + assignedLabel;
|
||||||
|
}
|
||||||
|
|
||||||
if (typeof module !== 'undefined' && module.exports) {
|
if (typeof module !== 'undefined' && module.exports) {
|
||||||
|
buildMyWork.filterMyWork = filterMyWork;
|
||||||
|
buildMyWork.summarizeMyWork = summarizeMyWork;
|
||||||
module.exports = buildMyWork;
|
module.exports = buildMyWork;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -36,9 +36,23 @@ async def issues() -> list[dict]:
|
||||||
|
|
||||||
|
|
||||||
async def pull_requests() -> list[dict]:
|
async def pull_requests() -> list[dict]:
|
||||||
return await fetch(
|
assigned = await fetch(
|
||||||
"repos/issues/search?state=open&assigned=true&type=pulls&limit=50"
|
"repos/issues/search?state=open&assigned=true&type=pulls&limit=50"
|
||||||
)
|
)
|
||||||
|
review_requested = await fetch(
|
||||||
|
"repos/issues/search?state=open&review_requested=true&type=pulls&limit=50"
|
||||||
|
)
|
||||||
|
merged: dict[int, dict] = {}
|
||||||
|
for reason, pulls in (
|
||||||
|
("assigned_to_me", assigned or []),
|
||||||
|
("review_requested", review_requested or []),
|
||||||
|
):
|
||||||
|
for pull in pulls:
|
||||||
|
identity = pull.get("id")
|
||||||
|
if identity not in merged:
|
||||||
|
merged[identity] = {**pull, "work_reasons": []}
|
||||||
|
merged[identity]["work_reasons"].append(reason)
|
||||||
|
return list(merged.values())
|
||||||
|
|
||||||
|
|
||||||
async def activity_events() -> list[dict]:
|
async def activity_events() -> list[dict]:
|
||||||
|
|
|
||||||
|
|
@ -192,6 +192,11 @@ async def context() -> JSONResponse:
|
||||||
for assignee in (p.get("assignees") or [])
|
for assignee in (p.get("assignees") or [])
|
||||||
if isinstance(assignee, dict)
|
if isinstance(assignee, dict)
|
||||||
],
|
],
|
||||||
|
work_reasons=[
|
||||||
|
reason
|
||||||
|
for reason in (p.get("work_reasons") or [])
|
||||||
|
if reason in ("assigned_to_me", "review_requested")
|
||||||
|
],
|
||||||
repository=(
|
repository=(
|
||||||
p["repository"].get("full_name", "")
|
p["repository"].get("full_name", "")
|
||||||
if isinstance(p.get("repository"), dict)
|
if isinstance(p.get("repository"), dict)
|
||||||
|
|
|
||||||
|
|
@ -37,6 +37,7 @@ class PullRequest(BaseModel):
|
||||||
user: str
|
user: str
|
||||||
labels: list[str] = []
|
labels: list[str] = []
|
||||||
assignees: list[str] = []
|
assignees: list[str] = []
|
||||||
|
work_reasons: list[str] = []
|
||||||
repository: str = ""
|
repository: str = ""
|
||||||
updated_at: str = ""
|
updated_at: str = ""
|
||||||
url: str
|
url: str
|
||||||
|
|
|
||||||
|
|
@ -6,7 +6,7 @@ from src import main
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.anyio
|
@pytest.mark.anyio
|
||||||
async def test_work_collections_use_supported_assigned_search_endpoint(monkeypatch):
|
async def test_work_collections_include_supported_review_request_search(monkeypatch):
|
||||||
requested_paths = []
|
requested_paths = []
|
||||||
|
|
||||||
async def fake_fetch(path):
|
async def fake_fetch(path):
|
||||||
|
|
@ -20,9 +20,39 @@ async def test_work_collections_use_supported_assigned_search_endpoint(monkeypat
|
||||||
assert requested_paths == [
|
assert requested_paths == [
|
||||||
"repos/issues/search?state=open&assigned=true&type=issues&limit=50",
|
"repos/issues/search?state=open&assigned=true&type=issues&limit=50",
|
||||||
"repos/issues/search?state=open&assigned=true&type=pulls&limit=50",
|
"repos/issues/search?state=open&assigned=true&type=pulls&limit=50",
|
||||||
|
"repos/issues/search?state=open&review_requested=true&type=pulls&limit=50",
|
||||||
]
|
]
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.anyio
|
||||||
|
async def test_pull_requests_merge_assignment_and_review_responsibilities(monkeypatch):
|
||||||
|
assigned = {
|
||||||
|
"id": 11,
|
||||||
|
"number": 7,
|
||||||
|
"title": "Review API",
|
||||||
|
"repository": {"full_name": "stackchain/api"},
|
||||||
|
}
|
||||||
|
review_only = {
|
||||||
|
"id": 12,
|
||||||
|
"number": 8,
|
||||||
|
"title": "Review mobile",
|
||||||
|
"repository": {"full_name": "stackchain/mobile"},
|
||||||
|
}
|
||||||
|
|
||||||
|
async def fake_fetch(path):
|
||||||
|
if "assigned=true" in path:
|
||||||
|
return [assigned]
|
||||||
|
return [assigned.copy(), review_only]
|
||||||
|
|
||||||
|
monkeypatch.setattr(gitea_proxy, "fetch", fake_fetch)
|
||||||
|
|
||||||
|
pulls = await gitea_proxy.pull_requests()
|
||||||
|
|
||||||
|
assert [pull["id"] for pull in pulls] == [11, 12]
|
||||||
|
assert pulls[0]["work_reasons"] == ["assigned_to_me", "review_requested"]
|
||||||
|
assert pulls[1]["work_reasons"] == ["review_requested"]
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.anyio
|
@pytest.mark.anyio
|
||||||
async def test_context_preserves_repository_and_update_time_for_cross_repo_work(monkeypatch):
|
async def test_context_preserves_repository_and_update_time_for_cross_repo_work(monkeypatch):
|
||||||
async def user():
|
async def user():
|
||||||
|
|
@ -53,6 +83,7 @@ async def test_context_preserves_repository_and_update_time_for_cross_repo_work(
|
||||||
"user": {"login": "alex"},
|
"user": {"login": "alex"},
|
||||||
"labels": [{"name": "priority-high"}],
|
"labels": [{"name": "priority-high"}],
|
||||||
"assignees": [{"login": "timmy"}],
|
"assignees": [{"login": "timmy"}],
|
||||||
|
"work_reasons": ["assigned_to_me", "review_requested"],
|
||||||
"repository": {"full_name": "stackchain/api"},
|
"repository": {"full_name": "stackchain/api"},
|
||||||
"updated_at": "2026-08-06T11:00:00Z",
|
"updated_at": "2026-08-06T11:00:00Z",
|
||||||
"html_url": "https://forge.example/stackchain/api/pulls/7",
|
"html_url": "https://forge.example/stackchain/api/pulls/7",
|
||||||
|
|
@ -71,3 +102,7 @@ async def test_context_preserves_repository_and_update_time_for_cross_repo_work(
|
||||||
assert payload["pull_requests"][0]["updated_at"] == "2026-08-06T11:00:00Z"
|
assert payload["pull_requests"][0]["updated_at"] == "2026-08-06T11:00:00Z"
|
||||||
assert payload["pull_requests"][0]["labels"] == ["priority-high"]
|
assert payload["pull_requests"][0]["labels"] == ["priority-high"]
|
||||||
assert payload["pull_requests"][0]["assignees"] == ["timmy"]
|
assert payload["pull_requests"][0]["assignees"] == ["timmy"]
|
||||||
|
assert payload["pull_requests"][0]["work_reasons"] == [
|
||||||
|
"assigned_to_me",
|
||||||
|
"review_requested",
|
||||||
|
]
|
||||||
|
|
|
||||||
|
|
@ -10,7 +10,7 @@ from src.views import dashboard
|
||||||
MY_WORK = Path(__file__).parents[1] / "frontend" / "my-work.js"
|
MY_WORK = Path(__file__).parents[1] / "frontend" / "my-work.js"
|
||||||
|
|
||||||
|
|
||||||
def test_my_work_queue_prioritizes_labels_then_assignment_and_keeps_repo_identity():
|
def test_my_work_queue_prioritizes_labels_then_reviews_and_keeps_repo_identity():
|
||||||
payload = {
|
payload = {
|
||||||
"user": {"login": "timmy"},
|
"user": {"login": "timmy"},
|
||||||
"issues": [
|
"issues": [
|
||||||
|
|
@ -44,6 +44,7 @@ def test_my_work_queue_prioritizes_labels_then_assignment_and_keeps_repo_identit
|
||||||
"title": "Review PR",
|
"title": "Review PR",
|
||||||
"state": "open",
|
"state": "open",
|
||||||
"repository": "stackchain/web",
|
"repository": "stackchain/web",
|
||||||
|
"work_reasons": ["review_requested"],
|
||||||
"updated_at": "2026-08-06T13:00:00Z",
|
"updated_at": "2026-08-06T13:00:00Z",
|
||||||
"url": "https://forge.example/web/pulls/4",
|
"url": "https://forge.example/web/pulls/4",
|
||||||
}
|
}
|
||||||
|
|
@ -62,13 +63,38 @@ process.stdout.write(JSON.stringify(queue));
|
||||||
|
|
||||||
assert [item["title"] for item in queue] == [
|
assert [item["title"] for item in queue] == [
|
||||||
"Priority issue",
|
"Priority issue",
|
||||||
"Assigned issue",
|
|
||||||
"Review PR",
|
"Review PR",
|
||||||
|
"Assigned issue",
|
||||||
]
|
]
|
||||||
assert queue[0]["key"] == "stackchain/api#7"
|
assert queue[0]["key"] == "stackchain/api#7"
|
||||||
assert queue[0]["reason"] == "P0 priority"
|
assert queue[0]["reason"] == "P0 priority"
|
||||||
assert queue[1]["reason"] == "Assigned to you"
|
assert queue[1]["reason"] == "Needs your review"
|
||||||
assert queue[2]["kind"] == "pull"
|
assert queue[1]["is_review"] is True
|
||||||
|
assert queue[2]["reason"] == "Assigned to you"
|
||||||
|
|
||||||
|
|
||||||
|
def test_my_work_reviews_filter_and_summary_are_actionable():
|
||||||
|
items = [
|
||||||
|
{"title": "Issue", "kind": "issue", "is_review": False, "is_assigned": True},
|
||||||
|
{"title": "Assigned PR", "kind": "pull", "is_review": False, "is_assigned": True},
|
||||||
|
{"title": "Review PR", "kind": "pull", "is_review": True, "is_assigned": False},
|
||||||
|
]
|
||||||
|
script = f"""
|
||||||
|
const buildMyWork = require({json.dumps(str(MY_WORK))});
|
||||||
|
const items = {json.dumps(items)};
|
||||||
|
process.stdout.write(JSON.stringify({{
|
||||||
|
reviews: buildMyWork.filterMyWork(items, 'review'),
|
||||||
|
summary: buildMyWork.summarizeMyWork(items),
|
||||||
|
}}));
|
||||||
|
"""
|
||||||
|
|
||||||
|
result = subprocess.run(
|
||||||
|
["node", "-e", script], check=True, capture_output=True, text=True
|
||||||
|
)
|
||||||
|
output = json.loads(result.stdout)
|
||||||
|
|
||||||
|
assert [item["title"] for item in output["reviews"]] == ["Review PR"]
|
||||||
|
assert output["summary"] == "1 review · 2 assigned"
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.anyio
|
@pytest.mark.anyio
|
||||||
|
|
@ -79,8 +105,10 @@ async def test_mobile_dashboard_puts_filterable_my_work_before_auxiliary_panels(
|
||||||
assert 'data-work-filter="all"' in html
|
assert 'data-work-filter="all"' in html
|
||||||
assert 'data-work-filter="issue"' in html
|
assert 'data-work-filter="issue"' in html
|
||||||
assert 'data-work-filter="pull"' in html
|
assert 'data-work-filter="pull"' in html
|
||||||
|
assert 'data-work-filter="review"' in html
|
||||||
assert '.work-filter' in html and 'min-height: 44px' in html
|
assert '.work-filter' in html and 'min-height: 44px' in html
|
||||||
assert '.my-work-card' in html and 'min-height: 44px' in html
|
assert '.my-work-card' in html and 'min-height: 44px' in html
|
||||||
assert '<script src="static/my-work.js"></script>' in html
|
assert '<script src="static/my-work.js"></script>' in html
|
||||||
assert "buildMyWork(data)" in html
|
assert "buildMyWork(data)" in html
|
||||||
assert "markMyWorkStale()" in html
|
assert "markMyWorkStale()" in html
|
||||||
|
assert "filterMyWork(lastMyWork, selectedWorkFilter)" in html
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user