Check disabled workflow when rerun jobs #26535
+76
−100
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,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 {showTemporaryTooltip} from '../modules/tippy.js'; | ||
| import {confirmModal} from './comp/ConfirmModal.js'; | ||
| import {showErrorToast} from '../modules/toast.js'; | ||
|
|
||
|
|
@@ -64,9 +64,9 @@ export function initGlobalButtonClickOnEnter() { | |
| }); | ||
| } | ||
|
|
||
| // fetchActionDoRedirect does real redirection to bypass the browser's limitations of "location" | ||
| // more details are in the backend's fetch-redirect handler | ||
| function fetchActionDoRedirect(redirect) { | ||
| const form = document.createElement('form'); | ||
| const input = document.createElement('input'); | ||
| form.method = 'post'; | ||
|
|
@@ -79,6 +79,33 @@ function doRedirect(redirect) { | |
| form.submit(); | ||
| } | ||
|
|
||
| async function fetchActionDoRequest(actionElem, url, opt) { | ||
| try { | ||
| const resp = await fetch(url, opt); | ||
| if (resp.status === 200) { | ||
| let {redirect} = await resp.json(); | ||
| redirect = redirect || actionElem.getAttribute('data-redirect'); | ||
| actionElem.classList.remove('dirty'); // remove the areYouSure check before reloading | ||
| if (redirect) { | ||
| fetchActionDoRedirect(redirect); | ||
| } else { | ||
| window.location.reload(); | ||
| } | ||
|
There was a problem hiding this comment. I know this is old code, but shouldn't we use 302 status code and then check resp.redirected and read location from resp.url? I think this is something for a future generic fetch handler. There was a problem hiding this comment. Nope IMO. Fetch's
If you would like to use You could take a try, if it would succeed, feel free to propose a new solution. There was a problem hiding this comment. Yes, that was implying |
||
| } else if (resp.status >= 400 && resp.status < 500) { | ||
| const data = await resp.json(); | ||
| // the code was quite messy, sometimes the backend uses "err", sometimes it uses "error", and even "user_error" | ||
| // but at the moment, as a new approach, we only use "errorMessage" here, backend can use JSONError() to respond. | ||
| await showErrorToast(data.errorMessage || `server error: ${resp.status}`); | ||
| } else { | ||
| await showErrorToast(`server error: ${resp.status}`); | ||
| } | ||
| } catch (e) { | ||
| console.error('error when doRequest', e); | ||
| actionElem.classList.remove('is-loading', 'small-loading-icon'); | ||
| await showErrorToast(i18n.network_error); | ||
| } | ||
| } | ||
|
|
||
| async function formFetchAction(e) { | ||
| if (!e.target.classList.contains('form-fetch-action')) return; | ||
|
|
||
|
|
@@ -115,50 +142,7 @@ async function formFetchAction(e) { | |
| reqOpt.body = formData; | ||
| } | ||
|
|
||
| await fetchActionDoRequest(formEl, reqUrl, reqOpt); | ||
| } | ||
|
|
||
| export function initGlobalCommon() { | ||
|
|
@@ -209,6 +193,7 @@ export function initGlobalCommon() { | |
| $('.tabular.menu .item').tab(); | ||
|
|
||
| document.addEventListener('submit', formFetchAction); | ||
| document.addEventListener('click', linkAction); | ||
| } | ||
|
|
||
| export function initGlobalDropzone() { | ||
|
|
@@ -269,41 +254,29 @@ export function initGlobalDropzone() { | |
| } | ||
|
|
||
| async function linkAction(e) { | ||
| // A "link-action" can post AJAX request to its "data-url" | ||
| // Then the browser is redirected to: the "redirect" in response, or "data-redirect" attribute, or current URL by reloading. | ||
| // If the "link-action" has "data-modal-confirm" attribute, a confirm modal dialog will be shown before taking action. | ||
| const el = e.target.closest('.link-action'); | ||
| if (!el) return; | ||
|
|
||
| e.preventDefault(); | ||
| const url = el.getAttribute('data-url'); | ||
| const doRequest = async () => { | ||
| el.disabled = true; | ||
| await fetchActionDoRequest(el, url, {method: 'POST', headers: {'X-Csrf-Token': csrfToken}}); | ||
| el.disabled = false; | ||
| }; | ||
|
|
||
| const modalConfirmContent = htmlEscape(el.getAttribute('data-modal-confirm') || ''); | ||
| if (!modalConfirmContent) { | ||
| await doRequest(); | ||
| return; | ||
| } | ||
|
|
||
| const isRisky = el.classList.contains('red') || el.classList.contains('yellow') || el.classList.contains('orange') || el.classList.contains('negative'); | ||
| if (await confirmModal({content: modalConfirmContent, buttonColor: isRisky ? 'orange' : 'green'})) { | ||
| await doRequest(); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -354,7 +327,6 @@ export function initGlobalLinkActions() { | |
|
|
||
| // Helpers. | ||
| $('.delete-button').on('click', showDeletePopup); | ||
| } | ||
|
|
||
| function initGlobalShowModal() { | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wxiaoguang could you port the Proxy implementation from #25940 here instead of extending this hack further? I think #25940 is not going to land anytime soon, so it's a good extract.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I have different opinion. I think this "array" is a right approach. The reasons are:
ga.push(), the same method.So, I would always prefer this approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does your hack even run in JS strict mode? Try adding
"use strict";to the file 😆.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not hack IMO, it strictly follows JS standard. And, why not run? I don't see any problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess you are right and the problem I've been seeing may be because of that missing init. Still I personally prefer the Proxy method.
BTW, I would like our code to run in strict mode, but don't feel like littering every file with a "use strict". Maybe there are solutions for webpack to automatically inject it into every chunk like https://babeljs.io/docs/babel-plugin-transform-strict-mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to evanw/esbuild#422 (comment) we could use the esbuild banner feature for it.