Skip to content

Commit dc8afd1

Browse files
bircnibircni
committed
fix: Review Comments
1 parent 1f8d276 commit dc8afd1

File tree

4 files changed

+34
-35
lines changed

4 files changed

+34
-35
lines changed

‎templates/repo/diff/comment_form.tmpl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
"CustomInit" true
1616
"MarkdownPreviewInRepo" $.root.Repository
1717
"MarkdownPreviewMode" "comment"
18-
"ShowSuggestionButton" $.root.PageIsPullFiles
18+
"ShowSuggestionButton" (and $.root.PageIsPullFiles (not $.root.Issue.PullRequest.IsAgitFlow))
1919
"TextareaName" "content"
2020
"TextareaPlaceholder" (ctx.Locale.Tr "repo.diff.comment.placeholder")
2121
"DropzoneParentContainer" "form"

‎templates/repo/diff/comments.tmpl‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
{{range .comments}}
22

33
{{$createdStr:= DateUtils.TimeSince .CreatedUnix}}
4-
{{$canApplySuggestion := and $.root.CanWriteToHeadRepo (ge .Line 0)}}
4+
{{$isAgit := and $.root.Issue $.root.Issue.PullRequest $.root.Issue.PullRequest.IsAgitFlow}}
5+
{{$canApplySuggestion := and $.root.CanWriteToHeadRepo (ge .Line 0) (not $isAgit)}}
56
<div class="comment" id="{{.HashTag}}" data-comment-id="{{.ID}}" data-apply-suggestion-url="{{$.root.RepoLink}}/comments/{{.ID}}/suggestions/apply" data-apply-suggestion-text="{{ctx.Locale.Tr "repo.diff.comment.apply_suggestion"}}" data-can-apply-suggestion="{{if $canApplySuggestion}}true{{else}}false{{end}}">
67
<div class="tw-mt-2 tw-mr-4">
78
{{if .OriginalAuthor}}

