From 2346d98db68f833a47499226b97ad34e6f8d9a4b 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 | 32 +++++++++--- assets/javascripts/overview.js | 58 +++++++++++---------- assets/stylesheets/overview.scss | 10 +++- t/ui/10-tests_overview.t | 12 ++--- templates/webapi/test/overview.html.ep | 72 +++++++++++++------------- 5 files changed, 107 insertions(+), 77 deletions(-) diff --git a/assets/javascripts/openqa.js b/assets/javascripts/openqa.js index 6021400ad1c..58c60e9292c 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,9 @@ function restartJob(ajaxUrl, jobId) { } addFlash('danger', errorMessage); }; - - return fetchWithCSRF(ajaxUrl, {method: 'POST'}) + const body = new FormData(); + body.append('comment', comment); + return fetchWithCSRF(ajaxUrl, {method: 'POST', body: body}) .then(response => { if (!response.ok) throw `Server returned ${response.status}: ${response.statusText}`; return response.json(); @@ -257,7 +263,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 +278,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', + 'The jobs have been restarted. Reload the page to show changes.' + ); + } 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..6c05abf64f4 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; +} diff --git a/t/ui/10-tests_overview.t b/t/ui/10-tests_overview.t index 9ba9199e105..cec108b758d 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'); @@ -546,7 +545,7 @@ subtest 'filter by result and state' => sub { subtest "job template names displayed on 'Test result overview' page" => sub { $driver->get('/group_overview/1002'); is($driver->find_element('.progress-bar-unfinished')->get_text(), - '1 unfinished', 'The number of unfinished jobs is right'); + '1 unfinished', 'expected number of unfinished jobs'); $driver->get('/tests/overview?distri=opensuse&version=13.1&build=0091&groupid=1002'); my @tds = $driver->find_elements('#results_DVD tbody tr .name'); @@ -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,17 +581,18 @@ 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"]'); + my $submit_button = $driver->find_element('#restartAndCommentJobsBtn'); $driver->find_element_by_id('text')->send_keys($comment_text); 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/Reload the page to show changes/, 'info about successful restart shown'; my @failed_job_ids = map { $_->id } $jobs->search({result => FAILED})->all; + is $comments->search({job_id => {-in => \@failed_job_ids}, text => $comment_text})->count, 2, + 'comments created on all relevant jobs'; is $comments->search({job_id => {-not_in => \@failed_job_ids}, text => $comment_text})->count, 0, 'comments not created on other jobs'; my $running_job_ids = map { $_->id } $jobs->search({state => RUNNING})->all; diff --git a/templates/webapi/test/overview.html.ep b/templates/webapi/test/overview.html.ep index b0a9e10f672..235044b192a 100644 --- a/templates/webapi/test/overview.html.ep +++ b/templates/webapi/test/overview.html.ep @@ -42,11 +42,10 @@
% my $allow_commenting = @$job_ids && current_user; % if ($allow_commenting) { - % } @@ -219,40 +218,39 @@ % if ($allow_commenting) {