Change form actions to fetch for submit review box (go-gitea#25219)

HesterG · wxiaoguang · web-flow · commit a43ea2247933 · 2023-06-14T16:01:37.000+08:00
Co-author: @wxiaoguang Close go-gitea#25096 The way to fix it in this PR is to change form submit to fetch using formData, and add flags to avoid post repeatedly. Should be able to apply to more forms that have the same issue after this PR. In the demo below, 'approve' is clicked several times, and then 'comment' is clicked several time after 'request changes' clicked. After: https://github.com/go-gitea/gitea/assets/17645053/beabeb1d-fe66-4b76-b048-4f022b4e83a0 Update: screenshots from /devtest > ![image](https://user-images.githubusercontent.com/2114189/245680011-ee4231e0-a53d-4c2a-a9c2-71ccd98005cc.png) > > ![image](https://user-images.githubusercontent.com/2114189/245680057-9215d348-63d8-406d-8828-17e171163aaa.png) > > ![image](https://user-images.githubusercontent.com/2114189/245680148-89d7b3d1-d7b6-442f-b69e-eadaee112482.png) --------- Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
diff --git a/modules/context/base.go b/modules/context/base.go
@@ -132,6 +132,10 @@ func (b *Base) JSON(status int, content interface{}) {
 	}
 }
 
+func (b *Base) JSONRedirect(redirect string) {
+	b.JSON(http.StatusOK, map[string]any{"redirect": redirect})
+}
+
 // RemoteAddr returns the client machine ip address
 func (b *Base) RemoteAddr() string {
 	return b.Req.RemoteAddr
diff --git a/routers/web/devtest/devtest.go b/routers/web/devtest/devtest.go
@@ -32,6 +32,16 @@ func List(ctx *context.Context) {
 	ctx.HTML(http.StatusOK, "devtest/list")
 }
 
+func FetchActionTest(ctx *context.Context) {
+	_ = ctx.Req.ParseForm()
+	ctx.Flash.Info(ctx.Req.Method + " " + ctx.Req.RequestURI + "<br>" +
+		"Form: " + ctx.Req.Form.Encode() + "<br>" +
+		"PostForm: " + ctx.Req.PostForm.Encode(),
+	)
+	time.Sleep(2 * time.Second)
+	ctx.JSONRedirect("")
+}
+
 func Tmpl(ctx *context.Context) {
 	now := time.Now()
 	ctx.Data["TimeNow"] = now
diff --git a/routers/web/repo/pull_review.go b/routers/web/repo/pull_review.go
@@ -193,7 +193,7 @@ func SubmitReview(ctx *context.Context) {
 	}
 	if ctx.HasError() {
 		ctx.Flash.Error(ctx.Data["ErrorMsg"].(string))
-		ctx.Redirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index))
+		ctx.JSONRedirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index))
 		return
 	}
 
@@ -214,7 +214,7 @@ func SubmitReview(ctx *context.Context) {
 			}
 
 			ctx.Flash.Error(translated)
-			ctx.Redirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index))
+			ctx.JSONRedirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index))
 			return
 		}
 	}
@@ -228,14 +228,13 @@ func SubmitReview(ctx *context.Context) {
 	if err != nil {
 		if issues_model.IsContentEmptyErr(err) {
 			ctx.Flash.Error(ctx.Tr("repo.issues.review.content.empty"))
-			ctx.Redirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index))
+			ctx.JSONRedirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index))
 		} else {
 			ctx.ServerError("SubmitReview", err)
 		}
 		return
 	}
-
-	ctx.Redirect(fmt.Sprintf("%s/pulls/%d#%s", ctx.Repo.RepoLink, issue.Index, comm.HashTag()))
+	ctx.JSONRedirect(fmt.Sprintf("%s/pulls/%d#%s", ctx.Repo.RepoLink, issue.Index, comm.HashTag()))
 }
 
 // DismissReview dismissing stale review by repo admin