‎templates/repo/issue/view_content/conversation.tmpl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
{{/* FIXME: DIFF-CONVERSATION-DATA: in the future this template should be refactor to avoid called by curly-braces-dot-dollar
1+
{{/* FIXME: DIFF-CONVERSATION-DATA: in the future this template should be refactor to avoid called by {{... "." $}}
22
At the moment, two kinds of request handler call this template:
33
* ExcerptBlob -> blob_excerpt.tmpl -> this
44
* Other compare and diff pages -> ... -> {section_unified.tmpl|section_split.tmpl} -> this)

‎web_src/js/features/repo-issue.ts‎

Lines changed: 30 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -257,12 +257,15 @@ export async function handleReply(el: HTMLElement) {
257257

258258
export function initSuggestionApplyButtons(root: ParentNode = document) {
259259
for (const comment of root.querySelectorAll<HTMLElement>('.comment[data-apply-suggestion-url]')) {
260-
const canApply = comment.getAttribute('data-can-apply-suggestion') === 'true';
261-
if (!canApply) continue;
260+
const canApplyValue = comment.getAttribute('data-can-apply-suggestion');
261+
if (canApplyValue === null) throw new Error('Missing data-can-apply-suggestion on comment');
262+
if (canApplyValue !== 'true') continue;
262263
const applyUrl = comment.getAttribute('data-apply-suggestion-url');
264+
if (!applyUrl) throw new Error('Missing data-apply-suggestion-url on comment');
263265
const commentId = comment.getAttribute('data-comment-id');
264-
if (!applyUrl || !commentId) continue;
265-
const buttonLabel = comment.getAttribute('data-apply-suggestion-text') || 'Apply suggestion';
266+
if (!commentId) throw new Error('Missing data-comment-id on comment');
267+
const buttonLabel = comment.getAttribute('data-apply-suggestion-text');
268+
if (!buttonLabel) throw new Error('Missing data-apply-suggestion-text on comment');
266269
const suggestionBlocks = Array.from(comment.querySelectorAll('pre > code.language-suggestion'));
267270
let index = 0;
268271
for (const codeEl of suggestionBlocks) {
@@ -272,10 +275,7 @@ export function initSuggestionApplyButtons(root: ParentNode = document) {
272275
}
273276
codeEl.setAttribute('data-suggestion-initialized', 'true');
274277
const pre = codeEl.parentElement;
275-
if (!pre) {
276-
index++;
277-
continue;
278-
}
278+
if (!pre) throw new Error('Suggestion code block is missing a parent element');
279279
pre.classList.add('suggestion-block');
280280
const actions = createElementFromHTML(html`
281281
<div class="suggestion-actions">
@@ -341,8 +341,9 @@ export function initRepoPullRequestReview() {
341341
e.preventDefault();
342342
if (el.classList.contains('is-loading')) return;
343343
const url = el.getAttribute('data-apply-url');
344+
if (!url) throw new Error('Missing data-apply-url on apply suggestion button');
344345
const index = el.getAttribute('data-suggestion-index');
345-
if (!url || index === null) return;
346+
if (index === null) throw new Error('Missing data-suggestion-index on apply suggestion button');
346347
try {
347348
el.classList.add('is-loading');
348349
const response = await POST(url, {data: new URLSearchParams({index})});
@@ -398,7 +399,7 @@ export function initRepoPullRequestReview() {
398399

399400
const getSuggestionLinesFromDiff = (path: string, side: 'left' | 'right', start: number, end: number): string[] => {
400401
const table = document.querySelector<HTMLTableElement>(`.code-diff table[data-path="${CSS.escape(path)}"]`);
401-
if (!table) return [];
402+
if (!table) throw new Error('Suggestion selection table not found');
402403
const sideClass = side === 'right' ? 'lines-num-new' : 'lines-num-old';
403404
const lines: string[] = [];
404405
const rangeStart = Math.min(start, end);
@@ -482,19 +483,12 @@ export function initRepoPullRequestReview() {
482483
}
483484
});
484485

485-
addDelegatedEventListener(document, 'click', '.code-diff td.lines-num span', (el, e: MouseEvent) => {
486-
const td = el.closest<HTMLTableCellElement>('td.lines-num');
487-
if (!td) return;
488-
const lineValue = td.getAttribute('data-line-num');
489-
if (!lineValue) return;
490-
const lineNum = Number(lineValue);
491-
if (!lineNum) return;
492-
const table = td.closest<HTMLTableElement>('table[data-path]');
493-
if (!table) return;
494-
const path = table.getAttribute('data-path');
495-
if (!path) return;
496-
const side = td.classList.contains('lines-num-new') ? 'right' : (td.classList.contains('lines-num-old') ? 'left' : null);
497-
if (!side) return;
486+
addDelegatedEventListener(document, 'click', '.code-diff td.lines-num[data-line-num]:not([data-line-num=""]) span', (el, e: MouseEvent) => {
487+
const td = el.closest<HTMLTableCellElement>('td.lines-num')!;
488+
const lineNum = Number(td.getAttribute('data-line-num')!);
489+
const table = td.closest<HTMLTableElement>('table[data-path]')!;
490+
const path = table.getAttribute('data-path')!;
491+
const side = td.classList.contains('lines-num-new') ? 'right' : 'left';
498492
const isShiftSelect = e.shiftKey && diffSelection?.path === path && diffSelection?.side === side;
499493
const start = isShiftSelect ? diffSelection!.start : lineNum;
500494
const end = lineNum;
@@ -504,26 +498,30 @@ export function initRepoPullRequestReview() {
504498

505499
addDelegatedEventListener(document, 'click', '.markdown-button-suggestion', (el, e) => {
506500
e.preventDefault();
507-
const emptyMessage = el.getAttribute('data-suggestion-empty') ?? 'No selected lines available for suggestion.';
508-
const proposedOnlyMessage = el.getAttribute('data-suggestion-proposed-only') ?? 'Suggestions can only be added on the proposed side.';
501+
const emptyMessage = el.getAttribute('data-suggestion-empty');
502+
if (!emptyMessage) throw new Error('Missing data-suggestion-empty on suggestion button');
503+
const proposedOnlyMessage = el.getAttribute('data-suggestion-proposed-only');
504+
if (!proposedOnlyMessage) throw new Error('Missing data-suggestion-proposed-only on suggestion button');
509505
const form = el.closest<HTMLFormElement>('form');
510-
const textarea = form?.querySelector<HTMLTextAreaElement>('textarea.markdown-text-editor');
511-
if (!form || !textarea) return;
506+
if (!form) throw new Error('Suggestion button is not inside a form');
507+
const textarea = form.querySelector<HTMLTextAreaElement>('textarea.markdown-text-editor');
508+
if (!textarea) throw new Error('Suggestion form is missing the markdown textarea');
512509
const path = form.querySelector<HTMLInputElement>("input[name='path']")?.value;
513510
const side = form.querySelector<HTMLInputElement>("input[name='side']")?.value;
514511
const lineStartValue = form.querySelector<HTMLInputElement>("input[name='line_start']")?.value;
515512
const lineEndValue = form.querySelector<HTMLInputElement>("input[name='line_end']")?.value;
516-
if (!path || !side) return;
517-
if (!lineStartValue || !lineEndValue) {
518-
showErrorToast(emptyMessage);
519-
return;
520-
}
513+
if (!path) throw new Error('Suggestion form missing path');
514+
if (!side) throw new Error('Suggestion form missing side');
515+
if (!lineStartValue || !lineEndValue) throw new Error('Suggestion form missing line range');
521516
if (side !== 'proposed') {
522517
showErrorToast(proposedOnlyMessage);
523518
return;
524519
}
525520
const lineStart = Number(lineStartValue);
526521
const lineEnd = Number(lineEndValue);
522+
if (!Number.isInteger(lineStart) || !Number.isInteger(lineEnd) || lineStart <= 0 || lineEnd <= 0) {
523+
throw new Error('Suggestion form has invalid line range');
524+
}
527525
const lines = getSuggestionLinesFromDiff(path, 'right', lineStart, lineEnd);
528526
if (!lines.length) {
529527
showErrorToast(emptyMessage);

0 commit comments

Comments
 (0)