From a4ae3e1c7ff02b8be509f5d639c436aa6b76d22c Mon Sep 17 00:00:00 2001 From: timmy Date: Mon, 24 Aug 2026 10:06:51 +0000 Subject: [PATCH] feat: publish draft pull requests from mobile (Closes #1344) --- frontend/index.html | 2 + frontend/pull-sheet.js | 49 ++++++++++- src/gitea_proxy.py | 63 ++++++++++++++ src/main.py | 35 ++++++++ .../test_mobile_publish_draft_pull_release.py | 87 +++++++++++++++++++ tests/test_pull_api.py | 80 +++++++++++++++++ 6 files changed, 314 insertions(+), 2 deletions(-) create mode 100644 tests/e2e/test_mobile_publish_draft_pull_release.py diff --git a/frontend/index.html b/frontend/index.html index 9fc798c..95bbf9c 100644 --- a/frontend/index.html +++ b/frontend/index.html @@ -1763,6 +1763,8 @@
Request review
+ +
diff --git a/frontend/pull-sheet.js b/frontend/pull-sheet.js index e7190d4..7a38b73 100644 --- a/frontend/pull-sheet.js +++ b/frontend/pull-sheet.js @@ -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'); diff --git a/src/gitea_proxy.py b/src/gitea_proxy.py index deda010..4bed2fb 100644 --- a/src/gitea_proxy.py +++ b/src/gitea_proxy.py @@ -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: diff --git a/src/main.py b/src/main.py index 3c2fa06..7803775 100644 --- a/src/main.py +++ b/src/main.py @@ -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) diff --git a/tests/e2e/test_mobile_publish_draft_pull_release.py b/tests/e2e/test_mobile_publish_draft_pull_release.py new file mode 100644 index 0000000..2835668 --- /dev/null +++ b/tests/e2e/test_mobile_publish_draft_pull_release.py @@ -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 diff --git a/tests/test_pull_api.py b/tests/test_pull_api.py index f9060aa..56b34c7 100644 --- a/tests/test_pull_api.py +++ b/tests/test_pull_api.py @@ -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 = []