Preview requested pull request diffs on mobile #124
|
|
@ -71,6 +71,14 @@ textarea { resize: vertical; min-height: 120px; }
|
|||
.review-sheet-header { display:flex; align-items:center; justify-content:space-between; gap:10px; flex-wrap:wrap; }
|
||||
.review-sheet-body { white-space:pre-wrap; overflow-wrap:anywhere; }
|
||||
.review-file, .review-history { padding:8px 0; border-bottom:1px solid #1b2d45; overflow-wrap:anywhere; }
|
||||
.review-file-toggle { min-height:44px; width:100%; display:flex; align-items:flex-start; justify-content:space-between; gap:8px; text-align:left; }
|
||||
.review-file-toggle strong { overflow-wrap:anywhere; }
|
||||
.review-diff { overflow-x:auto; max-width:100%; margin-top:8px; white-space:pre; }
|
||||
.review-diff-line { display:block; min-width:max-content; }
|
||||
.review-diff-line.hunk { color:#93c5fd; }
|
||||
.review-diff-line.added { color:#86efac; background:rgba(34,197,94,.09); }
|
||||
.review-diff-line.removed { color:#fca5a5; background:rgba(239,68,68,.09); }
|
||||
.review-diff-note, .review-diff-empty { display:block; padding:8px; color:#fcd34d; white-space:normal; }
|
||||
.review-action { min-height:44px; }
|
||||
@media (max-width: 600px) {
|
||||
header { align-items:flex-start; }
|
||||
|
|
@ -373,10 +381,15 @@ textarea { resize: vertical; min-height: 120px; }
|
|||
if (selectedReview !== item) return;
|
||||
qs('#review-sheet-body').textContent = detail.body || 'No description provided.';
|
||||
qs('#review-ci-state').textContent = 'CI ' + (detail.ci_state || 'unknown');
|
||||
qs('#review-files').innerHTML = (detail.files || []).length ? detail.files.map(file =>
|
||||
'<div class="review-file"><strong>' + escapeHtml(file.filename || 'Unknown file') + '</strong><div class="small">' +
|
||||
escapeHtml(file.status || 'changed') + ' · +' + Number(file.additions || 0) + ' / −' + Number(file.deletions || 0) + '</div></div>'
|
||||
qs('#review-files').innerHTML = (detail.files || []).length ? detail.files.map((file, index) =>
|
||||
createReviewController.renderDiffFile(file, index, escapeHtml)
|
||||
).join('') : '<div>No changed files reported.</div>';
|
||||
document.querySelectorAll('.review-file-toggle').forEach(button => {
|
||||
button.addEventListener('click', () => {
|
||||
const panel = document.getElementById(button.getAttribute('aria-controls'));
|
||||
if (panel) createReviewController.toggleDiff(button, panel);
|
||||
});
|
||||
});
|
||||
qs('#review-history').innerHTML = (detail.reviews || []).length ? detail.reviews.map(review =>
|
||||
'<div class="review-history"><strong>' + escapeHtml(review.user?.login || 'Reviewer') + '</strong> · ' +
|
||||
escapeHtml(review.state || 'commented') + (review.body ? '<div class="small">' + escapeHtml(review.body) + '</div>' : '') + '</div>'
|
||||
|
|
|
|||
|
|
@ -15,6 +15,44 @@ function createReviewController({ fetchJson }) {
|
|||
return { load };
|
||||
}
|
||||
|
||||
function diffLineClass(line) {
|
||||
if (line.startsWith('@@')) return 'hunk';
|
||||
if (line.startsWith('+')) return 'added';
|
||||
if (line.startsWith('-')) return 'removed';
|
||||
return 'context';
|
||||
}
|
||||
|
||||
function renderDiffFile(file, index, escapeHtml) {
|
||||
const panelId = 'review-diff-' + index;
|
||||
let preview;
|
||||
if (file.diff_available) {
|
||||
const lines = (file.diff_lines || []).map(line =>
|
||||
'<span class="review-diff-line ' + diffLineClass(String(line)) + '">' +
|
||||
escapeHtml(String(line)) + '</span>'
|
||||
).join('');
|
||||
preview = '<pre class="review-diff" id="' + panelId + '" hidden>' + lines +
|
||||
(file.diff_truncated ? '<span class="review-diff-note">Preview truncated · open in Gitea for the full diff.</span>' : '') +
|
||||
'</pre>';
|
||||
} else {
|
||||
const message = file.diff_binary ? 'Binary file · preview unavailable.' : 'Diff preview unavailable.';
|
||||
preview = '<div class="review-diff-empty" id="' + panelId + '" hidden>' + message +
|
||||
(file.diff_truncated ? ' The response was truncated.' : '') + '</div>';
|
||||
}
|
||||
return '<div class="review-file"><button class="review-file-toggle" aria-expanded="false" aria-controls="' + panelId + '">' +
|
||||
'<strong>' + escapeHtml(file.filename || 'Unknown file') + '</strong><span class="small">' +
|
||||
escapeHtml(file.status || 'changed') + ' · +' + Number(file.additions || 0) + ' / −' +
|
||||
Number(file.deletions || 0) + '</span></button>' + preview + '</div>';
|
||||
}
|
||||
|
||||
function toggleDiff(button, panel) {
|
||||
const expanded = button.getAttribute('aria-expanded') === 'true';
|
||||
button.setAttribute('aria-expanded', String(!expanded));
|
||||
panel.hidden = expanded;
|
||||
}
|
||||
|
||||
createReviewController.renderDiffFile = renderDiffFile;
|
||||
createReviewController.toggleDiff = toggleDiff;
|
||||
|
||||
if (typeof module !== 'undefined' && module.exports) {
|
||||
module.exports = createReviewController;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,10 +1,13 @@
|
|||
import os
|
||||
import shlex
|
||||
from typing import Any
|
||||
|
||||
import httpx
|
||||
|
||||
GITEA_URL = os.getenv("GITEA_URL", "http://127.0.0.1:3000").rstrip("/")
|
||||
GITEA_TOKEN = os.getenv("GITEA_TOKEN", "")
|
||||
REVIEW_DIFF_MAX_BYTES = 64 * 1024
|
||||
REVIEW_DIFF_MAX_LINES = 400
|
||||
|
||||
|
||||
def _auth() -> dict[str, str]:
|
||||
|
|
@ -21,6 +24,66 @@ async def fetch(path: str) -> Any:
|
|||
return r.json()
|
||||
|
||||
|
||||
async def fetch_text(path: str, max_bytes: int) -> tuple[str, bool]:
|
||||
chunks: list[bytes] = []
|
||||
size = 0
|
||||
truncated = False
|
||||
async with httpx.AsyncClient(base_url=GITEA_URL, timeout=10) as client:
|
||||
async with client.stream(
|
||||
"GET", f"/api/v1/{path}", headers={**_auth(), "Accept": "text/plain"}
|
||||
) as response:
|
||||
response.raise_for_status()
|
||||
async for chunk in response.aiter_bytes():
|
||||
remaining = max_bytes - size
|
||||
if len(chunk) > remaining:
|
||||
chunks.append(chunk[:remaining])
|
||||
truncated = True
|
||||
break
|
||||
chunks.append(chunk)
|
||||
size += len(chunk)
|
||||
return b"".join(chunks).decode("utf-8", errors="replace"), truncated
|
||||
|
||||
|
||||
def _diff_previews(diff: str, stream_truncated: bool) -> dict[str, dict]:
|
||||
previews: dict[str, dict] = {}
|
||||
current: dict | None = None
|
||||
in_hunk = False
|
||||
remaining = REVIEW_DIFF_MAX_LINES
|
||||
for line in diff.splitlines():
|
||||
if line.startswith("diff --git "):
|
||||
try:
|
||||
target = shlex.split(line)[3]
|
||||
filename = target[2:] if target.startswith("b/") else target
|
||||
except (IndexError, ValueError):
|
||||
current = None
|
||||
continue
|
||||
current = {
|
||||
"diff_lines": [],
|
||||
"diff_available": False,
|
||||
"diff_binary": False,
|
||||
"diff_truncated": stream_truncated,
|
||||
}
|
||||
previews[filename] = current
|
||||
in_hunk = False
|
||||
continue
|
||||
if current is None:
|
||||
continue
|
||||
if line.startswith("Binary files ") or line == "GIT binary patch":
|
||||
current["diff_binary"] = True
|
||||
in_hunk = False
|
||||
continue
|
||||
if line.startswith("@@"):
|
||||
in_hunk = True
|
||||
if in_hunk and not line.startswith("\\ No newline at end of file"):
|
||||
if remaining:
|
||||
current["diff_lines"].append(line)
|
||||
current["diff_available"] = True
|
||||
remaining -= 1
|
||||
else:
|
||||
current["diff_truncated"] = True
|
||||
return previews
|
||||
|
||||
|
||||
async def current_user() -> dict:
|
||||
return await fetch("user")
|
||||
|
||||
|
|
@ -79,6 +142,10 @@ async def pull_review_detail(repository: str, number: int) -> dict:
|
|||
sha = sha_value if isinstance(sha_value, str) else ""
|
||||
status = await fetch(f"repos/{repository}/commits/{sha}/status")
|
||||
reviews = await fetch(f"{base}/reviews")
|
||||
diff, diff_truncated = await fetch_text(
|
||||
f"repos/{repository}/pulls/{number}.diff", REVIEW_DIFF_MAX_BYTES
|
||||
)
|
||||
previews = _diff_previews(diff, diff_truncated)
|
||||
user_value = pull.get("user")
|
||||
user: dict = user_value if isinstance(user_value, dict) else {}
|
||||
normalized_files = [
|
||||
|
|
@ -87,6 +154,15 @@ async def pull_review_detail(repository: str, number: int) -> dict:
|
|||
"status": file.get("status") or "changed",
|
||||
"additions": file.get("additions") or 0,
|
||||
"deletions": file.get("deletions") or 0,
|
||||
**previews.get(
|
||||
file["filename"],
|
||||
{
|
||||
"diff_lines": [],
|
||||
"diff_available": False,
|
||||
"diff_binary": False,
|
||||
"diff_truncated": diff_truncated,
|
||||
},
|
||||
),
|
||||
}
|
||||
for file in (files if isinstance(files, list) else [])[:100]
|
||||
if isinstance(file, dict) and isinstance(file.get("filename"), str)
|
||||
|
|
|
|||
|
|
@ -159,6 +159,13 @@ async def test_pull_review_detail_combines_pr_files_status_and_reviews(monkeypat
|
|||
|
||||
monkeypatch.setattr(gitea_proxy, "fetch", fake_fetch)
|
||||
|
||||
async def fake_fetch_text(path, max_bytes):
|
||||
assert path == "repos/stackchain/api/pulls/7.diff"
|
||||
assert max_bytes == gitea_proxy.REVIEW_DIFF_MAX_BYTES
|
||||
return "", False
|
||||
|
||||
monkeypatch.setattr(gitea_proxy, "fetch_text", fake_fetch_text)
|
||||
|
||||
detail = await gitea_proxy.pull_review_detail("stackchain/api", 7)
|
||||
|
||||
assert requested_paths == [
|
||||
|
|
@ -173,3 +180,55 @@ async def test_pull_review_detail_combines_pr_files_status_and_reviews(monkeypat
|
|||
assert detail["reviews"][0]["state"] == "APPROVED"
|
||||
assert len(detail["files"]) == 1
|
||||
assert len(detail["reviews"]) == 1
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_pull_review_detail_attaches_bounded_per_file_diff_previews(monkeypatch):
|
||||
async def fake_fetch(path):
|
||||
if path.endswith("/pulls/7"):
|
||||
return {
|
||||
"title": "Review API",
|
||||
"head": {"sha": "abc123"},
|
||||
"user": {"login": "alex"},
|
||||
}
|
||||
if path.endswith("/files"):
|
||||
return [
|
||||
{"filename": "src/api.py", "status": "modified"},
|
||||
{"filename": "assets/logo.png", "status": "modified"},
|
||||
]
|
||||
if path.endswith("/reviews"):
|
||||
return []
|
||||
return {"state": "success"}
|
||||
|
||||
diff = """diff --git a/src/api.py b/src/api.py
|
||||
index 123..456 100644
|
||||
--- a/src/api.py
|
||||
+++ b/src/api.py
|
||||
@@ -1,2 +1,3 @@
|
||||
context
|
||||
-old <value>
|
||||
+new <value>
|
||||
diff --git a/assets/logo.png b/assets/logo.png
|
||||
Binary files a/assets/logo.png and b/assets/logo.png differ
|
||||
"""
|
||||
|
||||
async def fake_fetch_text(path, max_bytes):
|
||||
assert path == "repos/stackchain/api/pulls/7.diff"
|
||||
return diff, True
|
||||
|
||||
monkeypatch.setattr(gitea_proxy, "fetch", fake_fetch)
|
||||
monkeypatch.setattr(gitea_proxy, "fetch_text", fake_fetch_text)
|
||||
|
||||
detail = await gitea_proxy.pull_review_detail("stackchain/api", 7)
|
||||
|
||||
source, binary = detail["files"]
|
||||
assert source["diff_lines"] == [
|
||||
"@@ -1,2 +1,3 @@",
|
||||
" context",
|
||||
"-old <value>",
|
||||
"+new <value>",
|
||||
]
|
||||
assert source["diff_truncated"] is True
|
||||
assert source["diff_available"] is True
|
||||
assert binary["diff_available"] is False
|
||||
assert binary["diff_binary"] is True
|
||||
|
|
|
|||
|
|
@ -172,6 +172,35 @@ controller.load({{ repository: 'stackchain/api', number: 7 }}).then(detail =>
|
|||
}
|
||||
|
||||
|
||||
def test_review_diff_rows_escape_content_and_toggle_accessibly():
|
||||
script = f"""
|
||||
const reviewSheet = require({json.dumps(str(REVIEW_SHEET))});
|
||||
const escapeHtml = value => String(value)
|
||||
.replaceAll('&', '&').replaceAll('<', '<').replaceAll('>', '>');
|
||||
const html = reviewSheet.renderDiffFile({{
|
||||
filename: 'src/<api>.py', status: 'modified', additions: 1, deletions: 1,
|
||||
diff_available: true, diff_truncated: true,
|
||||
diff_lines: ['@@ -1 +1 @@', '-old <token>', '+new & safe']
|
||||
}}, 2, escapeHtml);
|
||||
const button = {{ attrs: {{ 'aria-expanded': 'false' }}, getAttribute(k) {{ return this.attrs[k]; }}, setAttribute(k,v) {{ this.attrs[k]=v; }} }};
|
||||
const panel = {{ hidden: true }};
|
||||
reviewSheet.toggleDiff(button, panel);
|
||||
process.stdout.write(JSON.stringify({{ html, expanded: button.attrs['aria-expanded'], hidden: panel.hidden }}));
|
||||
"""
|
||||
result = subprocess.run(
|
||||
["node", "-e", script], check=True, capture_output=True, text=True
|
||||
)
|
||||
output = json.loads(result.stdout)
|
||||
|
||||
assert 'aria-expanded="false"' in output["html"]
|
||||
assert 'src/<api>.py' in output["html"]
|
||||
assert '-old <token>' in output["html"]
|
||||
assert '+new & safe' in output["html"]
|
||||
assert 'Preview truncated' in output["html"]
|
||||
assert output["expanded"] == "true"
|
||||
assert output["hidden"] is False
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_review_requests_open_an_accessible_mobile_detail_sheet():
|
||||
html = await dashboard()
|
||||
|
|
@ -184,6 +213,8 @@ async def test_review_requests_open_an_accessible_mobile_detail_sheet():
|
|||
assert '@media (max-width: 600px)' in html
|
||||
assert '.review-sheet-panel' in html and 'width:100%' in html
|
||||
assert '.review-action' in html and 'min-height:44px' in html
|
||||
assert '.review-file-toggle' in html and 'min-height:44px' in html
|
||||
assert '.review-diff' in html and 'overflow-x:auto' in html
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user