mirror of
https://github.com/go-gitea/gitea.git
synced 2026-07-30 12:27:52 +00:00
fix(issue): display error toast on batch action failures instead of reloading page (#38593)
Batch operations in the issues list (labels, milestones, projects, assignees, etc.) could fail silently because `updateIssuesMeta` caught and suppressed fetch errors internally. The caller would then proceed to reload the page, clearing all selected issue checkboxes and giving the impression that the operation had succeeded. - Remove updateIssuesMeta and use our "fetch-action" framework to handle errors and page reloading - Refactor backend code to use ctx.JSONError --------- Signed-off-by: Sudhanshu Singh <sudhanshuwriterblc@gmail.com> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
@@ -11,7 +11,6 @@ import (
|
||||
issues_model "gitea.dev/models/issues"
|
||||
"gitea.dev/models/organization"
|
||||
"gitea.dev/modules/label"
|
||||
"gitea.dev/modules/log"
|
||||
repo_module "gitea.dev/modules/repository"
|
||||
"gitea.dev/modules/templates"
|
||||
"gitea.dev/modules/util"
|
||||
@@ -224,8 +223,7 @@ func UpdateIssueLabel(ctx *context.Context) {
|
||||
}
|
||||
}
|
||||
default:
|
||||
log.Warn("Unrecognized action: %s", action)
|
||||
ctx.HTTPError(http.StatusInternalServerError)
|
||||
ctx.JSONError("invalid action: " + action)
|
||||
return
|
||||
}
|
||||
|
||||
|
||||
@@ -21,7 +21,6 @@ import (
|
||||
user_model "gitea.dev/models/user"
|
||||
issue_indexer "gitea.dev/modules/indexer/issues"
|
||||
db_indexer "gitea.dev/modules/indexer/issues/db"
|
||||
"gitea.dev/modules/log"
|
||||
"gitea.dev/modules/optional"
|
||||
"gitea.dev/modules/setting"
|
||||
"gitea.dev/modules/util"
|
||||
@@ -407,8 +406,7 @@ func UpdateIssueStatus(ctx *context.Context) {
|
||||
|
||||
action := ctx.FormString("action")
|
||||
if action != "open" && action != "close" {
|
||||
log.Warn("Unrecognized action: %s", action)
|
||||
ctx.JSONOK()
|
||||
ctx.JSONError("invalid action: " + action)
|
||||
return
|
||||
}
|
||||
|
||||
@@ -428,9 +426,7 @@ func UpdateIssueStatus(ctx *context.Context) {
|
||||
if action == "close" && !issue.IsClosed {
|
||||
if err := issue_service.CloseIssue(ctx, issue, ctx.Doer, ""); err != nil {
|
||||
if issues_model.IsErrDependenciesLeft(err) {
|
||||
ctx.JSON(http.StatusPreconditionFailed, map[string]any{
|
||||
"error": ctx.Tr("repo.issues.dependency.issue_batch_close_blocked", issue.Index),
|
||||
})
|
||||
ctx.JSONError(ctx.Tr("repo.issues.dependency.issue_batch_close_blocked", issue.Index))
|
||||
return
|
||||
}
|
||||
ctx.ServerError("CloseIssue", err)
|
||||
|
||||
@@ -8,6 +8,7 @@ import {registerGlobalSelectorFunc} from '../modules/observer.ts';
|
||||
import {Idiomorph} from 'idiomorph';
|
||||
import {parseDom} from '../utils.ts';
|
||||
import {html} from '../utils/html.ts';
|
||||
import type {RequestData} from '../types.ts';
|
||||
|
||||
const {appSubUrl, runModeIsProd} = window.config;
|
||||
|
||||
@@ -15,16 +16,16 @@ type FetchActionOpts = {
|
||||
method: string;
|
||||
url: string;
|
||||
headers?: HeadersInit;
|
||||
body?: FormData;
|
||||
data?: RequestData;
|
||||
formSubmitter?: HTMLElement | null;
|
||||
|
||||
// pseudo selectors/commands to update the current page with the response text when the response is text (html)
|
||||
// e.g.: "$this", "$innerHTML", "$closest(tr) td .the-class", "$body #the-id"
|
||||
successSync: string;
|
||||
successSync?: string;
|
||||
|
||||
// the loading indicator element selector, it uses the same syntax as "data-fetch-sync" to find the element(s)
|
||||
// empty means no loading indicator, "$this" means the element itself
|
||||
loadingIndicator: string;
|
||||
loadingIndicator?: string;
|
||||
};
|
||||
|
||||
// fetchActionDoRedirect does real redirection to bypass the browser's limitations of "location"
|
||||
@@ -120,10 +121,10 @@ function buildFetchActionUrl(el: HTMLElement, opt: FetchActionOpts) {
|
||||
return url;
|
||||
}
|
||||
|
||||
async function performActionRequest(el: HTMLElement, opt: FetchActionOpts) {
|
||||
// Use "fetch" to send a request and handle the error cases, return the success response and let caller handle
|
||||
export async function performFetchActionRequest(el: HTMLElement, opt: FetchActionOpts): Promise<Response | null> {
|
||||
const attrIsLoading = 'data-fetch-is-loading';
|
||||
if (el.getAttribute(attrIsLoading)) return;
|
||||
if (!await confirmFetchAction(opt.formSubmitter ?? el)) return;
|
||||
if (el.getAttribute(attrIsLoading)) return null;
|
||||
|
||||
el.setAttribute(attrIsLoading, 'true');
|
||||
toggleLoadingIndicator(el, opt, true);
|
||||
@@ -132,11 +133,8 @@ async function performActionRequest(el: HTMLElement, opt: FetchActionOpts) {
|
||||
const url = buildFetchActionUrl(el, opt);
|
||||
const headers = new Headers(opt.headers);
|
||||
headers.set('X-Gitea-Fetch-Action', '1');
|
||||
const resp = await request(url, {method: opt.method, body: opt.body, headers});
|
||||
if (resp.ok) {
|
||||
await handleFetchActionSuccess(el, opt, resp);
|
||||
return;
|
||||
}
|
||||
const resp = await request(url, {method: opt.method, data: opt.data, headers});
|
||||
if (resp.ok) return resp;
|
||||
await handleFetchActionError(resp);
|
||||
} catch (err) {
|
||||
if (errorName(err) !== 'AbortError') {
|
||||
@@ -147,6 +145,14 @@ async function performActionRequest(el: HTMLElement, opt: FetchActionOpts) {
|
||||
toggleLoadingIndicator(el, opt, false);
|
||||
el.removeAttribute(attrIsLoading);
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
// Use "fetch" to send a request and fully handle its response
|
||||
export async function performFetchAction(el: HTMLElement, opt: FetchActionOpts) {
|
||||
if (!await confirmFetchAction(opt.formSubmitter ?? el)) return;
|
||||
const resp = await performFetchActionRequest(el, opt);
|
||||
if (resp) await handleFetchActionSuccess(el, opt, resp);
|
||||
}
|
||||
|
||||
type SubmitFormFetchActionOpts = {
|
||||
@@ -181,7 +187,7 @@ function prepareFormFetchActionOpts(formEl: HTMLFormElement, opts: SubmitFormFet
|
||||
return {
|
||||
method: formMethodUpper,
|
||||
url: reqUrl,
|
||||
body: reqBody,
|
||||
data: reqBody,
|
||||
formSubmitter: opts.formSubmitter,
|
||||
loadingIndicator: '$this', // for form submit, by default, the loading indicator is the whole form
|
||||
successSync: formEl.getAttribute('data-fetch-sync') ?? '', // by default, no fetch sync for form submit
|
||||
@@ -190,7 +196,7 @@ function prepareFormFetchActionOpts(formEl: HTMLFormElement, opts: SubmitFormFet
|
||||
|
||||
export async function submitFormFetchAction(formEl: HTMLFormElement, opts: SubmitFormFetchActionOpts = {}) {
|
||||
hideToastsAll();
|
||||
await performActionRequest(formEl, prepareFormFetchActionOpts(formEl, opts));
|
||||
await performFetchAction(formEl, prepareFormFetchActionOpts(formEl, opts));
|
||||
}
|
||||
|
||||
async function confirmFetchAction(el: HTMLElement) {
|
||||
@@ -221,7 +227,7 @@ async function confirmFetchAction(el: HTMLElement) {
|
||||
|
||||
async function performLinkFetchAction(el: HTMLElement) {
|
||||
hideToastsAll();
|
||||
await performActionRequest(el, {
|
||||
await performFetchAction(el, {
|
||||
method: el.getAttribute('data-fetch-method') || 'POST', // by default, the method is POST for link-action
|
||||
url: el.getAttribute('data-url')!,
|
||||
loadingIndicator: el.getAttribute('data-fetch-indicator') ?? '$this', // by default, the link-action itself is the loading indicator
|
||||
@@ -237,7 +243,7 @@ export async function performFetchActionTrigger(el: HTMLElement, triggerType: Fe
|
||||
const defaultLoadingIndicator = isUserInitiated ? '$this' : '';
|
||||
|
||||
if (isUserInitiated) hideToastsAll();
|
||||
await performActionRequest(el, {
|
||||
await performFetchAction(el, {
|
||||
method: el.getAttribute('data-fetch-method') || 'GET', // by default, the method is GET for fetch trigger action
|
||||
url: el.getAttribute('data-fetch-url')!,
|
||||
loadingIndicator: el.getAttribute('data-fetch-indicator') ?? defaultLoadingIndicator,
|
||||
|
||||
@@ -146,17 +146,6 @@ export function initRepoCloneButtons() {
|
||||
registerGlobalInitFunc('initRepoCloneButtonsCombo', initRepoCloneButtonsCombo);
|
||||
}
|
||||
|
||||
export async function updateIssuesMeta(url: string, action: string, issue_ids: string, id: string) {
|
||||
try {
|
||||
const response = await POST(url, {data: new URLSearchParams({action, issue_ids, id})});
|
||||
if (!response.ok) {
|
||||
throw new Error('Failed to update issues meta');
|
||||
}
|
||||
} catch (error) {
|
||||
console.error(error);
|
||||
}
|
||||
}
|
||||
|
||||
export function sanitizeRepoName(name: string): string {
|
||||
name = name.trim().replace(/[^-.\w]/g, '-');
|
||||
for (let lastName = ''; lastName !== name;) {
|
||||
|
||||
@@ -1,13 +1,11 @@
|
||||
import {updateIssuesMeta} from './repo-common.ts';
|
||||
import {toggleElem, queryElems, isElemVisible} from '../utils/dom.ts';
|
||||
import {html, htmlRaw} from '../utils/html.ts';
|
||||
import {confirmModal} from './comp/ConfirmModal.ts';
|
||||
import {errorMessage} from '../modules/errors.ts';
|
||||
import {showErrorToast} from '../modules/toast.ts';
|
||||
import {createSortable} from '../modules/sortable.ts';
|
||||
import {DELETE, POST} from '../modules/fetch.ts';
|
||||
import {parseDom} from '../utils.ts';
|
||||
import {fomanticQuery} from '../modules/fomantic/base.ts';
|
||||
import {performFetchAction} from './common-fetch-action.ts';
|
||||
import type {SortableEvent} from 'sortablejs';
|
||||
|
||||
function initRepoIssueListCheckboxes() {
|
||||
@@ -75,14 +73,8 @@ function initRepoIssueListCheckboxes() {
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
await updateIssuesMeta(url, action, issueIDs, elementId);
|
||||
window.location.reload();
|
||||
} catch (err) {
|
||||
// FIXME: this logic (including updateIssuesMeta) is not right, should refactor to our JSONError framework
|
||||
const e = err as {responseJSON?: {error: string}};
|
||||
showErrorToast(e.responseJSON?.error ?? errorMessage(err));
|
||||
}
|
||||
const data = new URLSearchParams({action, issue_ids: issueIDs, id: elementId});
|
||||
await performFetchAction(el, {method: 'post', url, data});
|
||||
},
|
||||
));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user