feat: batch mobile review suggestions (Closes #1384)
This commit is contained in:
parent
c8ffde7d96
commit
869543e992
|
|
@ -18,7 +18,13 @@ token that can read dashboard data, update the authenticated user's notification
|
||||||
threads, create and self-assign issues, discover, claim, and release issue assignments,
|
threads, create and self-assign issues, discover, claim, and release issue assignments,
|
||||||
list repository labels and open milestones, set or clear due dates on assigned issues, create issue comments, close assigned issues,
|
list repository labels and open milestones, set or clear due dates on assigned issues, create issue comments, close assigned issues,
|
||||||
inspect/comment on assigned pull
|
inspect/comment on assigned pull
|
||||||
requests, merge assigned pull requests, and submit pull-request reviews.
|
requests, merge assigned pull requests, and submit pull-request reviews. For authored
|
||||||
|
pulls, several non-overlapping one-line reviewer suggestions in the same bounded UTF-8
|
||||||
|
file can be staged during the mobile feedback pass, reviewed together, and committed as
|
||||||
|
one race-guarded branch update. The batch preserves the original blob identity, applies
|
||||||
|
replacements from the bottom up, rejects overlapping or stale lines, and verifies the
|
||||||
|
advanced pull head and final file before reporting success; suggestions in another file
|
||||||
|
wait for the next batch so Gitea's single-file contents API cannot produce partial commits.
|
||||||
After an exact merged commit fails release checks, its mobile release receipt can load the
|
After an exact merged commit fails release checks, its mobile release receipt can load the
|
||||||
failed job evidence and prepare a draft rollback pull request. The server revalidates the
|
failed job evidence and prepare a draft rollback pull request. The server revalidates the
|
||||||
operator's participation and push access, reverses only bounded UTF-8 files whose current
|
operator's participation and push access, reverses only bounded UTF-8 files whose current
|
||||||
|
|
|
||||||
|
|
@ -774,6 +774,12 @@ textarea { resize: vertical; min-height: 120px; }
|
||||||
.pull-feedback-suggestion-change > div { min-width:0; }
|
.pull-feedback-suggestion-change > div { min-width:0; }
|
||||||
.pull-feedback-suggestion-change pre { box-sizing:border-box; max-width:100%; max-height:160px; white-space:pre-wrap; overflow-wrap:anywhere; word-break:break-word; }
|
.pull-feedback-suggestion-change pre { box-sizing:border-box; max-width:100%; max-height:160px; white-space:pre-wrap; overflow-wrap:anywhere; word-break:break-word; }
|
||||||
.pull-feedback-suggestion-preview input { box-sizing:border-box; width:100%; min-height:44px; }
|
.pull-feedback-suggestion-preview input { box-sizing:border-box; width:100%; min-height:44px; }
|
||||||
|
.pull-feedback-batch-review { min-width:0; display:grid; gap:10px; margin:10px 0; padding:10px; border:1px solid #3b82f6; border-radius:10px; background:#07101e; }
|
||||||
|
.pull-feedback-batch-review[hidden] { display:none; }
|
||||||
|
.pull-feedback-batch-review h5 { margin:0; }
|
||||||
|
.pull-feedback-batch-review ul { min-width:0; margin:0; padding-left:20px; overflow-wrap:anywhere; }
|
||||||
|
.pull-feedback-batch-review input { box-sizing:border-box; width:100%; min-width:0; min-height:44px; }
|
||||||
|
#review-pull-feedback-batch { position:sticky; bottom:calc(8px + env(safe-area-inset-bottom)); z-index:4; width:100%; min-height:44px; }
|
||||||
.review-handoff { position:sticky; bottom:0; z-index:3; display:grid; gap:8px; padding:10px 4px; padding-bottom:calc(10px + env(safe-area-inset-bottom)); background:rgba(11,21,38,.98); border-top:1px solid #2a496e; }
|
.review-handoff { position:sticky; bottom:0; z-index:3; display:grid; gap:8px; padding:10px 4px; padding-bottom:calc(10px + env(safe-area-inset-bottom)); background:rgba(11,21,38,.98); border-top:1px solid #2a496e; }
|
||||||
.review-handoff button { min-height:44px; }
|
.review-handoff button { min-height:44px; }
|
||||||
.review-handoff-link { min-height:44px; display:flex; align-items:center; justify-content:center; border:1px solid #60a5fa; border-radius:8px; color:#bfdbfe; font-weight:700; text-decoration:none; }
|
.review-handoff-link { min-height:44px; display:flex; align-items:center; justify-content:center; border:1px solid #60a5fa; border-radius:8px; color:#bfdbfe; font-weight:700; text-decoration:none; }
|
||||||
|
|
|
||||||
|
|
@ -1755,9 +1755,21 @@
|
||||||
<input id="pull-feedback-suggestion-message" maxlength="120" value="fix: apply review suggestion">
|
<input id="pull-feedback-suggestion-message" maxlength="120" value="fix: apply review suggestion">
|
||||||
<div class="pull-feedback-file-actions">
|
<div class="pull-feedback-file-actions">
|
||||||
<button id="cancel-pull-feedback-suggestion" type="button" hidden>Cancel suggestion</button>
|
<button id="cancel-pull-feedback-suggestion" type="button" hidden>Cancel suggestion</button>
|
||||||
|
<button id="stage-pull-feedback-suggestion" type="button" hidden>Add to fix batch</button>
|
||||||
<button id="apply-pull-feedback-suggestion" type="button" hidden>Commit suggested change</button>
|
<button id="apply-pull-feedback-suggestion" type="button" hidden>Commit suggested change</button>
|
||||||
</div>
|
</div>
|
||||||
</section>
|
</section>
|
||||||
|
<section class="pull-feedback-batch-review" id="pull-feedback-batch-review" aria-labelledby="pull-feedback-batch-heading" hidden>
|
||||||
|
<h5 id="pull-feedback-batch-heading">Review fixes</h5>
|
||||||
|
<ul id="pull-feedback-batch-items"></ul>
|
||||||
|
<label for="pull-feedback-batch-message">Commit message</label>
|
||||||
|
<input id="pull-feedback-batch-message" maxlength="120" value="fix: apply review suggestions">
|
||||||
|
<div class="pull-feedback-file-actions">
|
||||||
|
<button id="cancel-pull-feedback-batch" type="button" hidden>Keep reviewing</button>
|
||||||
|
<button id="commit-pull-feedback-batch" type="button" hidden>Commit all fixes</button>
|
||||||
|
</div>
|
||||||
|
</section>
|
||||||
|
<button id="review-pull-feedback-batch" type="button" hidden>Review fixes (0)</button>
|
||||||
<section class="pull-feedback-file-editor" id="pull-feedback-file-editor" hidden>
|
<section class="pull-feedback-file-editor" id="pull-feedback-file-editor" hidden>
|
||||||
<h5>Edit <span id="pull-feedback-file-path"></span></h5>
|
<h5>Edit <span id="pull-feedback-file-path"></span></h5>
|
||||||
<label for="pull-feedback-file-content">File content</label>
|
<label for="pull-feedback-file-content">File content</label>
|
||||||
|
|
|
||||||
|
|
@ -416,6 +416,7 @@ function bindFeedbackControls(doc, controller, getSelected, getDetail, getLogin)
|
||||||
const clearSuggestion = () => {
|
const clearSuggestion = () => {
|
||||||
qs('#pull-feedback-suggestion-preview').hidden = true;
|
qs('#pull-feedback-suggestion-preview').hidden = true;
|
||||||
qs('#cancel-pull-feedback-suggestion').hidden = true;
|
qs('#cancel-pull-feedback-suggestion').hidden = true;
|
||||||
|
qs('#stage-pull-feedback-suggestion').hidden = true;
|
||||||
qs('#apply-pull-feedback-suggestion').hidden = true;
|
qs('#apply-pull-feedback-suggestion').hidden = true;
|
||||||
if (active) active.suggestion = null;
|
if (active) active.suggestion = null;
|
||||||
};
|
};
|
||||||
|
|
@ -430,6 +431,25 @@ function bindFeedbackControls(doc, controller, getSelected, getDetail, getLogin)
|
||||||
lines.splice(line - 1, 1, ...suggestion.content.split('\n'));
|
lines.splice(line - 1, 1, ...suggestion.content.split('\n'));
|
||||||
return { before, after:suggestion.content, content:lines.join('\n') + (trailingNewline ? '\n' : '') };
|
return { before, after:suggestion.content, content:lines.join('\n') + (trailingNewline ? '\n' : '') };
|
||||||
};
|
};
|
||||||
|
const renderBatchButton = () => {
|
||||||
|
const count = active?.batch ? Object.values(active.batch)
|
||||||
|
.reduce((total, file) => total + file.suggestions.length, 0) : 0;
|
||||||
|
const button = qs('#review-pull-feedback-batch');
|
||||||
|
button.hidden = count < 2;
|
||||||
|
button.textContent = 'Review fixes (' + count + ')';
|
||||||
|
};
|
||||||
|
const rebuildBatchFile = entry => {
|
||||||
|
const trailingNewline = entry.file.content.endsWith('\n');
|
||||||
|
const lines = entry.file.content.split('\n');
|
||||||
|
if (trailingNewline) lines.pop();
|
||||||
|
const ordered = [...entry.suggestions].sort((left, right) => right.line - left.line);
|
||||||
|
if (new Set(ordered.map(item => item.line)).size !== ordered.length) return null;
|
||||||
|
for (const suggestion of ordered) {
|
||||||
|
if (suggestion.line < 1 || suggestion.line > lines.length) return null;
|
||||||
|
lines.splice(suggestion.line - 1, 1, ...suggestion.content.split('\n'));
|
||||||
|
}
|
||||||
|
return lines.join('\n') + (trailingNewline ? '\n' : '');
|
||||||
|
};
|
||||||
const render = () => {
|
const render = () => {
|
||||||
if (!active) return;
|
if (!active) return;
|
||||||
const items = comments();
|
const items = comments();
|
||||||
|
|
@ -500,15 +520,17 @@ function bindFeedbackControls(doc, controller, getSelected, getDetail, getLogin)
|
||||||
const reviewer = (detail?.reviewers || []).find(candidate => candidate.review_id === reviewId);
|
const reviewer = (detail?.reviewers || []).find(candidate => candidate.review_id === reviewId);
|
||||||
if (!detail || !item || !reviewer) return;
|
if (!detail || !item || !reviewer) return;
|
||||||
const login = getLogin?.() || '';
|
const login = getLogin?.() || '';
|
||||||
active = { item, detail, reviewer, login,
|
active = { item, detail, reviewer, login, batch:{},
|
||||||
state:controller.loadFeedbackPass(item, detail, reviewer, login) };
|
state:controller.loadFeedbackPass(item, detail, reviewer, login) };
|
||||||
panel.hidden = false;
|
panel.hidden = false;
|
||||||
qs('#pull-feedback-file-editor').hidden = true;
|
qs('#pull-feedback-file-editor').hidden = true;
|
||||||
qs('#pull-feedback-suggestion-preview').hidden = true;
|
qs('#pull-feedback-suggestion-preview').hidden = true;
|
||||||
|
qs('#pull-feedback-batch-review').hidden = true;
|
||||||
qs('#cancel-pull-feedback-fix').hidden = true;
|
qs('#cancel-pull-feedback-fix').hidden = true;
|
||||||
qs('#commit-pull-feedback-fix').hidden = true;
|
qs('#commit-pull-feedback-fix').hidden = true;
|
||||||
active.file = null;
|
active.file = null;
|
||||||
active.suggestion = null;
|
active.suggestion = null;
|
||||||
|
renderBatchButton();
|
||||||
qs('#pull-feedback-status').textContent = active.state.posted ?
|
qs('#pull-feedback-status').textContent = active.state.posted ?
|
||||||
'Response already posted. You can request an updated review.' : 'Progress saves on this device.';
|
'Response already posted. You can request an updated review.' : 'Progress saves on this device.';
|
||||||
render();
|
render();
|
||||||
|
|
@ -534,6 +556,7 @@ function bindFeedbackControls(doc, controller, getSelected, getDetail, getLogin)
|
||||||
qs('#pull-feedback-file-editor').hidden = true;
|
qs('#pull-feedback-file-editor').hidden = true;
|
||||||
qs('#pull-feedback-suggestion-preview').hidden = false;
|
qs('#pull-feedback-suggestion-preview').hidden = false;
|
||||||
qs('#cancel-pull-feedback-suggestion').hidden = false;
|
qs('#cancel-pull-feedback-suggestion').hidden = false;
|
||||||
|
qs('#stage-pull-feedback-suggestion').hidden = false;
|
||||||
qs('#apply-pull-feedback-suggestion').hidden = false;
|
qs('#apply-pull-feedback-suggestion').hidden = false;
|
||||||
qs('#pull-feedback-status').textContent = 'Review the bounded replacement before committing.';
|
qs('#pull-feedback-status').textContent = 'Review the bounded replacement before committing.';
|
||||||
qs('#apply-pull-feedback-suggestion').focus();
|
qs('#apply-pull-feedback-suggestion').focus();
|
||||||
|
|
@ -548,6 +571,90 @@ function bindFeedbackControls(doc, controller, getSelected, getDetail, getLogin)
|
||||||
clearSuggestion();
|
clearSuggestion();
|
||||||
qs('#review-pull-feedback-suggestion').focus();
|
qs('#review-pull-feedback-suggestion').focus();
|
||||||
});
|
});
|
||||||
|
qs('#stage-pull-feedback-suggestion').addEventListener('click', () => {
|
||||||
|
if (!active?.suggestion) return;
|
||||||
|
const pending = active.suggestion;
|
||||||
|
const path = pending.file.path;
|
||||||
|
const stagedPath = Object.keys(active.batch)[0];
|
||||||
|
if (stagedPath && stagedPath !== path) {
|
||||||
|
qs('#pull-feedback-status').textContent = 'Commit the staged file before batching suggestions from another file.';
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
const entry = active.batch[path] || {file:pending.file, suggestions:[]};
|
||||||
|
if (entry.file.blob_sha !== pending.file.blob_sha ||
|
||||||
|
entry.suggestions.some(item => item.commentId === pending.commentId)) return;
|
||||||
|
const comment = current();
|
||||||
|
entry.suggestions.push({
|
||||||
|
line:Number(comment.suggestion.line), content:comment.suggestion.content,
|
||||||
|
commentId:pending.commentId,
|
||||||
|
});
|
||||||
|
entry.content = rebuildBatchFile(entry);
|
||||||
|
if (entry.content === null) {
|
||||||
|
entry.suggestions.pop();
|
||||||
|
qs('#pull-feedback-status').textContent = 'This suggestion overlaps a staged fix. Keep one of the conflicting changes.';
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
active.batch[path] = entry;
|
||||||
|
clearSuggestion();
|
||||||
|
renderBatchButton();
|
||||||
|
qs('#pull-feedback-status').textContent = 'Suggestion added to the fix batch. Keep reviewing this feedback.';
|
||||||
|
qs('#next-pull-feedback').focus();
|
||||||
|
});
|
||||||
|
qs('#review-pull-feedback-batch').addEventListener('click', () => {
|
||||||
|
if (!active?.batch) return;
|
||||||
|
const list = qs('#pull-feedback-batch-items');
|
||||||
|
list.replaceChildren(...Object.values(active.batch).map(entry => {
|
||||||
|
const item = doc.createElement('li');
|
||||||
|
item.textContent = entry.file.path + ' · ' + entry.suggestions.length +
|
||||||
|
(entry.suggestions.length === 1 ? ' suggestion' : ' suggestions');
|
||||||
|
return item;
|
||||||
|
}));
|
||||||
|
qs('#pull-feedback-batch-review').hidden = false;
|
||||||
|
qs('#cancel-pull-feedback-batch').hidden = false;
|
||||||
|
qs('#commit-pull-feedback-batch').hidden = false;
|
||||||
|
qs('#pull-feedback-batch-review').scrollIntoView({block:'center', behavior:'smooth'});
|
||||||
|
qs('#commit-pull-feedback-batch').focus();
|
||||||
|
});
|
||||||
|
qs('#cancel-pull-feedback-batch').addEventListener('click', () => {
|
||||||
|
qs('#pull-feedback-batch-review').hidden = true;
|
||||||
|
qs('#cancel-pull-feedback-batch').hidden = true;
|
||||||
|
qs('#commit-pull-feedback-batch').hidden = true;
|
||||||
|
qs('#review-pull-feedback-batch').focus();
|
||||||
|
});
|
||||||
|
qs('#commit-pull-feedback-batch').addEventListener('click', async () => {
|
||||||
|
if (!active?.batch) return;
|
||||||
|
const entries = Object.values(active.batch);
|
||||||
|
const count = entries.reduce((total, entry) => total + entry.suggestions.length, 0);
|
||||||
|
const message = qs('#pull-feedback-batch-message').value.trim();
|
||||||
|
if (count < 2 || !message) return;
|
||||||
|
if (!globalThis.confirm('Commit ' + count + ' suggested changes to ' + active.item.key + '?')) return;
|
||||||
|
const button = qs('#commit-pull-feedback-batch');
|
||||||
|
button.disabled = true;
|
||||||
|
qs('#pull-feedback-status').textContent = 'Committing and verifying all suggested changes…';
|
||||||
|
try {
|
||||||
|
const result = await controller.commitFeedbackBatch(active.item, {
|
||||||
|
file:{
|
||||||
|
path:entries[0].file.path, content:entries[0].content,
|
||||||
|
expectedBlobSha:entries[0].file.blob_sha,
|
||||||
|
},
|
||||||
|
suggestionCount:count, message, expectedHeadSha:active.detail.head_sha,
|
||||||
|
});
|
||||||
|
active.detail.head_sha = result.head_sha;
|
||||||
|
active.state.headSha = result.head_sha;
|
||||||
|
active.batch = {};
|
||||||
|
save();
|
||||||
|
qs('#pull-feedback-batch-review').hidden = true;
|
||||||
|
qs('#cancel-pull-feedback-batch').hidden = true;
|
||||||
|
qs('#commit-pull-feedback-batch').hidden = true;
|
||||||
|
renderBatchButton();
|
||||||
|
qs('#pull-feedback-status').textContent = count + ' suggested changes committed. Mark each comment Addressed when satisfied.';
|
||||||
|
render();
|
||||||
|
} catch (error) {
|
||||||
|
qs('#pull-feedback-status').textContent = error.message + ' Batch kept; reload if the branch changed, or retry.';
|
||||||
|
button.disabled = false;
|
||||||
|
button.focus();
|
||||||
|
}
|
||||||
|
});
|
||||||
qs('#apply-pull-feedback-suggestion').addEventListener('click', async () => {
|
qs('#apply-pull-feedback-suggestion').addEventListener('click', async () => {
|
||||||
if (!active?.suggestion) return;
|
if (!active?.suggestion) return;
|
||||||
const message = qs('#pull-feedback-suggestion-message').value.trim();
|
const message = qs('#pull-feedback-suggestion-message').value.trim();
|
||||||
|
|
@ -951,6 +1058,28 @@ function createPullSheet({ fetchJson, storage, onState, createConversationPager
|
||||||
}).finally(() => { feedbackFixRequest = null; });
|
}).finally(() => { feedbackFixRequest = null; });
|
||||||
return feedbackFixRequest;
|
return feedbackFixRequest;
|
||||||
},
|
},
|
||||||
|
commitFeedbackBatch(item, draft) {
|
||||||
|
if (feedbackFixRequest) return feedbackFixRequest;
|
||||||
|
feedbackFixRequest = fetchJson(pathFor(item) + '/feedback-batch', {
|
||||||
|
method:'POST',
|
||||||
|
headers:{ Accept:'application/json', 'Content-Type':'application/json' },
|
||||||
|
body:JSON.stringify({
|
||||||
|
file:{
|
||||||
|
path:draft.file.path, content:draft.file.content,
|
||||||
|
expected_blob_sha:draft.file.expectedBlobSha,
|
||||||
|
},
|
||||||
|
suggestion_count:draft.suggestionCount,
|
||||||
|
message:draft.message, expected_head_sha:draft.expectedHeadSha,
|
||||||
|
}),
|
||||||
|
}).then(result => {
|
||||||
|
if (!result?.head_sha || result.head_sha === draft.expectedHeadSha ||
|
||||||
|
result.path !== draft.file.path || result.suggestion_count !== draft.suggestionCount) {
|
||||||
|
throw new Error('The suggestion batch was not confirmed.');
|
||||||
|
}
|
||||||
|
return result;
|
||||||
|
}).finally(() => { feedbackFixRequest = null; });
|
||||||
|
return feedbackFixRequest;
|
||||||
|
},
|
||||||
loadChecks(item) {
|
loadChecks(item) {
|
||||||
if (checkRequest) return checkRequest;
|
if (checkRequest) return checkRequest;
|
||||||
checkRequest = fetchJson(pathFor(item) + '/checks', {
|
checkRequest = fetchJson(pathFor(item) + '/checks', {
|
||||||
|
|
|
||||||
|
|
@ -2419,6 +2419,24 @@ async def commit_authored_pull_feedback_fix(
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
async def commit_authored_pull_feedback_batch(
|
||||||
|
repository: str,
|
||||||
|
number: int,
|
||||||
|
file: dict,
|
||||||
|
suggestion_count: int,
|
||||||
|
message: str,
|
||||||
|
expected_head_sha: str,
|
||||||
|
) -> dict:
|
||||||
|
"""Commit multiple reviewed replacements in one file as one verified commit."""
|
||||||
|
if not 2 <= suggestion_count <= 8:
|
||||||
|
raise IssueNotAvailableError("Feedback batch is outside the editable bounds")
|
||||||
|
result = await commit_authored_pull_feedback_fix(
|
||||||
|
repository, number, file.get("path", ""), file.get("content", ""), message,
|
||||||
|
expected_head_sha, file.get("expected_blob_sha", ""),
|
||||||
|
)
|
||||||
|
return {**result, "suggestion_count": suggestion_count}
|
||||||
|
|
||||||
|
|
||||||
async def update_authored_pull_branch(
|
async def update_authored_pull_branch(
|
||||||
repository: str, number: int, expected_head_sha: str
|
repository: str, number: int, expected_head_sha: str
|
||||||
) -> dict:
|
) -> dict:
|
||||||
|
|
|
||||||
43
src/main.py
43
src/main.py
|
|
@ -1166,6 +1166,21 @@ class PullFeedbackFileUpdate(BaseModel):
|
||||||
expected_blob_sha: str = Field(min_length=1, max_length=128)
|
expected_blob_sha: str = Field(min_length=1, max_length=128)
|
||||||
|
|
||||||
|
|
||||||
|
class PullFeedbackBatchFile(BaseModel):
|
||||||
|
path: str = Field(min_length=1, max_length=1024)
|
||||||
|
content: str = Field(max_length=131_072)
|
||||||
|
expected_blob_sha: str = Field(min_length=1, max_length=128)
|
||||||
|
|
||||||
|
|
||||||
|
class PullFeedbackBatchUpdate(BaseModel):
|
||||||
|
file: PullFeedbackBatchFile
|
||||||
|
suggestion_count: int = Field(ge=2, le=8)
|
||||||
|
message: str = Field(min_length=1, max_length=120)
|
||||||
|
expected_head_sha: str = Field(
|
||||||
|
min_length=7, max_length=64, pattern=r"^[A-Fa-f0-9]+$"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
class PullContentUpdate(BaseModel):
|
class PullContentUpdate(BaseModel):
|
||||||
title: str = Field(min_length=1, max_length=255)
|
title: str = Field(min_length=1, max_length=255)
|
||||||
body: str = Field(default="", max_length=10_000)
|
body: str = Field(default="", max_length=10_000)
|
||||||
|
|
@ -7159,6 +7174,34 @@ async def commit_authored_pull_feedback_fix(
|
||||||
return JSONResponse(result)
|
return JSONResponse(result)
|
||||||
|
|
||||||
|
|
||||||
|
@app.post("/api/v1/repos/{owner}/{repo}/pulls/{number}/feedback-batch")
|
||||||
|
async def commit_authored_pull_feedback_batch(
|
||||||
|
update: PullFeedbackBatchUpdate,
|
||||||
|
owner: str,
|
||||||
|
repo: str,
|
||||||
|
number: int = PathParam(gt=0),
|
||||||
|
):
|
||||||
|
try:
|
||||||
|
result = await asyncio.wait_for(
|
||||||
|
gitea_proxy.commit_authored_pull_feedback_batch(
|
||||||
|
f"{owner}/{repo}", number, update.file.model_dump(),
|
||||||
|
update.suggestion_count, update.message, update.expected_head_sha,
|
||||||
|
),
|
||||||
|
timeout=ISSUE_ACTION_TIMEOUT_SECONDS,
|
||||||
|
)
|
||||||
|
except gitea_proxy.IssueNotAvailableError:
|
||||||
|
return JSONResponse(
|
||||||
|
{"error": "The pull request or a suggested file changed. No fixes were committed."},
|
||||||
|
status_code=409,
|
||||||
|
)
|
||||||
|
except Exception:
|
||||||
|
return JSONResponse(
|
||||||
|
{"error": "The suggestion batch could not be confirmed. Keep the batch and retry."},
|
||||||
|
status_code=503, headers={"Retry-After": "1"},
|
||||||
|
)
|
||||||
|
return JSONResponse(result)
|
||||||
|
|
||||||
|
|
||||||
@app.get("/api/v1/repos/{owner}/{repo}/pulls/{number}/review-data")
|
@app.get("/api/v1/repos/{owner}/{repo}/pulls/{number}/review-data")
|
||||||
async def assigned_pull_review_data(
|
async def assigned_pull_review_data(
|
||||||
owner: str, repo: str, number: int = PathParam(gt=0)
|
owner: str, repo: str, number: int = PathParam(gt=0)
|
||||||
|
|
|
||||||
108
tests/e2e/test_mobile_apply_review_suggestion_batch_release.py
Normal file
108
tests/e2e/test_mobile_apply_review_suggestion_batch_release.py
Normal file
|
|
@ -0,0 +1,108 @@
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
|
||||||
|
ROOT = Path(__file__).parents[2]
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("viewport", [(320, 568), (390, 844)])
|
||||||
|
def test_mobile_author_stages_and_commits_two_same_file_suggestions_atomically(viewport):
|
||||||
|
playwright = pytest.importorskip("playwright.sync_api")
|
||||||
|
html = (ROOT / "frontend" / "index.html").read_text()
|
||||||
|
detail = {
|
||||||
|
"state": "open", "merged": False, "head_sha": "abc1234",
|
||||||
|
"capabilities": {"authored": True},
|
||||||
|
"reviewers": [{
|
||||||
|
"review_id": 42, "login": "sam", "status": "changes_requested",
|
||||||
|
"head_sha": "abc1234", "blocking": True, "feedback_state": "loaded",
|
||||||
|
"comments": [
|
||||||
|
{"id": 99, "path": "src/api.py", "line": 2, "body": "Use the typed empty result.",
|
||||||
|
"suggestion": {"line": 2, "content": " return empty_result()"}},
|
||||||
|
{"id": 100, "path": "src/api.py", "line": 3, "body": "Keep the result typed.",
|
||||||
|
"suggestion": {"line": 3, "content": " return typed_result()"}},
|
||||||
|
],
|
||||||
|
}],
|
||||||
|
"files": [
|
||||||
|
{"filename": "src/api.py", "status": "modified", "diff_available": True,
|
||||||
|
"diff_lines": ["@@ -2 +2 @@", "+ return None"]},
|
||||||
|
|
||||||
|
],
|
||||||
|
}
|
||||||
|
|
||||||
|
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.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 => {
|
||||||
|
globalThis.confirm = () => true;
|
||||||
|
globalThis.batchCalls = [];
|
||||||
|
globalThis.batchItem = {repository:'stackchain/api', number:7, key:'stackchain/api#7'};
|
||||||
|
globalThis.batchDetail = detail;
|
||||||
|
globalThis.batchController = createPullSheet({fetchJson: async (path, options = {}) => {
|
||||||
|
batchCalls.push({path, options});
|
||||||
|
if ((options.method || 'GET') === 'GET') {
|
||||||
|
return {path:'src/api.py', head_sha:'abc1234', blob_sha:'blob1',
|
||||||
|
content:'def lookup():\\n return None\\n return result\\n'};
|
||||||
|
}
|
||||||
|
return {path:'src/api.py', suggestion_count:2, previous_head_sha:'abc1234', head_sha:'def5678'};
|
||||||
|
}});
|
||||||
|
document.querySelector('#pull-sheet').classList.add('open');
|
||||||
|
document.querySelector('#pull-review').open = true;
|
||||||
|
document.querySelector('#pull-files').innerHTML = detail.files.map((file, index) =>
|
||||||
|
createPullSheet.renderFile(file, index, false, value => String(value))
|
||||||
|
).join('');
|
||||||
|
createPullSheet.bindFeedbackControls(document, batchController,
|
||||||
|
() => batchItem, () => batchDetail, () => 'alex');
|
||||||
|
createPullSheet.review(detail, null, document);
|
||||||
|
}""",
|
||||||
|
detail,
|
||||||
|
)
|
||||||
|
|
||||||
|
page.locator(".pull-review-feedback summary").click()
|
||||||
|
page.locator('[data-address-review-feedback="42"]').click()
|
||||||
|
page.locator("#review-pull-feedback-suggestion").click()
|
||||||
|
page.wait_for_function("batchCalls.length === 1")
|
||||||
|
page.locator("#stage-pull-feedback-suggestion").click()
|
||||||
|
assert page.locator("#review-pull-feedback-batch").text_content() == "Review fixes (1)"
|
||||||
|
page.locator("#next-pull-feedback").click()
|
||||||
|
page.locator("#review-pull-feedback-suggestion").click()
|
||||||
|
page.wait_for_function("batchCalls.length === 2")
|
||||||
|
page.locator("#stage-pull-feedback-suggestion").click()
|
||||||
|
assert page.locator("#review-pull-feedback-batch").text_content() == "Review fixes (2)"
|
||||||
|
page.locator("#review-pull-feedback-batch").click()
|
||||||
|
assert page.locator("#pull-feedback-batch-review").is_visible()
|
||||||
|
assert page.locator("#pull-feedback-batch-items li").count() == 1
|
||||||
|
page.locator("#commit-pull-feedback-batch").click()
|
||||||
|
page.wait_for_function("batchCalls.length === 3")
|
||||||
|
result = page.evaluate("""() => ({
|
||||||
|
calls:batchCalls, head:batchDetail.head_sha,
|
||||||
|
reviewHidden:document.querySelector('#pull-feedback-batch-review').hidden,
|
||||||
|
status:document.querySelector('#pull-feedback-status').textContent,
|
||||||
|
scrollWidth:document.documentElement.scrollWidth,
|
||||||
|
clientWidth:document.documentElement.clientWidth,
|
||||||
|
heights:Array.from(document.querySelectorAll('#pull-feedback-pass button'))
|
||||||
|
.filter(button => button.getClientRects().length > 0).map(button => button.getBoundingClientRect().height),
|
||||||
|
})""")
|
||||||
|
browser.close()
|
||||||
|
|
||||||
|
assert result["scrollWidth"] <= result["clientWidth"]
|
||||||
|
assert min(result["heights"]) >= 44
|
||||||
|
assert result["head"] == "def5678"
|
||||||
|
assert result["reviewHidden"] is True
|
||||||
|
assert "2 suggested changes committed" in result["status"].lower()
|
||||||
|
request = result["calls"][2]
|
||||||
|
assert request["path"].endswith("/feedback-batch")
|
||||||
|
assert request["options"]["method"] == "POST"
|
||||||
|
body = request["options"]["body"]
|
||||||
|
assert '"expected_head_sha":"abc1234"' in body
|
||||||
|
assert '"file":{"path":"src/api.py"' in body
|
||||||
|
assert '"suggestion_count":2' in body
|
||||||
|
assert 'return empty_result()' in body
|
||||||
|
assert 'return typed_result()' in body
|
||||||
|
|
@ -108,6 +108,74 @@ async def test_gitea_commits_feedback_fix_and_verifies_advanced_pull_head():
|
||||||
]
|
]
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.anyio
|
||||||
|
async def test_gitea_commits_multiple_same_file_suggestions_as_one_commit():
|
||||||
|
requests = []
|
||||||
|
pull = {
|
||||||
|
"number": 7, "state": "open", "merged": False,
|
||||||
|
"user": {"login": "alex"},
|
||||||
|
"head": {"sha": "abc1234", "ref": "alex/review-fix", "repo": {"full_name": "stackchain/api"}},
|
||||||
|
"base": {"repo": {"full_name": "stackchain/api"}},
|
||||||
|
}
|
||||||
|
|
||||||
|
async def handler(request):
|
||||||
|
requests.append((request.method, request.url.path))
|
||||||
|
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":
|
||||||
|
return httpx.Response(200, json=pull)
|
||||||
|
if request.url.path == "/api/v1/repos/stackchain/api/contents/src/api.py" and request.method == "GET":
|
||||||
|
updated = request.url.params["ref"] == "def5678"
|
||||||
|
return httpx.Response(200, json=_content_payload(
|
||||||
|
"first fixed\nsecond fixed\n" if updated else "first\nsecond\n",
|
||||||
|
"blob2" if updated else "blob1",
|
||||||
|
))
|
||||||
|
if request.url.path == "/api/v1/repos/stackchain/api/contents/src/api.py" and request.method == "PUT":
|
||||||
|
body = json.loads(request.content)
|
||||||
|
assert body["branch"] == "alex/review-fix"
|
||||||
|
assert base64.b64decode(body["content"]).decode() == "first fixed\nsecond fixed\n"
|
||||||
|
pull["head"]["sha"] = "def5678"
|
||||||
|
return httpx.Response(200, json={"commit": {"sha": "def5678"}})
|
||||||
|
raise AssertionError(f"unexpected request: {request.method} {request.url}")
|
||||||
|
|
||||||
|
gitea_proxy.start_client(transport=httpx.MockTransport(handler))
|
||||||
|
try:
|
||||||
|
result = await gitea_proxy.commit_authored_pull_feedback_batch(
|
||||||
|
"stackchain/api", 7,
|
||||||
|
{"path": "src/api.py", "content": "first fixed\nsecond fixed\n", "expected_blob_sha": "blob1"},
|
||||||
|
2, "fix: apply review suggestions", "abc1234",
|
||||||
|
)
|
||||||
|
finally:
|
||||||
|
await gitea_proxy.stop_client()
|
||||||
|
|
||||||
|
assert result["previous_head_sha"] == "abc1234"
|
||||||
|
assert result["head_sha"] == "def5678"
|
||||||
|
assert result["path"] == "src/api.py"
|
||||||
|
assert result["suggestion_count"] == 2
|
||||||
|
assert len([request for request in requests if request[0] == "PUT"]) == 1
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.anyio
|
||||||
|
async def test_feedback_batch_api_requires_at_least_two_suggestions(monkeypatch):
|
||||||
|
called = False
|
||||||
|
|
||||||
|
async def commit(*args):
|
||||||
|
nonlocal called
|
||||||
|
called = True
|
||||||
|
|
||||||
|
monkeypatch.setattr(main.gitea_proxy, "commit_authored_pull_feedback_batch", commit, raising=False)
|
||||||
|
transport = httpx.ASGITransport(app=main.app)
|
||||||
|
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
||||||
|
response = await client.post(
|
||||||
|
"/api/v1/repos/stackchain/api/pulls/7/feedback-batch",
|
||||||
|
json={"message": "fix: suggestions", "expected_head_sha": "abc1234", "suggestion_count": 1,
|
||||||
|
"file": {"path": "src/api.py", "content": "one\n", "expected_blob_sha": "blob1"}},
|
||||||
|
)
|
||||||
|
|
||||||
|
assert response.status_code == 422
|
||||||
|
assert called is False
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.anyio
|
@pytest.mark.anyio
|
||||||
async def test_feedback_file_api_loads_and_commits_an_authored_pull_fix(monkeypatch):
|
async def test_feedback_file_api_loads_and_commits_an_authored_pull_fix(monkeypatch):
|
||||||
calls = []
|
calls = []
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user