From 3d559325e906bae6a03d7743257e1685f0ede6fc Mon Sep 17 00:00:00 2001 From: silverwind Date: Mon, 27 May 2024 22:12:12 +0200 Subject: [PATCH 01/11] Fix actions fetching logic and loading state, prevent duplicate toasts --- web_src/js/components/RepoActionView.vue | 78 +++++++++++++++--------- web_src/js/modules/toast.js | 10 ++- 2 files changed, 57 insertions(+), 31 deletions(-) diff --git a/web_src/js/components/RepoActionView.vue b/web_src/js/components/RepoActionView.vue index 8b39d0504b5d4..4ac4998a12319 100644 --- a/web_src/js/components/RepoActionView.vue +++ b/web_src/js/components/RepoActionView.vue @@ -6,6 +6,7 @@ import {toggleElem} from '../utils/dom.js'; import {formatDatetime} from '../utils/time.js'; import {renderAnsi} from '../render/ansi.js'; import {GET, POST, DELETE} from '../modules/fetch.js'; +import {showErrorToast} from '../modules/toast.js'; const sfc = { name: 'RepoActionView', @@ -87,9 +88,9 @@ const sfc = { async mounted() { // load job data and then auto-reload periodically - // need to await first loadJob so this.currentJobStepsStates is initialized and can be used in hashChangeListener - await this.loadJob(); - this.intervalID = setInterval(this.loadJob, 1000); + // need to await first loadData so this.currentJobStepsStates is initialized and can be used in hashChangeListener + await this.loadData(); + this.intervalID = setInterval(this.loadData, 1000); document.body.addEventListener('click', this.closeDropdown); this.hashChangeListener(); window.addEventListener('hashchange', this.hashChangeListener); @@ -142,7 +143,10 @@ const sfc = { toggleStepLogs(idx) { this.currentJobStepsStates[idx].expanded = !this.currentJobStepsStates[idx].expanded; if (this.currentJobStepsStates[idx].expanded) { - this.loadJob(); // try to load the data immediately instead of waiting for next timer interval + this.currentJobStepsStates[idx].loading = true; + // force-load the data, otherwise the state will end up incorrect if loadData + // is already running and the job step will never expand. + this.loadData(true); } }, // cancel a run @@ -206,7 +210,7 @@ const sfc = { async deleteArtifact(name) { if (!window.confirm(this.locale.confirmDeleteArtifact.replace('%s', name))) return; await DELETE(`${this.run.link}/artifacts/${name}`); - await this.loadJob(); + await this.loadData(); }, async fetchJob() { @@ -222,8 +226,8 @@ const sfc = { return await resp.json(); }, - async loadJob() { - if (this.loading) return; + async loadData(force = false) { + if (this.loading && !force) return; try { this.loading = true; @@ -235,32 +239,46 @@ const sfc = { ]); } catch (err) { if (err instanceof TypeError) return; // avoid network error while unloading page - throw err; + showErrorToast(err.message); + // reset all loading states, we can't easily tell which one failed at this point + for (let i = 0; i < this.currentJob.steps.length; i++) { + if (this.currentJobStepsStates[i].loading) { + this.currentJobStepsStates[i].loading = false; + } + } } - this.artifacts = artifacts['artifacts'] || []; - - // save the state to Vue data, then the UI will be updated - this.run = job.state.run; - this.currentJob = job.state.currentJob; + if (artifacts) { + this.artifacts = artifacts['artifacts'] || []; + } - // sync the currentJobStepsStates to store the job step states - for (let i = 0; i < this.currentJob.steps.length; i++) { - if (!this.currentJobStepsStates[i]) { - // initial states for job steps - this.currentJobStepsStates[i] = {cursor: null, expanded: false}; + if (job) { + // save the state to Vue data, then the UI will be updated + this.run = job.state.run; + this.currentJob = job.state.currentJob; + + // sync the currentJobStepsStates to store the job step states + for (let i = 0; i < this.currentJob.steps.length; i++) { + if (!this.currentJobStepsStates[i]) { + // initial states for job steps + this.currentJobStepsStates[i] = {cursor: null, expanded: false, loading: false}; + } + } + for (const logs of job.logs.stepsLog) { + // save the cursor, it will be passed to backend next time + this.currentJobStepsStates[logs.step].cursor = logs.cursor; + // append logs to the UI + this.appendLogs(logs.step, logs.lines, logs.started); + // update loading state + if (this.currentJobStepsStates[logs.step].loading && logs.cursor) { + this.currentJobStepsStates[logs.step].loading = false; + } } - } - // append logs to the UI - for (const logs of job.logs.stepsLog) { - // save the cursor, it will be passed to backend next time - this.currentJobStepsStates[logs.step].cursor = logs.cursor; - this.appendLogs(logs.step, logs.lines, logs.started); - } - if (this.run.done && this.intervalID) { - clearInterval(this.intervalID); - this.intervalID = null; + if (this.run.done && this.intervalID) { + clearInterval(this.intervalID); + this.intervalID = null; + } } } finally { this.loading = false; @@ -313,7 +331,7 @@ const sfc = { this.currentJobStepsStates[step].expanded = true; // need to await for load job if the step log is loaded for the first time // so logline can be selected by querySelector - await this.loadJob(); + await this.loadData(); } const logLine = this.$refs.steps.querySelector(selectedLogStep); if (!logLine) return; @@ -479,7 +497,7 @@ export function initRepositoryActionView() { - + diff --git a/web_src/js/modules/toast.js b/web_src/js/modules/toast.js index d12d203718bae..5b97fe4c206d5 100644 --- a/web_src/js/modules/toast.js +++ b/web_src/js/modules/toast.js @@ -21,7 +21,13 @@ const levels = { }; // See https://github.com/apvarun/toastify-js#api for options -function showToast(message, level, {gravity, position, duration, useHtmlBody, ...other} = {}) { +function showToast(message, level, {gravity, position, duration, useHtmlBody, preventDuplicates = true, ...other} = {}) { + // prevent showing duplicate toasts with same level and message + if (preventDuplicates) { + const el = document.querySelector(`.toastify[data-message="${CSS.escape(message)}"]`); + if (el?.matches(`.toastify[data-level="${CSS.escape(level)}"]`)) return; + } + const {icon, background, duration: levelDuration} = levels[level ?? 'info']; const toast = Toastify({ text: ` @@ -38,6 +44,8 @@ function showToast(message, level, {gravity, position, duration, useHtmlBody, .. }); toast.showToast(); + toast.toastElement.setAttribute('data-message', message); + toast.toastElement.setAttribute('data-level', level); toast.toastElement.querySelector('.toast-close').addEventListener('click', () => toast.hideToast()); return toast; } From 77e6bec03a0dc2c0e83dcac3899b9f135244265a Mon Sep 17 00:00:00 2001 From: silverwind Date: Mon, 27 May 2024 22:31:12 +0200 Subject: [PATCH 02/11] add body variable --- web_src/js/modules/toast.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/web_src/js/modules/toast.js b/web_src/js/modules/toast.js index 5b97fe4c206d5..ab761b7ceb788 100644 --- a/web_src/js/modules/toast.js +++ b/web_src/js/modules/toast.js @@ -22,9 +22,11 @@ const levels = { // See https://github.com/apvarun/toastify-js#api for options function showToast(message, level, {gravity, position, duration, useHtmlBody, preventDuplicates = true, ...other} = {}) { + const body = String(useHtmlBody ? message : htmlEscape(message)); + // prevent showing duplicate toasts with same level and message if (preventDuplicates) { - const el = document.querySelector(`.toastify[data-message="${CSS.escape(message)}"]`); + const el = document.querySelector(`.toastify[data-body="${CSS.escape(body)}"]`); if (el?.matches(`.toastify[data-level="${CSS.escape(level)}"]`)) return; } @@ -32,7 +34,7 @@ function showToast(message, level, {gravity, position, duration, useHtmlBody, pr const toast = Toastify({ text: `
${svg(icon)}
-
${useHtmlBody ? message : htmlEscape(message)}
+
${body}
`, escapeMarkup: false, @@ -44,7 +46,7 @@ function showToast(message, level, {gravity, position, duration, useHtmlBody, pr }); toast.showToast(); - toast.toastElement.setAttribute('data-message', message); + toast.toastElement.setAttribute('data-body', body); toast.toastElement.setAttribute('data-level', level); toast.toastElement.querySelector('.toast-close').addEventListener('click', () => toast.hideToast()); return toast; From 60e424df869cd7cee6b8e81e3df80b1eec6cab5e Mon Sep 17 00:00:00 2001 From: silverwind Date: Mon, 27 May 2024 22:40:57 +0200 Subject: [PATCH 03/11] Update web_src/js/components/RepoActionView.vue --- web_src/js/components/RepoActionView.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web_src/js/components/RepoActionView.vue b/web_src/js/components/RepoActionView.vue index 4ac4998a12319..181e101aedc86 100644 --- a/web_src/js/components/RepoActionView.vue +++ b/web_src/js/components/RepoActionView.vue @@ -240,7 +240,7 @@ const sfc = { } catch (err) { if (err instanceof TypeError) return; // avoid network error while unloading page showErrorToast(err.message); - // reset all loading states, we can't easily tell which one failed at this point + // reset all step loading states, we can't easily tell which one failed at this point for (let i = 0; i < this.currentJob.steps.length; i++) { if (this.currentJobStepsStates[i].loading) { this.currentJobStepsStates[i].loading = false; From a8695fe99ddd1ba0a515395bca77259ecfee8c38 Mon Sep 17 00:00:00 2001 From: silverwind Date: Tue, 25 Jun 2024 21:03:39 +0200 Subject: [PATCH 04/11] Update web_src/js/modules/toast.js Co-authored-by: delvh --- web_src/js/modules/toast.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/web_src/js/modules/toast.js b/web_src/js/modules/toast.js index ab761b7ceb788..2a8daeead9e3d 100644 --- a/web_src/js/modules/toast.js +++ b/web_src/js/modules/toast.js @@ -26,8 +26,7 @@ function showToast(message, level, {gravity, position, duration, useHtmlBody, pr // prevent showing duplicate toasts with same level and message if (preventDuplicates) { - const el = document.querySelector(`.toastify[data-body="${CSS.escape(body)}"]`); - if (el?.matches(`.toastify[data-level="${CSS.escape(level)}"]`)) return; + if (document.querySelector(`.toastify[data-body="${CSS.escape(body)}"][data-level="${CSS.escape(level)}"]`)) return; } const {icon, background, duration: levelDuration} = levels[level ?? 'info']; From f88e8178ff9c4277fcb75692797c4be85a602109 Mon Sep 17 00:00:00 2001 From: silverwind Date: Tue, 25 Jun 2024 21:05:47 +0200 Subject: [PATCH 05/11] remove boolean trap --- web_src/js/components/RepoActionView.vue | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/web_src/js/components/RepoActionView.vue b/web_src/js/components/RepoActionView.vue index bd097b2284c4b..49ba68cb94593 100644 --- a/web_src/js/components/RepoActionView.vue +++ b/web_src/js/components/RepoActionView.vue @@ -90,7 +90,9 @@ const sfc = { // load job data and then auto-reload periodically // need to await first loadData so this.currentJobStepsStates is initialized and can be used in hashChangeListener await this.loadData(); - this.intervalID = setInterval(this.loadData, 1000); + this.intervalID = setInterval(() => { + this.loadData(); + }, 1000); document.body.addEventListener('click', this.closeDropdown); this.hashChangeListener(); window.addEventListener('hashchange', this.hashChangeListener); @@ -146,7 +148,7 @@ const sfc = { this.currentJobStepsStates[idx].loading = true; // force-load the data, otherwise the state will end up incorrect if loadData // is already running and the job step will never expand. - this.loadData(true); + this.loadData({force: true}); } }, // cancel a run @@ -226,7 +228,7 @@ const sfc = { return await resp.json(); }, - async loadData(force = false) { + async loadData({force = false} = {}) { if (this.loading && !force) return; try { this.loading = true; From 77c4aa20cead6aed48554706487af073881e80fc Mon Sep 17 00:00:00 2001 From: silverwind Date: Tue, 25 Jun 2024 21:28:43 +0200 Subject: [PATCH 06/11] fix lint --- web_src/js/modules/toast.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web_src/js/modules/toast.js b/web_src/js/modules/toast.js index 2a8daeead9e3d..985b38a3e0091 100644 --- a/web_src/js/modules/toast.js +++ b/web_src/js/modules/toast.js @@ -25,8 +25,8 @@ function showToast(message, level, {gravity, position, duration, useHtmlBody, pr const body = String(useHtmlBody ? message : htmlEscape(message)); // prevent showing duplicate toasts with same level and message - if (preventDuplicates) { - if (document.querySelector(`.toastify[data-body="${CSS.escape(body)}"][data-level="${CSS.escape(level)}"]`)) return; + if (preventDuplicates && document.querySelector(`.toastify[data-body="${CSS.escape(body)}"][data-level="${CSS.escape(level)}"]`)) { + return; } const {icon, background, duration: levelDuration} = levels[level ?? 'info']; From f654f1210dea656101e8ec4ad5c06f6d08681e29 Mon Sep 17 00:00:00 2001 From: silverwind Date: Tue, 25 Jun 2024 21:31:21 +0200 Subject: [PATCH 07/11] invert option --- web_src/js/modules/toast.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web_src/js/modules/toast.js b/web_src/js/modules/toast.js index 985b38a3e0091..2e9e51e65b44a 100644 --- a/web_src/js/modules/toast.js +++ b/web_src/js/modules/toast.js @@ -21,11 +21,11 @@ const levels = { }; // See https://github.com/apvarun/toastify-js#api for options -function showToast(message, level, {gravity, position, duration, useHtmlBody, preventDuplicates = true, ...other} = {}) { +function showToast(message, level, {gravity, position, duration, useHtmlBody, allowDuplicates, ...other} = {}) { const body = String(useHtmlBody ? message : htmlEscape(message)); // prevent showing duplicate toasts with same level and message - if (preventDuplicates && document.querySelector(`.toastify[data-body="${CSS.escape(body)}"][data-level="${CSS.escape(level)}"]`)) { + if (!allowDuplicates && document.querySelector(`.toastify[data-body="${CSS.escape(body)}"][data-level="${CSS.escape(level)}"]`)) { return; } From abfbb59e5608ffa6b974794dbb4ee4cdcb5790d9 Mon Sep 17 00:00:00 2001 From: silverwind Date: Tue, 25 Jun 2024 21:42:31 +0200 Subject: [PATCH 08/11] default preventDuplicates to default, use single key, use it for htmx --- web_src/js/components/RepoActionView.vue | 2 +- web_src/js/htmx.js | 8 ++++++-- web_src/js/modules/toast.js | 8 ++++---- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/web_src/js/components/RepoActionView.vue b/web_src/js/components/RepoActionView.vue index 49ba68cb94593..985d3b2642617 100644 --- a/web_src/js/components/RepoActionView.vue +++ b/web_src/js/components/RepoActionView.vue @@ -241,7 +241,7 @@ const sfc = { ]); } catch (err) { if (err instanceof TypeError) return; // avoid network error while unloading page - showErrorToast(err.message); + showErrorToast(err.message, {preventDuplicates: true}); // reset all step loading states, we can't easily tell which one failed at this point for (let i = 0; i < this.currentJob.steps.length; i++) { if (this.currentJobStepsStates[i].loading) { diff --git a/web_src/js/htmx.js b/web_src/js/htmx.js index 6169d2f82f291..91d6f7b7593c1 100644 --- a/web_src/js/htmx.js +++ b/web_src/js/htmx.js @@ -10,11 +10,15 @@ htmx.config.scrollIntoViewOnBoost = false; // https://htmx.org/events/#htmx:sendError document.body.addEventListener('htmx:sendError', (event) => { // TODO: add translations - showErrorToast(`Network error when calling ${event.detail.requestConfig.path}`); + showErrorToast(`Network error when calling ${event.detail.requestConfig.path}`, { + preventDuplicates: true, + }); }); // https://htmx.org/events/#htmx:responseError document.body.addEventListener('htmx:responseError', (event) => { // TODO: add translations - showErrorToast(`Error ${event.detail.xhr.status} when calling ${event.detail.requestConfig.path}`); + showErrorToast(`Error ${event.detail.xhr.status} when calling ${event.detail.requestConfig.path}`, { + preventDuplicates: true, + }); }); diff --git a/web_src/js/modules/toast.js b/web_src/js/modules/toast.js index 2e9e51e65b44a..9b288fde3dfbb 100644 --- a/web_src/js/modules/toast.js +++ b/web_src/js/modules/toast.js @@ -21,11 +21,12 @@ const levels = { }; // See https://github.com/apvarun/toastify-js#api for options -function showToast(message, level, {gravity, position, duration, useHtmlBody, allowDuplicates, ...other} = {}) { +function showToast(message, level, {gravity, position, duration, useHtmlBody, preventDuplicates, ...other} = {}) { const body = String(useHtmlBody ? message : htmlEscape(message)); + const key = `${level}-${body}`; // prevent showing duplicate toasts with same level and message - if (!allowDuplicates && document.querySelector(`.toastify[data-body="${CSS.escape(body)}"][data-level="${CSS.escape(level)}"]`)) { + if (preventDuplicates && document.querySelector(`.toastify[data-key="${CSS.escape(key)}"]`)) { return; } @@ -45,8 +46,7 @@ function showToast(message, level, {gravity, position, duration, useHtmlBody, al }); toast.showToast(); - toast.toastElement.setAttribute('data-body', body); - toast.toastElement.setAttribute('data-level', level); + toast.toastElement.setAttribute('data-key', key); toast.toastElement.querySelector('.toast-close').addEventListener('click', () => toast.hideToast()); return toast; } From a0c9cf921949235916475f84384f258d07dadb2f Mon Sep 17 00:00:00 2001 From: silverwind Date: Tue, 25 Jun 2024 21:54:11 +0200 Subject: [PATCH 09/11] restrict htmx preventDuplicates to only polling elements --- web_src/js/htmx.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/web_src/js/htmx.js b/web_src/js/htmx.js index 91d6f7b7593c1..798f674eacd00 100644 --- a/web_src/js/htmx.js +++ b/web_src/js/htmx.js @@ -7,11 +7,15 @@ import 'idiomorph/dist/idiomorph-ext.js'; htmx.config.requestClass = 'is-loading'; htmx.config.scrollIntoViewOnBoost = false; +function isPollingElement(el) { + return Boolean(el.getAttribute('hx-trigger')?.startsWith('every ')); +} + // https://htmx.org/events/#htmx:sendError document.body.addEventListener('htmx:sendError', (event) => { // TODO: add translations showErrorToast(`Network error when calling ${event.detail.requestConfig.path}`, { - preventDuplicates: true, + preventDuplicates: isPollingElement(event.target), }); }); @@ -19,6 +23,6 @@ document.body.addEventListener('htmx:sendError', (event) => { document.body.addEventListener('htmx:responseError', (event) => { // TODO: add translations showErrorToast(`Error ${event.detail.xhr.status} when calling ${event.detail.requestConfig.path}`, { - preventDuplicates: true, + preventDuplicates: isPollingElement(event.target), }); }); From 181ffa06d90fcadaae655f99d189f22de40e1e4f Mon Sep 17 00:00:00 2001 From: silverwind Date: Wed, 26 Jun 2024 16:35:24 +0200 Subject: [PATCH 10/11] revert toast-related changes --- web_src/js/htmx.js | 12 ++---------- web_src/js/modules/toast.js | 13 ++----------- 2 files changed, 4 insertions(+), 21 deletions(-) diff --git a/web_src/js/htmx.js b/web_src/js/htmx.js index 798f674eacd00..6169d2f82f291 100644 --- a/web_src/js/htmx.js +++ b/web_src/js/htmx.js @@ -7,22 +7,14 @@ import 'idiomorph/dist/idiomorph-ext.js'; htmx.config.requestClass = 'is-loading'; htmx.config.scrollIntoViewOnBoost = false; -function isPollingElement(el) { - return Boolean(el.getAttribute('hx-trigger')?.startsWith('every ')); -} - // https://htmx.org/events/#htmx:sendError document.body.addEventListener('htmx:sendError', (event) => { // TODO: add translations - showErrorToast(`Network error when calling ${event.detail.requestConfig.path}`, { - preventDuplicates: isPollingElement(event.target), - }); + showErrorToast(`Network error when calling ${event.detail.requestConfig.path}`); }); // https://htmx.org/events/#htmx:responseError document.body.addEventListener('htmx:responseError', (event) => { // TODO: add translations - showErrorToast(`Error ${event.detail.xhr.status} when calling ${event.detail.requestConfig.path}`, { - preventDuplicates: isPollingElement(event.target), - }); + showErrorToast(`Error ${event.detail.xhr.status} when calling ${event.detail.requestConfig.path}`); }); diff --git a/web_src/js/modules/toast.js b/web_src/js/modules/toast.js index 9b288fde3dfbb..d12d203718bae 100644 --- a/web_src/js/modules/toast.js +++ b/web_src/js/modules/toast.js @@ -21,20 +21,12 @@ const levels = { }; // See https://github.com/apvarun/toastify-js#api for options -function showToast(message, level, {gravity, position, duration, useHtmlBody, preventDuplicates, ...other} = {}) { - const body = String(useHtmlBody ? message : htmlEscape(message)); - const key = `${level}-${body}`; - - // prevent showing duplicate toasts with same level and message - if (preventDuplicates && document.querySelector(`.toastify[data-key="${CSS.escape(key)}"]`)) { - return; - } - +function showToast(message, level, {gravity, position, duration, useHtmlBody, ...other} = {}) { const {icon, background, duration: levelDuration} = levels[level ?? 'info']; const toast = Toastify({ text: `
${svg(icon)}
-
${body}
+
${useHtmlBody ? message : htmlEscape(message)}
`, escapeMarkup: false, @@ -46,7 +38,6 @@ function showToast(message, level, {gravity, position, duration, useHtmlBody, pr }); toast.showToast(); - toast.toastElement.setAttribute('data-key', key); toast.toastElement.querySelector('.toast-close').addEventListener('click', () => toast.hideToast()); return toast; } From 18b02851bed3bd0750523ef46cffb625f4b485c8 Mon Sep 17 00:00:00 2001 From: silverwind Date: Wed, 26 Jun 2024 16:50:00 +0200 Subject: [PATCH 11/11] Update web_src/js/components/RepoActionView.vue --- web_src/js/components/RepoActionView.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web_src/js/components/RepoActionView.vue b/web_src/js/components/RepoActionView.vue index 985d3b2642617..49ba68cb94593 100644 --- a/web_src/js/components/RepoActionView.vue +++ b/web_src/js/components/RepoActionView.vue @@ -241,7 +241,7 @@ const sfc = { ]); } catch (err) { if (err instanceof TypeError) return; // avoid network error while unloading page - showErrorToast(err.message, {preventDuplicates: true}); + showErrorToast(err.message); // reset all step loading states, we can't easily tell which one failed at this point for (let i = 0; i < this.currentJob.steps.length; i++) { if (this.currentJobStepsStates[i].loading) {