feat: recover closed authored pulls from search (Closes #1358)
This commit is contained in:
timmy committed
1 parent
e4d0cc9115
commit
99750d44f5
8 files changed
+410
-15
No files matched your search
@@ -0,0 +1,79 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import os
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
if os.getenv("STACKCHAIN_RUN_RELEASE_E2E") != "1":
|
||||
pytest.skip("rendered authored-pull recovery checks run only in the browser gate", allow_module_level=True)
|
||||
pytest.importorskip("playwright.sync_api")
|
||||
from playwright.sync_api import expect, sync_playwright
|
||||
|
||||
|
||||
ROOT = Path(__file__).resolve().parents[2]
|
||||
FRONTEND = ROOT / "frontend"
|
||||
|
||||
|
||||
@pytest.mark.parametrize(("width", "height"), [(320, 568), (390, 844)])
|
||||
def test_mobile_search_reopens_closed_authored_pull_into_my_work(width, height):
|
||||
html = re.sub(r'<script src="static/[^"]+"></script>', "", (FRONTEND / "index.html").read_text())
|
||||
|
||||
with sync_playwright() as playwright:
|
||||
browser = playwright.chromium.launch()
|
||||
page = browser.new_page(viewport={"width": width, "height": height})
|
||||
page.set_content(html)
|
||||
page.add_style_tag(path=FRONTEND / "dashboard.css")
|
||||
page.add_script_tag(path=FRONTEND / "search-preview.js")
|
||||
page.locator("#search-preview").evaluate("node => node.classList.add('open')")
|
||||
page.evaluate("""() => {
|
||||
const detail = {
|
||||
repository:'stackchain/web', number:9, kind:'pull', state:'closed',
|
||||
authored_pull_reopenable:true, head_sha:'abc1234',
|
||||
};
|
||||
const button = document.querySelector('#start-search-result');
|
||||
globalThis.renderSearchPreviewStart(detail, {status:'ready'}, button);
|
||||
globalThis.recoveryMetrics = {calls:[], history:['search']};
|
||||
const mutate = globalThis.searchPreviewMutation((path, options) => {
|
||||
globalThis.recoveryMetrics.calls.push({path, body:JSON.parse(options.body)});
|
||||
return Promise.resolve({...detail, state:'open'});
|
||||
});
|
||||
const preview = globalThis.createSearchPreview({
|
||||
fetchJson:async () => detail,
|
||||
mutate,
|
||||
onState:() => {},
|
||||
});
|
||||
preview.open(detail);
|
||||
const recover = globalThis.createSearchAuthoredPullRecovery({
|
||||
confirm:message => window.confirm(message),
|
||||
reopen:target => preview.reopenPull(target),
|
||||
refresh:async () => { globalThis.recoveryMetrics.refreshed = true; },
|
||||
find:() => ({repository:'stackchain/web',number:9,kind:'pull',key:'stackchain/web#9'}),
|
||||
open:item => globalThis.recoveryMetrics.history.push(item.kind),
|
||||
unavailable:message => { globalThis.recoveryMetrics.unavailable = message; },
|
||||
});
|
||||
button.addEventListener('click', () => recover(detail));
|
||||
}""")
|
||||
|
||||
action = page.locator("#start-search-result")
|
||||
expect(action).to_be_visible()
|
||||
expect(action).to_have_text("Reopen in My Work")
|
||||
bounds = action.bounding_box()
|
||||
assert bounds and bounds["height"] >= 44
|
||||
assert page.evaluate("document.documentElement.scrollWidth <= window.innerWidth")
|
||||
|
||||
page.once("dialog", lambda dialog: dialog.accept())
|
||||
action.click()
|
||||
page.wait_for_function("globalThis.recoveryMetrics.history.length === 2")
|
||||
metrics = page.evaluate("globalThis.recoveryMetrics")
|
||||
|
||||
assert metrics["calls"] == [{
|
||||
"path": "api/v1/repos/stackchain/web/pulls/9/reopen",
|
||||
"body": {"expected_head_sha": "abc1234"},
|
||||
}]
|
||||
assert metrics["refreshed"] is True
|
||||
assert metrics["history"] == ["search", "pull"]
|
||||
assert "unavailable" not in metrics
|
||||
browser.close()
|
||||
@@ -107,3 +107,10 @@ def test_browser_job_gates_authored_pull_close_recovery_journey():
|
||||
browser = text[text.index(" browser-journey:") : text.index(" release-candidate:")]
|
||||
|
||||
assert "tests/e2e/test_mobile_close_authored_pull_release.py" in browser
|
||||
|
||||
|
||||
def test_browser_job_gates_search_authored_pull_recovery_journey():
|
||||
text = WORKFLOW.read_text()
|
||||
browser = text[text.index(" browser-journey:") : text.index(" release-candidate:")]
|
||||
|
||||
assert "tests/e2e/test_mobile_search_authored_pull_recovery_release.py" in browser
|
||||
@@ -827,6 +827,143 @@ if (!states.some(state => state.status === 'reopened')) throw new Error('reopen
|
||||
subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True)
|
||||
|
||||
|
||||
def test_search_preview_closed_authored_pull_reopen_sends_head_guard_to_pull_api():
|
||||
script = f"""
|
||||
require({json.dumps(str(SEARCH_PREVIEW))});
|
||||
(async () => {{
|
||||
const calls = [];
|
||||
const mutate = globalThis.searchPreviewMutation(async (path, options) => {{
|
||||
calls.push({{path, options}});
|
||||
return {{repository:'stackchain/web',number:9,state:'open',head_sha:'abc1234'}};
|
||||
}});
|
||||
const detail = {{
|
||||
repository:'stackchain/web',number:9,kind:'pull',state:'closed',
|
||||
authored_pull_reopenable:true,head_sha:'abc1234',
|
||||
}};
|
||||
const result = await mutate(detail, 'reopen-pull');
|
||||
process.stdout.write(JSON.stringify({{calls,result}}));
|
||||
}})().catch(error => {{ console.error(error); process.exit(1); }});
|
||||
"""
|
||||
result = subprocess.run(["node", "-e", script], capture_output=True, text=True)
|
||||
|
||||
assert result.returncode == 0, result.stderr
|
||||
assert json.loads(result.stdout) == {
|
||||
"calls": [{
|
||||
"path": "api/v1/repos/stackchain/web/pulls/9/reopen",
|
||||
"options": {
|
||||
"method": "PATCH",
|
||||
"headers": {"Content-Type": "application/json"},
|
||||
"body": '{"expected_head_sha":"abc1234"}',
|
||||
},
|
||||
}],
|
||||
"result": {
|
||||
"repository": "stackchain/web",
|
||||
"number": 9,
|
||||
"state": "open",
|
||||
"head_sha": "abc1234",
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
def test_search_preview_closed_authored_pull_exposes_reopen_in_my_work_action():
|
||||
script = f"""
|
||||
require({json.dumps(str(SEARCH_PREVIEW))});
|
||||
const detail = {{
|
||||
repository:'stackchain/web',number:9,kind:'pull',state:'closed',
|
||||
authored_pull_reopenable:true,head_sha:'abc1234',
|
||||
}};
|
||||
const ready = {{hidden:true,textContent:'',disabled:false}};
|
||||
globalThis.renderSearchPreviewStart(detail, {{status:'ready'}}, ready);
|
||||
const pending = {{hidden:true,textContent:'',disabled:false}};
|
||||
globalThis.renderSearchPreviewStart(detail, {{status:'reopening'}}, pending);
|
||||
process.stdout.write(JSON.stringify({{ready,pending}}));
|
||||
"""
|
||||
result = subprocess.run(["node", "-e", script], capture_output=True, text=True)
|
||||
|
||||
assert result.returncode == 0, result.stderr
|
||||
assert json.loads(result.stdout) == {
|
||||
"ready": {
|
||||
"hidden": False,
|
||||
"textContent": "Reopen in My Work",
|
||||
"disabled": False,
|
||||
},
|
||||
"pending": {
|
||||
"hidden": False,
|
||||
"textContent": "Reopen in My Work",
|
||||
"disabled": True,
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
def test_search_preview_closed_authored_pull_reopen_is_single_flight():
|
||||
script = f"""
|
||||
const createSearchPreview = require({json.dumps(str(SEARCH_PREVIEW))});
|
||||
(async () => {{
|
||||
let calls = 0;
|
||||
let resolveMutation;
|
||||
const states = [];
|
||||
const detail = {{
|
||||
repository:'stackchain/web',number:9,kind:'pull',state:'closed',
|
||||
authored_pull_reopenable:true,head_sha:'abc1234',
|
||||
}};
|
||||
const preview = createSearchPreview({{
|
||||
fetchJson:async () => detail,
|
||||
mutate:(_detail, action) => {{
|
||||
if (action !== 'reopen-pull') throw new Error('wrong action');
|
||||
calls += 1;
|
||||
return new Promise(resolve => {{ resolveMutation = resolve; }});
|
||||
}},
|
||||
onState:state => states.push(state),
|
||||
}});
|
||||
await preview.open(detail);
|
||||
const first = preview.reopenPull(detail);
|
||||
const second = preview.reopenPull(detail);
|
||||
if (first !== second || calls !== 1) throw new Error('pull recovery was not single-flight');
|
||||
resolveMutation({{...detail,state:'open'}});
|
||||
await first;
|
||||
process.stdout.write(JSON.stringify({{calls,status:states.at(-1).status}}));
|
||||
}})().catch(error => {{ console.error(error); process.exit(1); }});
|
||||
"""
|
||||
result = subprocess.run(["node", "-e", script], capture_output=True, text=True)
|
||||
|
||||
assert result.returncode == 0, result.stderr
|
||||
assert json.loads(result.stdout) == {"calls": 1, "status": "reopened"}
|
||||
|
||||
|
||||
def test_authored_pull_recovery_confirms_refreshes_and_opens_above_search_history():
|
||||
script = f"""
|
||||
require({json.dumps(str(SEARCH_PREVIEW))});
|
||||
(async () => {{
|
||||
const events = [];
|
||||
const history = ['search'];
|
||||
const detail = {{repository:'stackchain/web',number:9,kind:'pull',head_sha:'abc1234'}};
|
||||
const item = {{repository:'stackchain/web',number:9,kind:'pull',key:'stackchain/web#9'}};
|
||||
const recover = globalThis.createSearchAuthoredPullRecovery({{
|
||||
confirm:message => {{ events.push(['confirm',message]); return true; }},
|
||||
reopen:target => {{ events.push(['reopen',target.head_sha]); return Promise.resolve({{...target,state:'open'}}); }},
|
||||
refresh:() => {{ events.push(['refresh']); return Promise.resolve(); }},
|
||||
find:target => target.number === 9 ? item : null,
|
||||
open:opened => {{ events.push(['open',opened.key]); history.push('pull'); }},
|
||||
unavailable:message => events.push(['unavailable',message]),
|
||||
}});
|
||||
const outcome = await recover(detail);
|
||||
process.stdout.write(JSON.stringify({{events,history,outcome}}));
|
||||
}})().catch(error => {{ console.error(error); process.exit(1); }});
|
||||
"""
|
||||
result = subprocess.run(["node", "-e", script], capture_output=True, text=True)
|
||||
|
||||
assert result.returncode == 0, result.stderr
|
||||
payload = json.loads(result.stdout)
|
||||
assert payload["events"] == [
|
||||
["confirm", "Reopen stackchain/web #9?"],
|
||||
["reopen", "abc1234"],
|
||||
["refresh"],
|
||||
["open", "stackchain/web#9"],
|
||||
]
|
||||
assert payload["history"] == ["search", "pull"]
|
||||
assert payload["outcome"] == "opened"
|
||||
|
||||
|
||||
def test_search_preview_watch_is_single_flight_and_keeps_visible_context_on_failure():
|
||||
script = f"""
|
||||
const createSearchPreview = require({json.dumps(str(SEARCH_PREVIEW))});
|
||||
@@ -1159,9 +1296,11 @@ def test_assigned_pull_preview_hands_off_to_existing_my_work_sheet_without_claim
|
||||
def test_requested_pull_preview_opens_native_review_without_assignment_mutation():
|
||||
html = dashboard_bundle_text()
|
||||
css = (FRONTEND / "dashboard.css").read_text()
|
||||
preview = SEARCH_PREVIEW.read_text()
|
||||
|
||||
assert "startButton.hidden = !(detail.reviewable" in html
|
||||
assert "detail.reviewable ? 'Review now'" in html
|
||||
assert "renderSearchPreviewStart(detail, state, startButton)" in html
|
||||
assert "button.hidden = !(detail.reviewable" in preview
|
||||
assert "detail.reviewable ? 'Review now'" in preview
|
||||
handler = html.split(
|
||||
"qs('#start-search-result').addEventListener('click'", 1
|
||||
)[1].split("qs('#close-whiteboard')", 1)[0]
|
||||
@@ -1176,9 +1315,10 @@ def test_requested_pull_preview_opens_native_review_without_assignment_mutation(
|
||||
def test_search_preview_offers_assign_and_start_for_eligible_issues():
|
||||
html = dashboard_bundle_text()
|
||||
css = (FRONTEND / "dashboard.css").read_text()
|
||||
preview = SEARCH_PREVIEW.read_text()
|
||||
|
||||
assert 'id="start-search-result"' in html
|
||||
assert "(detail.assigned_to_me ? 'Start in Today' : 'Assign & start')" in html
|
||||
assert "(detail.assigned_to_me ? 'Start in Today' : 'Assign & start')" in preview
|
||||
assert "const searchAssignAndStart = createSearchStart(detail => searchPreview.claim(detail))" in html
|
||||
assert "searchAssignAndStart.run(detail, { alreadyOwned: detail.assigned_to_me })" in html
|
||||
assert ".search-preview-primary-actions" in css
|
||||
@@ -1332,9 +1472,10 @@ process.stdout.write(JSON.stringify({{outcome, refreshes, messages}}));
|
||||
|
||||
def test_closed_issue_preview_reopens_then_resumes_through_capacity_guard():
|
||||
html = dashboard_bundle_text()
|
||||
preview = SEARCH_PREVIEW.read_text()
|
||||
|
||||
assert "detail.reopenable ? 'Reopen & resume'" in html
|
||||
assert "mutate:(detail,action)=>fetchReviewJson(" in html
|
||||
assert "detail.reopenable ? 'Reopen & resume'" in preview
|
||||
assert "mutate:searchPreviewMutation(fetchReviewJson)" in html
|
||||
handler = html.split("qs('#start-search-result').addEventListener('click'", 1)[1].split(
|
||||
"qs('#close-whiteboard')", 1
|
||||
)[0]
|
||||
|
||||
@@ -875,6 +875,108 @@ async def test_work_preview_marks_authoritative_requested_pull_review_actionable
|
||||
assert "/api/v1/repos/stackchain/web/pulls/9" in requested_paths
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_work_preview_offers_closed_authored_unmerged_pull_recovery_with_head_guard():
|
||||
requested_paths = []
|
||||
|
||||
async def handler(request):
|
||||
requested_paths.append(request.url.path)
|
||||
if request.url.path.endswith("/user"):
|
||||
return httpx.Response(200, json={"login": "timmy"})
|
||||
if request.url.path.endswith("/pulls/9"):
|
||||
return httpx.Response(200, json={
|
||||
"number": 9,
|
||||
"state": "closed",
|
||||
"merged": False,
|
||||
"user": {"login": "timmy"},
|
||||
"head": {"sha": "abc1234"},
|
||||
"requested_reviewers": [],
|
||||
})
|
||||
return httpx.Response(200, json={
|
||||
"number": 9,
|
||||
"title": "Recover mobile flow",
|
||||
"state": "closed",
|
||||
"html_url": "http://127.0.0.1:3000/stackchain/web/pulls/9",
|
||||
"user": {"login": "timmy"},
|
||||
"pull_request": {"merged": False},
|
||||
"assignees": [],
|
||||
})
|
||||
|
||||
gitea_proxy.start_client(transport=httpx.MockTransport(handler))
|
||||
try:
|
||||
preview = await gitea_proxy.work_preview("stackchain/web", "pull", 9)
|
||||
finally:
|
||||
await gitea_proxy.stop_client()
|
||||
|
||||
assert preview["authored_pull_reopenable"] is True
|
||||
assert preview["head_sha"] == "abc1234"
|
||||
assert "/api/v1/repos/stackchain/web/pulls/9" in requested_paths
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_work_preview_does_not_offer_closed_pull_recovery_to_non_author():
|
||||
async def handler(request):
|
||||
if request.url.path.endswith("/user"):
|
||||
return httpx.Response(200, json={"login": "timmy"})
|
||||
if request.url.path.endswith("/pulls/9"):
|
||||
return httpx.Response(200, json={
|
||||
"number": 9,
|
||||
"state": "closed",
|
||||
"merged": False,
|
||||
"user": {"login": "alexander"},
|
||||
"head": {"sha": "abc1234"},
|
||||
})
|
||||
return httpx.Response(200, json={
|
||||
"number": 9,
|
||||
"title": "Foreign pull",
|
||||
"state": "closed",
|
||||
"html_url": "http://127.0.0.1:3000/stackchain/web/pulls/9",
|
||||
"user": {"login": "alexander"},
|
||||
"pull_request": {"merged": False},
|
||||
"assignees": [],
|
||||
})
|
||||
|
||||
gitea_proxy.start_client(transport=httpx.MockTransport(handler))
|
||||
try:
|
||||
preview = await gitea_proxy.work_preview("stackchain/web", "pull", 9)
|
||||
finally:
|
||||
await gitea_proxy.stop_client()
|
||||
|
||||
assert preview["authored_pull_reopenable"] is False
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_work_preview_does_not_offer_recovery_for_merged_pull():
|
||||
async def handler(request):
|
||||
if request.url.path.endswith("/user"):
|
||||
return httpx.Response(200, json={"login": "timmy"})
|
||||
if request.url.path.endswith("/pulls/9"):
|
||||
return httpx.Response(200, json={
|
||||
"number": 9,
|
||||
"state": "closed",
|
||||
"merged": True,
|
||||
"user": {"login": "timmy"},
|
||||
"head": {"sha": "abc1234"},
|
||||
})
|
||||
return httpx.Response(200, json={
|
||||
"number": 9,
|
||||
"title": "Merged pull",
|
||||
"state": "closed",
|
||||
"html_url": "http://127.0.0.1:3000/stackchain/web/pulls/9",
|
||||
"user": {"login": "timmy"},
|
||||
"pull_request": {"merged": True},
|
||||
"assignees": [],
|
||||
})
|
||||
|
||||
gitea_proxy.start_client(transport=httpx.MockTransport(handler))
|
||||
try:
|
||||
preview = await gitea_proxy.work_preview("stackchain/web", "pull", 9)
|
||||
finally:
|
||||
await gitea_proxy.stop_client()
|
||||
|
||||
assert preview["authored_pull_reopenable"] is False
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_work_preview_offers_reopen_only_for_closed_issues():
|
||||
async def handler(request):
|
||||
|
||||
Reference in new issue
Block a user