From 6a9918836634a0d2b8329fdc78580f0bb468268e Mon Sep 17 00:00:00 2001 From: Ioannis Bonatakis Date: Wed, 23 Oct 2024 13:10:56 +0200 Subject: [PATCH] Reuse restartJob in order of restart jobs in Overview page Refactor restartJob in order to make sure that is able to handle request for multiple restarts. This follows up with some changes to the form and the javascript part of the overview.html.ep, which triggers the restart with the use of restartJob, as opposed of reimplement the logic. The advantage is that we have one common behavior and error handling on the restart operation. - An optional comment can be passed not to restartJob, which is included to the request. - restartJob handles both single and multible job restarts. - The form is designed to handle each occasion in separate functions making the code becomes more modular and easier to maintain. - The overvies's restartJobs function will restart only jobs with valid id. https://progress.opensuse.org/issues/166559 Signed-off-by: Ioannis Bonatakis --- assets/javascripts/openqa.js | 31 +++++++++++--- assets/javascripts/overview.js | 58 ++++++++++++++------------ assets/stylesheets/overview.scss | 10 ++++- t/ui/10-tests_overview.t | 6 +-- templates/webapi/test/overview.html.ep | 45 ++++++++++++-------- 5 files changed, 94 insertions(+), 56 deletions(-) diff --git a/assets/javascripts/openqa.js b/assets/javascripts/openqa.js index 6021400ad1c..b08df323368 100644 --- a/assets/javascripts/openqa.js +++ b/assets/javascripts/openqa.js @@ -238,7 +238,12 @@ function forceJobRestartViaRestartLink(restartLink) { restartLink.click(); } -function restartJob(ajaxUrl, jobId) { +function restartJob(ajaxUrl, jobIds, comment) { + let singleJobId; + if (!Array.isArray(jobIds)) { + singleJobId = jobIds; + jobIds = [jobIds]; + } var showError = function (reason) { var errorMessage = 'Unable to restart job'; if (reason) { @@ -248,8 +253,8 @@ function restartJob(ajaxUrl, jobId) { } addFlash('danger', errorMessage); }; - - return fetchWithCSRF(ajaxUrl, {method: 'POST'}) + const body = {jobs: jobIds, comment: comment}; + return fetchWithCSRF(ajaxUrl, {method: 'POST', body: JSON.stringify(body)}) .then(response => { if (!response.ok) throw `Server returned ${response.status}: ${response.statusText}`; return response.json(); @@ -257,7 +262,14 @@ function restartJob(ajaxUrl, jobId) { .then(responseJSON => { var newJobUrl; try { - newJobUrl = responseJSON.test_url[0][jobId]; + if (singleJobId) { + newJobUrl = responseJSON.test_url[0][singleJobId]; + } else { + const testUrlData = responseJSON.test_url; + if (Array.isArray(testUrlData)) { + newJobUrl = testUrlData.map(item => Object.values(item)[0]); + } + } } catch { // Intentionally ignore all errors } @@ -265,13 +277,20 @@ function restartJob(ajaxUrl, jobId) { showJobRestartResults( responseJSON, newJobUrl, - restartJob.bind(undefined, addParam(ajaxUrl, 'force', '1'), jobId) + restartJob.bind(undefined, addParam(ajaxUrl, 'force', '1'), jobIds, comment) ) ) { return; } if (newJobUrl) { - window.location.replace(newJobUrl); + if (Array.isArray(newJobUrl)) { + addFlash('info', 'Overview will be reloaded'); + setTimeout(() => { + window.location.reload(); + }, 2000); + } else { + window.location.replace(newJobUrl); + } } else { throw 'URL for new job not available'; } diff --git a/assets/javascripts/overview.js b/assets/javascripts/overview.js index 8f1d5322aa1..9b04b238a76 100644 --- a/assets/javascripts/overview.js +++ b/assets/javascripts/overview.js @@ -287,16 +287,36 @@ function showAddCommentsDialog() { modal.show(); } -function restartOrCommentJobs(form) { +function restartJobsWithComment(btn) { + const form = btn.form; const text = form.elements.text.value; + const jobs = btn.dataset.jobs + .split(',') + .map(Number) + .filter(n => !isNaN(n)); + const apiUrl = btn.dataset.url; if (!text.length) { return window.alert("The comment text mustn't be empty."); } - const actionBtn = form.clickedButton ? form.clickedButton.value : null; - console.log(actionBtn); - let reqUrl = form.clickedButton.getAttribute('formaction'); - console.log(form.clickedButton.getAttribute('formaction')); - const progressIndication = document.getElementById('add-comments-progress-indication'); + + const progressIndication = document.getElementById('add-comments-progress-indication'); + const controls = document.getElementById('add-comments-controls'); + progressIndication.style.display = 'flex'; + controls.style.display = 'none'; + restartJob(apiUrl, jobs, text); + progressIndication.style.display = 'none'; + controls.style.display = 'inline'; + window.addCommentsModal.hide(); +} + +function addComments(btn) { + const form = btn.form; + const text = form.elements.text.value; + const apiUrl = btn.dataset.url; + if (!text.length) { + return window.alert("The comment text mustn't be empty."); + } + const progressIndication = document.getElementById('add-comments-progress-indication'); const controls = document.getElementById('add-comments-controls'); progressIndication.style.display = 'flex'; controls.style.display = 'none'; @@ -305,31 +325,17 @@ function restartOrCommentJobs(form) { controls.style.display = 'inline'; window.addCommentsModal.hide(); }; - - let infoText = - 'The comments have been created. Reload the page to show changes.'; - let errText = 'The comments could not be added:'; - if (actionBtn === 'restartAndCommentJobs') { - infoText = 'Reload the page to show restarted jobs.'; - errText = 'Failed to restart jobs: '; - } - fetchWithCSRF(reqUrl, {method: 'POST', body: new FormData(form)}) + fetchWithCSRF(apiUrl, {method: 'POST', body: new FormData(form)}) .then(response => { if (!response.ok) throw `Server returned ${response.status}: ${response.statusText}`; - addFlash('info', infoText); + addFlash( + 'info', + 'The comments have been created. Reload the page to show changes.' + ); done(); - return response.json(); - }) - .then(resData => { - console.log(resData); - if (resData.errors && resData.errors.length > 0) { - for (let i in resData.errors) { - addFlash('warning', 'Warning: Errors found in Response:\n' + resData.errors[i].trim()); - } - } }) .catch(error => { - addFlash('danger', `${errText} ${error}`); + addFlash('danger', `The comments could not be added: ${error}`); done(); }); } diff --git a/assets/stylesheets/overview.scss b/assets/stylesheets/overview.scss index 8083fc1f15b..527b7fe250e 100644 --- a/assets/stylesheets/overview.scss +++ b/assets/stylesheets/overview.scss @@ -94,4 +94,12 @@ #add-comments-progress-indication { display: none; width: 100%; -} \ No newline at end of file +} + +.group-comment-icon-stack { + font-size: 0.9em; +} + +.group-comment-icon-stack .fa-undo { + color: #0c4374; /* Your specified color */ +} diff --git a/t/ui/10-tests_overview.t b/t/ui/10-tests_overview.t index 9ba9199e105..4d07473dfb3 100644 --- a/t/ui/10-tests_overview.t +++ b/t/ui/10-tests_overview.t @@ -156,7 +156,6 @@ like($driver->find_elements('.failedmodule', 'css')->[1]->get_attribute('href'), my @descriptions = $driver->find_elements('td.name a', 'css'); is(scalar @descriptions, 2, 'only test suites with description content are shown as links'); -disable_bootstrap_animations; $descriptions[0]->click(); is($driver->find_element('.popover-header')->get_text, 'kde', 'description popover shows content'); @@ -554,7 +553,6 @@ subtest "job template names displayed on 'Test result overview' page" => sub { my @descriptions = $driver->find_elements('td.name a', 'css'); is(scalar @descriptions, 2, 'only test suites with description content are shown as links'); - disable_bootstrap_animations; $descriptions[0]->click(); is(wait_for_element(selector => '.popover-header')->get_text, 'kde_variant', 'description popover shows content'); }; @@ -565,7 +563,6 @@ subtest 'add comments' => sub { $driver->find_element_by_link_text('Login')->click; $driver->get('/tests/overview?state=done&result=failed'); - disable_bootstrap_animations; $driver->find_element('button[title="Restart or comment jobs"]')->click; my $comment_text = 'comment via add-comments'; my $submit_button = $driver->find_element('#add-comments-controls button[id="commentJobsBtn"]'); @@ -584,7 +581,6 @@ subtest 'add comments' => sub { subtest 'restart jobs with comment' => sub { $driver->get('/tests/overview?state=done&result=failed'); - disable_bootstrap_animations; $driver->find_element('button[title="Restart or comment jobs"]')->click; my $comment_text = 'comment current jobs and restart'; my $submit_button = $driver->find_element('#add-comments-controls button[id="restartAndCommentJobsBtn"]'); @@ -592,7 +588,7 @@ subtest 'add comments' => sub { is $submit_button->get_text, 'Restart and comment on 2 jobs', 'submit button displayed with number of jobs'; $submit_button->click; wait_for_ajax msg => 'comments created'; - like $driver->find_element_by_id('flash-messages')->get_text, qr/Reload the page to show restarted jobs/, + like $driver->find_element_by_id('flash-messages')->get_text, qr/Overview will be reloaded/, 'info about successful restart shown'; my @failed_job_ids = map { $_->id } $jobs->search({result => FAILED})->all; is $comments->search({job_id => {-not_in => \@failed_job_ids}, text => $comment_text})->count, 0, diff --git a/templates/webapi/test/overview.html.ep b/templates/webapi/test/overview.html.ep index b0a9e10f672..289bf6ddcbd 100644 --- a/templates/webapi/test/overview.html.ep +++ b/templates/webapi/test/overview.html.ep @@ -44,9 +44,9 @@ % if ($allow_commenting) { % } @@ -221,7 +221,7 @@