diff --git a/routers/web/web.go b/routers/web/web.go
@@ -1411,6 +1411,7 @@ func registerRoutes(m *web.Route) {
 
 	if !setting.IsProd {
 		m.Any("/devtest", devtest.List)
+		m.Any("/devtest/fetch-action-test", devtest.FetchActionTest)
 		m.Any("/devtest/{sub}", devtest.Tmpl)
 	}
 
diff --git a/templates/devtest/fetch-action.tmpl b/templates/devtest/fetch-action.tmpl
@@ -0,0 +1,42 @@
+{{template "base/head" .}}
+<div class="page-content devtest ui container">
+	{{template "base/alert" .}}
+	<div>
+		<h1>link-action</h1>
+		<div>
+			Use "window.fetch" to send a request to backend, the request is defined in an "A" or "BUTTON" element.
+			It might be renamed to "link-fetch-action" to match the "form-fetch-action".
+		</div>
+		<div>
+			<button class="link-action" data-url="fetch-action-test?k=1">test</button>
+		</div>
+	</div>
+	<div>
+		<h1>form-fetch-action</h1>
+		<div>Use "window.fetch" to send a form request to backend</div>
+		<div>
+			<form method="get" action="fetch-action-test?k=1" class="form-fetch-action">
+				<button name="btn">submit get</button>
+			</form>
+			<form method="post" action="fetch-action-test?k=1" class="form-fetch-action">
+				<div><textarea name="text" rows="3" class="js-quick-submit"></textarea></div>
+				<div><label><input name="check" type="checkbox"> check</label></div>
+				<div><button name="btn">submit post</button></div>
+			</form>
+			<form method="post" action="/no-such-uri" class="form-fetch-action">
+				<div class="gt-py-5">bad action url</div>
+				<div><button name="btn">submit test</button></div>
+			</form>
+		</div>
+	</div>
+</div>
+<style>
+	.ui.message.flash-message {
+		text-align: left;
+	}
+	.form-fetch-action {
+		margin-bottom: 1em;
+		border: 1px red dashed; /* show the border for demo purpose */
+	}
+</style>
+{{template "base/footer" .}}
diff --git a/templates/devtest/gitea-ui.tmpl b/templates/devtest/gitea-ui.tmpl
@@ -89,6 +89,17 @@
 		<div><span data-tooltip-content="test tooltip" data-tooltip-interactive="true">text with interactive tooltip</span></div>
 	</div>
 
+	<div>
+		<h1>Loading</h1>
+		<div class="is-loading small-loading-icon gt-border-secondary gt-py-2"><span>loading ...</span></div>
+		<div class="is-loading gt-border-secondary gt-py-4">
+			<p>loading ...</p>
+			<p>loading ...</p>
+			<p>loading ...</p>
+			<p>loading ...</p>
+		</div>
+	</div>
+
 	<div>
 		<h1>GiteaOriginUrl</h1>
 		<div><gitea-origin-url data-url="test/url"></gitea-origin-url></div>
diff --git a/templates/devtest/list.tmpl b/templates/devtest/list.tmpl
@@ -1,12 +1,15 @@
-<style>
-	@media (prefers-color-scheme: dark) {
-		:root {
-			color-scheme: dark;
-		}
-	}
-</style>
+{{template "base/head" .}}
+
 <ul>
 	{{range .SubNames}}
 	<li><a href="{{AppSubUrl}}/devtest/{{.}}">{{.}}</a></li>
 	{{end}}
 </ul>
+
+<style>
+ul {
+	line-height: 2em;
+}
+</style>
+
+{{template "base/footer" .}}
diff --git a/templates/repo/diff/new_review.tmpl b/templates/repo/diff/new_review.tmpl
@@ -6,7 +6,7 @@
 	</button>
 	<div class="review-box-panel tippy-target">
 		<div class="ui segment">
-			<form class="ui form" action="{{.Link}}/reviews/submit" method="post">
+			<form class="ui form form-fetch-action" action="{{.Link}}/reviews/submit" method="post">
 				{{.CsrfTokenHtml}}
 				<input type="hidden" name="commit_id" value="{{.AfterCommitID}}">
 				<div class="field gt-df gt-ac">
diff --git a/web_src/css/modules/animations.css b/web_src/css/modules/animations.css
@@ -4,20 +4,22 @@
 }
 
 .is-loading {
-  background: transparent !important;
-  color: transparent !important;
-  border: transparent !important;
   pointer-events: none !important;
   position: relative !important;
   overflow: hidden !important;
 }
 
+.is-loading > * {
+  opacity: 0.3;
+}
+
 .is-loading::after {
   content: "";
   position: absolute;
   display: block;
-  width: 4rem;
   height: 4rem;
+  max-height: 50%;
+  aspect-ratio: 1 / 1;
   left: 50%;
   top: 50%;
   transform: translate(-50%, -50%);
@@ -28,30 +30,31 @@
   border-radius: 100%;
 }
 
+.is-loading.small-loading-icon::after {
+  border-width: 2px;
+}
+
 .markup pre.is-loading,
 .editor-loading.is-loading,
 .pdf-content.is-loading {
   height: var(--height-loading);
 }
 
+/* TODO: not needed, use "is-loading small-loading-icon" instead */
 .btn-octicon.is-loading::after {
   border-width: 2px;
   height: 1.25rem;
   width: 1.25rem;
 }
 
+/* TODO: not needed, use "is-loading small-loading-icon" instead */
 code.language-math.is-loading::after {
   padding: 0;
   border-width: 2px;
   width: 1.25rem;
   height: 1.25rem;
 }
 
