mirror of
				https://github.com/go-gitea/gitea
				synced 2025-11-04 05:18:25 +00:00 
			
		
		
		
	Fix PR diff review form submit (#32596)
Fix #31622, there is a longstanding bug in #19612, it doesn't handle submit event, correctly.
This commit is contained in:
		@@ -7,30 +7,20 @@ import {validateTextareaNonEmpty} from './comp/ComboMarkdownEditor.ts';
 | 
			
		||||
import {initViewedCheckboxListenerFor, countAndUpdateViewedFiles, initExpandAndCollapseFilesButton} from './pull-view-file.ts';
 | 
			
		||||
import {initImageDiff} from './imagediff.ts';
 | 
			
		||||
import {showErrorToast} from '../modules/toast.ts';
 | 
			
		||||
import {submitEventSubmitter, queryElemSiblings, hideElem, showElem, animateOnce} from '../utils/dom.ts';
 | 
			
		||||
import {
 | 
			
		||||
  submitEventSubmitter,
 | 
			
		||||
  queryElemSiblings,
 | 
			
		||||
  hideElem,
 | 
			
		||||
  showElem,
 | 
			
		||||
  animateOnce,
 | 
			
		||||
  addDelegatedEventListener,
 | 
			
		||||
  createElementFromHTML,
 | 
			
		||||
} from '../utils/dom.ts';
 | 
			
		||||
import {POST, GET} from '../modules/fetch.ts';
 | 
			
		||||
import {fomanticQuery} from '../modules/fomantic/base.ts';
 | 
			
		||||
 | 
			
		||||
const {pageData, i18n} = window.config;
 | 
			
		||||
 | 
			
		||||
function initRepoDiffReviewButton() {
 | 
			
		||||
  const reviewBox = document.querySelector('#review-box');
 | 
			
		||||
  if (!reviewBox) return;
 | 
			
		||||
 | 
			
		||||
  const counter = reviewBox.querySelector('.review-comments-counter');
 | 
			
		||||
  if (!counter) return;
 | 
			
		||||
 | 
			
		||||
  $(document).on('click', 'button[name="pending_review"]', (e) => {
 | 
			
		||||
    const $form = $(e.target).closest('form');
 | 
			
		||||
    // Watch for the form's submit event.
 | 
			
		||||
    $form.on('submit', () => {
 | 
			
		||||
      const num = parseInt(counter.getAttribute('data-pending-comment-number')) + 1 || 1;
 | 
			
		||||
      counter.setAttribute('data-pending-comment-number', num);
 | 
			
		||||
      counter.textContent = num;
 | 
			
		||||
      animateOnce(reviewBox, 'pulse-1p5-200');
 | 
			
		||||
    });
 | 
			
		||||
  });
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
function initRepoDiffFileViewToggle() {
 | 
			
		||||
  $('.file-view-toggle').on('click', function () {
 | 
			
		||||
    for (const el of queryElemSiblings(this)) {
 | 
			
		||||
@@ -47,19 +37,15 @@ function initRepoDiffFileViewToggle() {
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
function initRepoDiffConversationForm() {
 | 
			
		||||
  $(document).on('submit', '.conversation-holder form', async (e) => {
 | 
			
		||||
  addDelegatedEventListener<HTMLFormElement>(document, 'submit', '.conversation-holder form', async (form, e) => {
 | 
			
		||||
    e.preventDefault();
 | 
			
		||||
    const textArea = form.querySelector<HTMLTextAreaElement>('textarea');
 | 
			
		||||
    if (!validateTextareaNonEmpty(textArea)) return;
 | 
			
		||||
    if (form.classList.contains('is-loading')) return;
 | 
			
		||||
 | 
			
		||||
    const $form = $(e.target);
 | 
			
		||||
    const textArea = e.target.querySelector('textarea');
 | 
			
		||||
    if (!validateTextareaNonEmpty(textArea)) {
 | 
			
		||||
      return;
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    if (e.target.classList.contains('is-loading')) return;
 | 
			
		||||
    try {
 | 
			
		||||
      e.target.classList.add('is-loading');
 | 
			
		||||
      const formData = new FormData($form[0]);
 | 
			
		||||
      form.classList.add('is-loading');
 | 
			
		||||
      const formData = new FormData(form);
 | 
			
		||||
 | 
			
		||||
      // if the form is submitted by a button, append the button's name and value to the form data
 | 
			
		||||
      const submitter = submitEventSubmitter(e);
 | 
			
		||||
@@ -68,26 +54,42 @@ function initRepoDiffConversationForm() {
 | 
			
		||||
        formData.append(submitter.name, submitter.value);
 | 
			
		||||
      }
 | 
			
		||||
 | 
			
		||||
      const response = await POST(e.target.getAttribute('action'), {data: formData});
 | 
			
		||||
      const $newConversationHolder = $(await response.text());
 | 
			
		||||
      const {path, side, idx} = $newConversationHolder.data();
 | 
			
		||||
      const trLineType = form.closest('tr').getAttribute('data-line-type');
 | 
			
		||||
      const response = await POST(form.getAttribute('action'), {data: formData});
 | 
			
		||||
      const newConversationHolder = createElementFromHTML(await response.text());
 | 
			
		||||
      const path = newConversationHolder.getAttribute('data-path');
 | 
			
		||||
      const side = newConversationHolder.getAttribute('data-side');
 | 
			
		||||
      const idx = newConversationHolder.getAttribute('data-idx');
 | 
			
		||||
 | 
			
		||||
      form.closest('.conversation-holder').replaceWith(newConversationHolder);
 | 
			
		||||
      form = null; // prevent further usage of the form because it should have been replaced
 | 
			
		||||
 | 
			
		||||
      $form.closest('.conversation-holder').replaceWith($newConversationHolder);
 | 
			
		||||
      let selector;
 | 
			
		||||
      if ($form.closest('tr').data('line-type') === 'same') {
 | 
			
		||||
      if (trLineType === 'same') {
 | 
			
		||||
        selector = `[data-path="${path}"] .add-code-comment[data-idx="${idx}"]`;
 | 
			
		||||
      } else {
 | 
			
		||||
        selector = `[data-path="${path}"] .add-code-comment[data-side="${side}"][data-idx="${idx}"]`;
 | 
			
		||||
      }
 | 
			
		||||
      for (const el of document.querySelectorAll(selector)) {
 | 
			
		||||
        el.classList.add('tw-invisible');
 | 
			
		||||
        el.classList.add('tw-invisible'); // TODO need to figure out why
 | 
			
		||||
      }
 | 
			
		||||
      fomanticQuery(newConversationHolder.querySelectorAll('.ui.dropdown')).dropdown();
 | 
			
		||||
 | 
			
		||||
      // the default behavior is to add a pending review, so if no submitter, it also means "pending_review"
 | 
			
		||||
      if (!submitter || submitter?.matches('button[name="pending_review"]')) {
 | 
			
		||||
        const reviewBox = document.querySelector('#review-box');
 | 
			
		||||
        const counter = reviewBox?.querySelector('.review-comments-counter');
 | 
			
		||||
        if (!counter) return;
 | 
			
		||||
        const num = parseInt(counter.getAttribute('data-pending-comment-number')) + 1 || 1;
 | 
			
		||||
        counter.setAttribute('data-pending-comment-number', String(num));
 | 
			
		||||
        counter.textContent = String(num);
 | 
			
		||||
        animateOnce(reviewBox, 'pulse-1p5-200');
 | 
			
		||||
      }
 | 
			
		||||
      $newConversationHolder.find('.dropdown').dropdown();
 | 
			
		||||
    } catch (error) {
 | 
			
		||||
      console.error('Error:', error);
 | 
			
		||||
      showErrorToast(i18n.network_error);
 | 
			
		||||
    } finally {
 | 
			
		||||
      e.target.classList.remove('is-loading');
 | 
			
		||||
      form?.classList.remove('is-loading');
 | 
			
		||||
    }
 | 
			
		||||
  });
 | 
			
		||||
 | 
			
		||||
@@ -219,7 +221,6 @@ export function initRepoDiffView() {
 | 
			
		||||
  initDiffFileList();
 | 
			
		||||
  initDiffCommitSelect();
 | 
			
		||||
  initRepoDiffShowMore();
 | 
			
		||||
  initRepoDiffReviewButton();
 | 
			
		||||
  initRepoDiffFileViewToggle();
 | 
			
		||||
  initViewedCheckboxListenerFor();
 | 
			
		||||
  initExpandAndCollapseFilesButton();
 | 
			
		||||
 
 | 
			
		||||
@@ -1,7 +1,7 @@
 | 
			
		||||
import $ from 'jquery';
 | 
			
		||||
import {htmlEscape} from 'escape-goat';
 | 
			
		||||
import {createTippy, showTemporaryTooltip} from '../modules/tippy.ts';
 | 
			
		||||
import {hideElem, showElem, toggleElem} from '../utils/dom.ts';
 | 
			
		||||
import {addDelegatedEventListener, createElementFromHTML, hideElem, showElem, toggleElem} from '../utils/dom.ts';
 | 
			
		||||
import {setFileFolding} from './file-fold.ts';
 | 
			
		||||
import {ComboMarkdownEditor, getComboMarkdownEditor, initComboMarkdownEditor} from './comp/ComboMarkdownEditor.ts';
 | 
			
		||||
import {parseIssuePageInfo, toAbsoluteUrl} from '../utils.ts';
 | 
			
		||||
@@ -443,21 +443,19 @@ export function initRepoPullRequestReview() {
 | 
			
		||||
    });
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  $(document).on('click', '.add-code-comment', async function (e) {
 | 
			
		||||
    if (e.target.classList.contains('btn-add-single')) return; // https://github.com/go-gitea/gitea/issues/4745
 | 
			
		||||
  addDelegatedEventListener(document, 'click', '.add-code-comment', async (el, e) => {
 | 
			
		||||
    e.preventDefault();
 | 
			
		||||
 | 
			
		||||
    const isSplit = this.closest('.code-diff')?.classList.contains('code-diff-split');
 | 
			
		||||
    const side = this.getAttribute('data-side');
 | 
			
		||||
    const idx = this.getAttribute('data-idx');
 | 
			
		||||
    const path = this.closest('[data-path]')?.getAttribute('data-path');
 | 
			
		||||
    const tr = this.closest('tr');
 | 
			
		||||
    const isSplit = el.closest('.code-diff')?.classList.contains('code-diff-split');
 | 
			
		||||
    const side = el.getAttribute('data-side');
 | 
			
		||||
    const idx = el.getAttribute('data-idx');
 | 
			
		||||
    const path = el.closest('[data-path]')?.getAttribute('data-path');
 | 
			
		||||
    const tr = el.closest('tr');
 | 
			
		||||
    const lineType = tr.getAttribute('data-line-type');
 | 
			
		||||
 | 
			
		||||
    const ntr = tr.nextElementSibling;
 | 
			
		||||
    let $ntr = $(ntr);
 | 
			
		||||
    let ntr = tr.nextElementSibling;
 | 
			
		||||
    if (!ntr?.classList.contains('add-comment')) {
 | 
			
		||||
      $ntr = $(`
 | 
			
		||||
      ntr = createElementFromHTML(`
 | 
			
		||||
        <tr class="add-comment" data-line-type="${lineType}">
 | 
			
		||||
          ${isSplit ? `
 | 
			
		||||
            <td class="add-comment-left" colspan="4"></td>
 | 
			
		||||
@@ -466,24 +464,18 @@ export function initRepoPullRequestReview() {
 | 
			
		||||
            <td class="add-comment-left add-comment-right" colspan="5"></td>
 | 
			
		||||
          `}
 | 
			
		||||
        </tr>`);
 | 
			
		||||
      $(tr).after($ntr);
 | 
			
		||||
      tr.after(ntr);
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    const $td = $ntr.find(`.add-comment-${side}`);
 | 
			
		||||
    const $commentCloud = $td.find('.comment-code-cloud');
 | 
			
		||||
    if (!$commentCloud.length && !$ntr.find('button[name="pending_review"]').length) {
 | 
			
		||||
      try {
 | 
			
		||||
        const response = await GET(this.closest('[data-new-comment-url]')?.getAttribute('data-new-comment-url'));
 | 
			
		||||
        const html = await response.text();
 | 
			
		||||
        $td.html(html);
 | 
			
		||||
        $td.find("input[name='line']").val(idx);
 | 
			
		||||
        $td.find("input[name='side']").val(side === 'left' ? 'previous' : 'proposed');
 | 
			
		||||
        $td.find("input[name='path']").val(path);
 | 
			
		||||
        const editor = await initComboMarkdownEditor($td[0].querySelector('.combo-markdown-editor'));
 | 
			
		||||
        editor.focus();
 | 
			
		||||
      } catch (error) {
 | 
			
		||||
        console.error(error);
 | 
			
		||||
      }
 | 
			
		||||
    const td = ntr.querySelector(`.add-comment-${side}`);
 | 
			
		||||
    const commentCloud = td.querySelector('.comment-code-cloud');
 | 
			
		||||
    if (!commentCloud && !ntr.querySelector('button[name="pending_review"]')) {
 | 
			
		||||
      const response = await GET(el.closest('[data-new-comment-url]')?.getAttribute('data-new-comment-url'));
 | 
			
		||||
      td.innerHTML = await response.text();
 | 
			
		||||
      td.querySelector<HTMLInputElement>("input[name='line']").value = idx;
 | 
			
		||||
      td.querySelector<HTMLInputElement>("input[name='side']").value = (side === 'left' ? 'previous' : 'proposed');
 | 
			
		||||
      td.querySelector<HTMLInputElement>("input[name='path']").value = path;
 | 
			
		||||
      const editor = await initComboMarkdownEditor(td.querySelector<HTMLElement>('.combo-markdown-editor'));
 | 
			
		||||
      editor.focus();
 | 
			
		||||
    }
 | 
			
		||||
  });
 | 
			
		||||
}
 | 
			
		||||
 
 | 
			
		||||
@@ -2,6 +2,7 @@ import {createElementFromAttrs, createElementFromHTML, querySingleVisibleElem} f
 | 
			
		||||
 | 
			
		||||
test('createElementFromHTML', () => {
 | 
			
		||||
  expect(createElementFromHTML('<a>foo<span>bar</span></a>').outerHTML).toEqual('<a>foo<span>bar</span></a>');
 | 
			
		||||
  expect(createElementFromHTML('<tr data-x="1"><td>foo</td></tr>').outerHTML).toEqual('<tr data-x="1"><td>foo</td></tr>');
 | 
			
		||||
});
 | 
			
		||||
 | 
			
		||||
test('createElementFromAttrs', () => {
 | 
			
		||||
 
 | 
			
		||||
@@ -301,10 +301,17 @@ export function replaceTextareaSelection(textarea: HTMLTextAreaElement, text: st
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
// Warning: Do not enter any unsanitized variables here
 | 
			
		||||
export function createElementFromHTML(htmlString: string): HTMLElement {
 | 
			
		||||
export function createElementFromHTML<T extends HTMLElement>(htmlString: string): T {
 | 
			
		||||
  htmlString = htmlString.trim();
 | 
			
		||||
  // some tags like "tr" are special, it must use a correct parent container to create
 | 
			
		||||
  if (htmlString.startsWith('<tr')) {
 | 
			
		||||
    const container = document.createElement('table');
 | 
			
		||||
    container.innerHTML = htmlString;
 | 
			
		||||
    return container.querySelector<T>('tr');
 | 
			
		||||
  }
 | 
			
		||||
  const div = document.createElement('div');
 | 
			
		||||
  div.innerHTML = htmlString.trim();
 | 
			
		||||
  return div.firstChild as HTMLElement;
 | 
			
		||||
  div.innerHTML = htmlString;
 | 
			
		||||
  return div.firstChild as T;
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
export function createElementFromAttrs(tagName: string, attrs: Record<string, any>, ...children: (Node|string)[]): HTMLElement {
 | 
			
		||||
@@ -340,3 +347,11 @@ export function querySingleVisibleElem<T extends HTMLElement>(parent: Element, s
 | 
			
		||||
  if (candidates.length > 1) throw new Error(`Expected exactly one visible element matching selector "${selector}", but found ${candidates.length}`);
 | 
			
		||||
  return candidates.length ? candidates[0] as T : null;
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
export function addDelegatedEventListener<T extends HTMLElement>(parent: Node, type: string, selector: string, listener: (elem: T, e: Event) => void | Promise<any>, options?: boolean | AddEventListenerOptions) {
 | 
			
		||||
  parent.addEventListener(type, (e: Event) => {
 | 
			
		||||
    const elem = (e.target as HTMLElement).closest(selector);
 | 
			
		||||
    if (!elem) return;
 | 
			
		||||
    listener(elem as T, e);
 | 
			
		||||
  }, options);
 | 
			
		||||
}
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user