Publish draft pull requests from mobile #1345

Merged
timmy merged 1 commits from timmy/1344-publish-draft-pull into main 2026-08-24 10:16:35 +00:00
6 changed files with 314 additions and 2 deletions

View File

@ -1763,6 +1763,8 @@
<details class="pull-review-request" id="pull-review-request">
<summary>Request review</summary>
<div>
<button id="publish-draft-pull" type="button" hidden>Ready for review</button>
<div id="publish-draft-pull-status" class="small" aria-live="assertive"></div>
<button id="load-pull-reviewers" type="button">Choose reviewer</button>
<label for="pull-review-recipient" class="small">Eligible repository reviewer</label>
<select id="pull-review-recipient" disabled><option value="">Select a teammate</option></select>

View File

@ -220,7 +220,7 @@ function resetReviewRequestControls(doc, detail) {
qs('#pull-feedback-status').textContent = '';
}
function bindReviewRequestControls(doc, controller, getSelected, getDetail) {
function bindReviewRequestControls(doc, controller, getSelected, getDetail, getLogin) {
const qs = selector => doc.querySelector(selector);
const load = qs('#load-pull-reviewers');
if (load.dataset.reviewRequestBound === 'true') return;
@ -228,6 +228,11 @@ function bindReviewRequestControls(doc, controller, getSelected, getDetail) {
controller.setReviewDetail = detail => {
resetReviewRequestControls(doc, detail);
controller.edit?.setDetail(detail, Boolean(detail?.saved_at));
const publishable = detail?.state === 'open' && detail?.draft === true &&
detail?.author === getLogin?.() && detail?.head_sha;
qs('#publish-draft-pull').hidden = !publishable;
if (publishable) qs('#pull-review-request').open = true;
qs('#publish-draft-pull-status').textContent = '';
};
resetReviewRequestControls(doc, null);
qs('#request-updated-pull-review').addEventListener('click', () => {
@ -244,6 +249,30 @@ function bindReviewRequestControls(doc, controller, getSelected, getDetail) {
qs('#pull-merge-state').textContent = 'Waiting for @' + result.reviewer + 's review';
qs('#merge-pull').disabled = true;
};
qs('#publish-draft-pull').addEventListener('click', async () => {
const selected = getSelected?.();
const detail = getDetail?.();
if (!selected || !detail?.head_sha || detail.draft !== true ||
!globalThis.confirm('Publish ' + selected.key + ' as ready for review?')) return;
const button = qs('#publish-draft-pull');
button.disabled = true;
qs('#publish-draft-pull-status').textContent = 'Publishing draft…';
try {
const result = await controller.publishReady(selected, detail.head_sha);
if (getSelected?.() !== selected || getDetail?.() !== detail) return;
Object.assign(detail, result);
selected.title = result.title;
qs('#pull-sheet-title').textContent = result.title;
controller.setReviewDetail(detail);
qs('#publish-draft-pull-status').textContent = 'Published and ready for review.';
qs('#pull-review-request').open = true;
load.focus();
} catch (error) {
qs('#publish-draft-pull-status').textContent = error.message + ' The pull request remains a draft; retry.';
button.disabled = false;
button.focus();
}
});
qs('#pull-reviewer-statuses').addEventListener('click', async event => {
const button = event.target.closest?.('[data-cancel-review-request]');
if (!button || button.disabled) return;
@ -534,7 +563,7 @@ function bindContextEditor(doc, controller, getSelected, getDetail, getLogin) {
}
function bindOwnershipControls(doc, controller, getSelected, finish, getDetail, getLogin) {
bindReviewRequestControls(doc, controller, getSelected, getDetail);
bindReviewRequestControls(doc, controller, getSelected, getDetail, getLogin);
bindFeedbackControls(doc, controller, getSelected, getDetail, getLogin);
controller.edit = bindContextEditor(doc, controller, getSelected, getDetail, getLogin);
const qs = selector => doc.querySelector(selector);
@ -610,6 +639,7 @@ function createPullSheet({ fetchJson, storage, createConversationPager = globalT
let reviewCandidateRequest = null;
let reviewRequestMutation = null;
let reviewCancelMutation = null;
let readyMutation = null;
let editRequest = null;
let feedbackRequest = null;
const reviewRequests = new Map();
@ -719,6 +749,21 @@ function createPullSheet({ fetchJson, storage, createConversationPager = globalT
}).finally(() => { reviewCancelMutation = null; });
return reviewCancelMutation;
},
publishReady(item, expectedHeadSha) {
if (readyMutation) return readyMutation;
readyMutation = fetchJson(pathFor(item) + '/ready', {
method:'PATCH',
headers:{ Accept:'application/json', 'Content-Type':'application/json' },
body:JSON.stringify({ expected_head_sha:expectedHeadSha }),
}).then(result => {
if (result?.number !== item.number || result?.head_sha !== expectedHeadSha ||
result?.state !== 'open' || result?.draft !== false || !result?.title) {
throw new Error('Draft publication was not confirmed.');
}
return result;
}).finally(() => { readyMutation = null; });
return readyMutation;
},
loadEditDraft(item) {
try {
const value = JSON.parse(storage?.getItem(editDraftKey(item)) || 'null');

View File

@ -2028,6 +2028,69 @@ async def request_assigned_pull_review(
}
async def publish_authored_assigned_pull(
repository: str, number: int, expected_head_sha: str
) -> dict:
login, pull = await _current_login_and_target(
f"repos/{repository}/pulls/{number}"
)
author = pull.get("user") if isinstance(pull.get("user"), dict) else {}
head = pull.get("head") if isinstance(pull.get("head"), dict) else {}
title = pull.get("title") if isinstance(pull.get("title"), str) else ""
ready_title = re.sub(
r"^(?:WIP\s*:|\[WIP\]|Draft\s*:|\[Draft\])\s*",
"",
title,
count=1,
flags=re.IGNORECASE,
).strip()
if (
pull.get("state") != "open"
or pull.get("merged") is True
or pull.get("draft") is not True
or author.get("login", "").casefold() != login.casefold()
or not _login_in_users(login, pull.get("assignees"))
or head.get("sha") != expected_head_sha
or not ready_title
or ready_title == title
):
raise IssueNotAvailableError("Draft pull request is no longer publishable")
response = await _get_client().patch(
f"/api/v1/repos/{repository}/pulls/{number}",
headers=_auth(),
json={"title": ready_title},
)
response.raise_for_status()
confirmed_response = await _get_client().get(
f"/api/v1/repos/{repository}/pulls/{number}", headers=_auth()
)
confirmed_response.raise_for_status()
confirmed = confirmed_response.json()
confirmed_head = (
confirmed.get("head")
if isinstance(confirmed, dict) and isinstance(confirmed.get("head"), dict)
else {}
)
if (
not isinstance(confirmed, dict)
or confirmed.get("number") != number
or confirmed.get("state") != "open"
or confirmed.get("draft") is not False
or confirmed.get("title") != ready_title
or confirmed_head.get("sha") != expected_head_sha
):
raise ValueError("Gitea did not confirm the pull request is ready")
return {
"repository": repository,
"number": number,
"title": ready_title,
"head_sha": expected_head_sha,
"state": "open",
"draft": False,
}
async def cancel_assigned_pull_review(
repository: str, number: int, reviewer: str, expected_head_sha: str
) -> dict:

View File

@ -1117,6 +1117,12 @@ class PullReviewRequest(BaseModel):
)
class PullReadyRequest(BaseModel):
expected_head_sha: str = Field(
min_length=7, max_length=64, pattern=r"^[A-Fa-f0-9]+$"
)
class PullContentUpdate(BaseModel):
title: str = Field(min_length=1, max_length=255)
body: str = Field(default="", max_length=10_000)
@ -6609,6 +6615,35 @@ async def update_authored_assigned_pull_content(
return JSONResponse(result)
@app.patch("/api/v1/repos/{owner}/{repo}/pulls/{number}/ready")
async def publish_authored_assigned_pull(
update: PullReadyRequest,
owner: str,
repo: str,
number: int = PathParam(gt=0),
):
repository = f"{owner}/{repo}"
try:
result = await asyncio.wait_for(
gitea_proxy.publish_authored_assigned_pull(
repository, number, update.expected_head_sha
),
timeout=ISSUE_ACTION_TIMEOUT_SECONDS,
)
except gitea_proxy.IssueNotAvailableError:
return JSONResponse(
{"error": "The draft pull request changed. Reload before publishing."},
status_code=409,
)
except Exception:
return JSONResponse(
{"error": "The draft pull request could not be published. Please retry."},
status_code=503,
headers={"Retry-After": "1"},
)
return JSONResponse(result)
@app.get("/api/v1/repos/{owner}/{repo}/pulls/{number}/review-data")
async def assigned_pull_review_data(
owner: str, repo: str, number: int = PathParam(gt=0)

View File

@ -0,0 +1,87 @@
from pathlib import Path
import pytest
ROOT = Path(__file__).parents[2]
@pytest.mark.parametrize("viewport", [(320, 568), (390, 844)])
def test_mobile_author_publishes_draft_and_continues_to_reviewer_picker(viewport):
playwright = pytest.importorskip("playwright.sync_api")
html = (ROOT / "frontend" / "index.html").read_text()
detail = {
"number": 7,
"title": "WIP: Ship mobile flow",
"author": "timmy",
"state": "open",
"draft": True,
"merged": False,
"head_sha": "abc1234",
}
with playwright.sync_playwright() as runtime:
try:
browser = runtime.chromium.launch(headless=True)
except Exception as error:
pytest.skip(f"Chromium is not installed: {error}")
page = browser.new_page(viewport={"width": viewport[0], "height": viewport[1]})
page.on("dialog", lambda dialog: dialog.accept())
page.set_content(html, wait_until="domcontentloaded")
page.add_style_tag(path=ROOT / "frontend" / "dashboard.css")
page.add_script_tag(path=ROOT / "frontend" / "pull-sheet.js")
page.evaluate(
"""detail => {
window.publishDetail = detail;
window.publishItem = {repository:'stackchain/api', number:7, key:'stackchain/api#7'};
const controller = createPullSheet({
storage:null,
fetchJson:(path, options) => {
if (!path.endsWith('/ready') || options.method !== 'PATCH') {
return Promise.reject(new Error('unexpected request'));
}
const payload = JSON.parse(options.body);
if (payload.expected_head_sha !== detail.head_sha) {
return Promise.reject(new Error('wrong head'));
}
return Promise.resolve({
repository:'stackchain/api', number:7, title:'Ship mobile flow',
head_sha:detail.head_sha, state:'open', draft:false,
});
},
});
createPullSheet.bindOwnershipControls(
document, controller, () => window.publishItem, async () => false,
() => window.publishDetail, () => 'timmy'
);
document.querySelector('#pull-sheet').classList.add('open');
controller.setReviewDetail(detail);
}""",
detail,
)
ready = page.get_by_role("button", name="Ready for review")
before = ready.evaluate("button => ({height:button.getBoundingClientRect().height, hidden:button.hidden})")
ready.click()
page.get_by_text("Published and ready for review.").wait_for()
metrics = page.evaluate(
"""() => ({
scrollWidth:document.documentElement.scrollWidth,
clientWidth:document.documentElement.clientWidth,
readyHidden:document.querySelector('#publish-draft-pull').hidden,
requestOpen:document.querySelector('#pull-review-request').open,
reviewerFocused:document.activeElement === document.querySelector('#load-pull-reviewers'),
title:window.publishDetail.title,
draft:window.publishDetail.draft,
})"""
)
browser.close()
assert before["hidden"] is False
assert before["height"] >= 44
assert metrics["scrollWidth"] <= metrics["clientWidth"]
assert metrics["readyHidden"] is True
assert metrics["requestOpen"] is True
assert metrics["reviewerFocused"] is True
assert metrics["title"] == "Ship mobile flow"
assert metrics["draft"] is False

View File

@ -8,6 +8,86 @@ from src import gitea_proxy, main
from src.security_event_store import SecurityEventStoreError
@pytest.mark.anyio
async def test_pull_author_can_publish_assigned_draft_at_expected_head(monkeypatch):
calls = []
async def publish(repository, number, expected_head_sha):
calls.append((repository, number, expected_head_sha))
return {
"repository": repository,
"number": number,
"title": "Ship mobile flow",
"head_sha": expected_head_sha,
"state": "open",
"draft": False,
}
monkeypatch.setattr(
main.gitea_proxy, "publish_authored_assigned_pull", publish, raising=False
)
transport = httpx.ASGITransport(app=main.app)
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
response = await client.patch(
"/api/v1/repos/stackchain/api/pulls/7/ready",
json={"expected_head_sha": "abc1234"},
)
assert response.status_code == 200
assert response.json()["draft"] is False
assert calls == [("stackchain/api", 7, "abc1234")]
@pytest.mark.anyio
async def test_gitea_publishes_only_authored_assigned_draft_and_verifies_ready_head():
requests = []
pull = {
"number": 7,
"title": "WIP: Ship mobile flow",
"state": "open",
"draft": True,
"merged": False,
"user": {"login": "alex"},
"assignees": [{"login": "alex"}],
"head": {"sha": "abc1234"},
}
async def handler(request):
requests.append((request.method, request.url.path, request.content))
if request.url.path == "/api/v1/user":
return httpx.Response(200, json={"login": "alex"})
if request.url.path == "/api/v1/repos/stackchain/api/pulls/7" and request.method == "GET":
return httpx.Response(200, json=pull)
if request.url.path == "/api/v1/repos/stackchain/api/pulls/7" and request.method == "PATCH":
assert json.loads(request.content) == {"title": "Ship mobile flow"}
pull.update({"title": "Ship mobile flow", "draft": False})
return httpx.Response(201, json=pull)
raise AssertionError(f"unexpected request: {request.method} {request.url.path}")
gitea_proxy.start_client(transport=httpx.MockTransport(handler))
try:
result = await gitea_proxy.publish_authored_assigned_pull(
"stackchain/api", 7, "abc1234"
)
finally:
await gitea_proxy.stop_client()
assert result == {
"repository": "stackchain/api",
"number": 7,
"title": "Ship mobile flow",
"head_sha": "abc1234",
"state": "open",
"draft": False,
}
assert [item[:2] for item in requests] == [
("GET", "/api/v1/user"),
("GET", "/api/v1/repos/stackchain/api/pulls/7"),
("PATCH", "/api/v1/repos/stackchain/api/pulls/7"),
("GET", "/api/v1/repos/stackchain/api/pulls/7"),
]
@pytest.mark.anyio
async def test_pull_author_can_update_assigned_open_pull_context(monkeypatch):
calls = []