-#oauth2-login-navigator.is-loading::after {
-  width: 40px;
-  height: 40px;
-}
-
 @keyframes fadein {
   0% {
     opacity: 0;
diff --git a/web_src/css/modules/tippy.css b/web_src/css/modules/tippy.css
@@ -29,6 +29,12 @@
   color: var(--color-text);
 }
 
+.tippy-box[data-theme="form-fetch-error"] {
+  border-color: var(--color-error-border);
+  background-color: var(--color-error-bg);
+  color: var(--color-error-text);
+}
+
 .tippy-content {
   position: relative;
   padding: 1rem;
diff --git a/web_src/js/features/common-global.js b/web_src/js/features/common-global.js
@@ -7,6 +7,7 @@ import {handleGlobalEnterQuickSubmit} from './comp/QuickSubmit.js';
 import {svg} from '../svg.js';
 import {hideElem, showElem, toggleElem} from '../utils/dom.js';
 import {htmlEscape} from 'escape-goat';
+import {createTippy} from '../modules/tippy.js';
 
 const {appUrl, csrfToken, i18n} = window.config;
 
@@ -60,6 +61,81 @@ export function initGlobalButtonClickOnEnter() {
   });
 }
 
+async function formFetchAction(e) {
+  if (!e.target.classList.contains('form-fetch-action')) return;
+
+  e.preventDefault();
+  const formEl = e.target;
+  if (formEl.classList.contains('is-loading')) return;
+
+  formEl.classList.add('is-loading');
+  if (formEl.clientHeight < 50) {
+    formEl.classList.add('small-loading-icon');
+  }
+
+  const formMethod = formEl.getAttribute('method') || 'get';
+  const formActionUrl = formEl.getAttribute('action');
+  const formData = new FormData(formEl);
+  const [submitterName, submitterValue] = [e.submitter?.getAttribute('name'), e.submitter?.getAttribute('value')];
+  if (submitterName) {
+    formData.append(submitterName, submitterValue || '');
+  }
+
+  let reqUrl = formActionUrl;
+  const reqOpt = {method: formMethod.toUpperCase(), headers: {'X-Csrf-Token': csrfToken}};
+  if (formMethod.toLowerCase() === 'get') {
+    const params = new URLSearchParams();
+    for (const [key, value] of formData) {
+      params.append(key, value.toString());
+    }
+    const pos = reqUrl.indexOf('?');
+    if (pos !== -1) {
+      reqUrl = reqUrl.slice(0, pos);
+    }
+    reqUrl += `?${params.toString()}`;
+  } else {
+    reqOpt.body = formData;
+  }
+
+  let errorTippy;
+  const onError = (msg) => {
+    formEl.classList.remove('is-loading', 'small-loading-icon');
+    if (errorTippy) errorTippy.destroy();
+    errorTippy = createTippy(formEl, {
+      content: msg,
+      interactive: true,
+      showOnCreate: true,
+      hideOnClick: true,
+      role: 'alert',
+      theme: 'form-fetch-error',
+      trigger: 'manual',
+      arrow: false,
+    });
+  };
+
+  const doRequest = async () => {
+    try {
+      const resp = await fetch(reqUrl, reqOpt);
+      if (resp.status === 200) {
+        const {redirect} = await resp.json();
+        formEl.classList.remove('dirty'); // remove the areYouSure check before reloading
+        if (redirect) {
+          window.location.href = redirect;
+        } else {
+          window.location.reload();
+        }
+      } else {
+        onError(`server error: ${resp.status}`);
+      }
+    } catch (e) {
+      onError(e.error);
+    }
+  };
+
+  // TODO: add "confirm" support like "link-action" in the future
+  await doRequest();
+}
+
 export function initGlobalCommon() {
   // Semantic UI modules.
   const $uiDropdowns = $('.ui.dropdown');
@@ -114,6 +190,8 @@ export function initGlobalCommon() {
     if (btn.classList.contains('loading')) return e.preventDefault();
     btn.classList.add('loading');
   });
