Merge pull request 'Publish draft pull requests from mobile' (#1345) from timmy/1344-publish-draft-pull into main

frontend/index.html

@@ -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>

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');

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:

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)

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

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 = []