+
+  document.addEventListener('submit', formFetchAction);
 }
 
 export function initGlobalDropzone() {
@@ -182,7 +260,7 @@ function linkAction(e) {
   const $this = $(e.target);
   const redirect = $this.attr('data-redirect');
 
-  const request = () => {
+  const doRequest = () => {
     $this.prop('disabled', true);
     $.post($this.attr('data-url'), {
       _csrf: csrfToken
@@ -201,7 +279,7 @@ function linkAction(e) {
 
   const modalConfirmHtml = htmlEscape($this.attr('data-modal-confirm') || '');
   if (!modalConfirmHtml) {
-    request();
+    doRequest();
     return;
   }
 
@@ -220,7 +298,7 @@ function linkAction(e) {
   $modal.appendTo(document.body);
   $modal.modal({
     onApprove() {
-      request();
+      doRequest();
     },
     onHidden() {
       $modal.remove();
diff --git a/web_src/js/features/comp/QuickSubmit.js b/web_src/js/features/comp/QuickSubmit.js
@@ -1,17 +1,24 @@
 import $ from 'jquery';
 
 export function handleGlobalEnterQuickSubmit(target) {
-  const $target = $(target);
-  const $form = $(target).closest('form');
-  if ($form.length) {
+  const form = target.closest('form');
+  if (form) {
+    if (!form.checkValidity()) {
+      form.reportValidity();
+      return;
+    }
+
+    if (form.classList.contains('form-fetch-action')) {
+      form.dispatchEvent(new SubmitEvent('submit', {bubbles: true, cancelable: true}));
+      return;
+    }
+
     // here use the event to trigger the submit event (instead of calling `submit()` method directly)
     // otherwise the `areYouSure` handler won't be executed, then there will be an annoying "confirm to leave" dialog
-    if ($form[0].checkValidity()) {
-      $form.trigger('submit');
-    }
+    $(form).trigger('submit');
   } else {
     // if no form, then the editor is for an AJAX request, dispatch an event to the target, let the target's event handler to do the AJAX request.
     // the 'ce-' prefix means this is a CustomEvent
-    $target.trigger('ce-quick-submit');
+    target.dispatchEvent(new CustomEvent('ce-quick-submit', {bubbles: true}));
   }
 }
diff --git a/web_src/js/features/repo-code.js b/web_src/js/features/repo-code.js
diff --git a/web_src/js/modules/tippy.js b/web_src/js/modules/tippy.js

Original file line number	Diff line number	Diff line change
`@@ -132,6 +132,10 @@ func (b *Base) JSON(status int, content interface{}) {`
`132`	`132`	`}`
`133`	`133`	`}`
`134`	`134`
	`135`	`+func (b *Base) JSONRedirect(redirect string) {`
	`136`	`+ b.JSON(http.StatusOK, map[string]any{"redirect": redirect})`
	`137`	`+}`
	`138`	`+`
`135`	`139`	`// RemoteAddr returns the client machine ip address`
`136`	`140`	`func (b *Base) RemoteAddr() string {`
`137`	`141`	`return b.Req.RemoteAddr`
Original file line number	Diff line number	Diff line change
`@@ -193,7 +193,7 @@ func SubmitReview(ctx *context.Context) {`
`193`	`193`	`}`
`194`	`194`	`if ctx.HasError() {`
`195`	`195`	`ctx.Flash.Error(ctx.Data["ErrorMsg"].(string))`
`196`		`- ctx.Redirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index))`
	`196`	`+ ctx.JSONRedirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index))`
`197`	`197`	`return`
`198`	`198`	`}`
`199`	`199`
`@@ -214,7 +214,7 @@ func SubmitReview(ctx *context.Context) {`
`214`	`214`	`}`
`215`	`215`
`216`	`216`	`ctx.Flash.Error(translated)`
`217`		`- ctx.Redirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index))`
	`217`	`+ ctx.JSONRedirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index))`
`218`	`218`	`return`
`219`	`219`	`}`
`220`	`220`	`}`
`@@ -228,14 +228,13 @@ func SubmitReview(ctx *context.Context) {`
`228`	`228`	`if err != nil {`
`229`	`229`	`if issues_model.IsContentEmptyErr(err) {`
`230`	`230`	`ctx.Flash.Error(ctx.Tr("repo.issues.review.content.empty"))`
`231`		`- ctx.Redirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index))`
	`231`	`+ ctx.JSONRedirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index))`
`232`	`232`	`} else {`
`233`	`233`	`ctx.ServerError("SubmitReview", err)`
`234`	`234`	`}`
`235`	`235`	`return`
`236`	`236`	`}`
`237`		`-`
`238`		`- ctx.Redirect(fmt.Sprintf("%s/pulls/%d#%s", ctx.Repo.RepoLink, issue.Index, comm.HashTag()))`
	`237`	`+ ctx.JSONRedirect(fmt.Sprintf("%s/pulls/%d#%s", ctx.Repo.RepoLink, issue.Index, comm.HashTag()))`
`239`	`238`	`}`
`240`	`239`
`241`	`240`	`// DismissReview dismissing stale review by repo admin`
Original file line number	Diff line number	Diff line change
`@@ -1411,6 +1411,7 @@ func registerRoutes(m *web.Route) {`
`1411`	`1411`
`1412`	`1412`	`if !setting.IsProd {`
`1413`	`1413`	`m.Any("/devtest", devtest.List)`
	`1414`	`+ m.Any("/devtest/fetch-action-test", devtest.FetchActionTest)`
`1414`	`1415`	`m.Any("/devtest/{sub}", devtest.Tmpl)`
`1415`	`1416`	`}`
`1416`	`1417`