Add option to restart jobs upon comment submission #5946
Conversation
| <input class="form-check-input" | ||
| type="checkbox" value="" id="wannaRestartCheck"> | ||
| <label class="form-check-label" for="wannaRestartCheck"> | ||
| Restart on Submit |
There was a problem hiding this comment.
| Restart on Submit | |
| Also restart job(s) on submit |
There was a problem hiding this comment.
Can I avoid also? Restart job(s) on submit sound sufficient, no?
There was a problem hiding this comment.
I see the "also" as necessary to make it clear that the checkbox wouldn't "restart instead of commenting" but "restart and comment". But after the discussion we just had I suggest to actually have two buttons:
- Left, less-alerting button for "comment", e.g. in light orange
- Right, more-alerting button for "Comment and restart", e.g. the current red color
This commit enhances the comment submission functionality with an integrated job restart option, improving workflow efficiency for users managing multiple jobs. For this `restart_job` helper added in `Helpers.pm` and `_restart` in `Job.pm` had to be refactored to encapsulate the logic and the reusability of the feature(restart_jobs) in multiple places within the application. https://progress.opensuse.org/issues/166559 Signed-off-by: Ioannis Bonatakis <[email protected]>
b10n1k
force-pushed
the
batch_comment_with_restart
branch
from
d4080b2 to
2c24af9
Compare
|
for the sake of completeness and avoid any confusion I fixed the issue with the test and some requests from feedback. But I flag the PR as |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5946 +/- ##
==========================================
- Coverage 98.73% 98.71% -0.03%
==========================================
Files 395 395
Lines 38764 38782 +18
==========================================
+ Hits 38275 38282 +7
- Misses 489 500 +11 ☔ View full report in Codecov by Sentry. |
| my ($res, $jobs, $auto, $single_job_id, $dup_route); | ||
| my %args = (jobs => $job_id); | ||
| $self->param('jobid', $job_id); | ||
| eval { ($res, $jobs, $auto, $single_job_id, $dup_route) = $self->restart_job(\%args); }; |
There was a problem hiding this comment.
The _restart method already supports multiple jobs as parameters.
The documentation for the /restart route says "Restart job(s)."
Calling this for every single job and validating the same request parameters again and again is not necessary.
Also you are passing something in %args but you never read that in the function.
And there is already one function that restarts one job, that could also be called, instead of calling the whole _restart thing:
my $res = OpenQA::Resource::Jobs::job_restart($jobs, @params);
There was a problem hiding this comment.
@Martchus and that's actually also what I meant earlier today: _restart is a complete endpoint:
sub restart ($self) { $self->_restart }
making a helper that implements a complete endpoint, doesn't make much sense to me.
|
This pull request is now in conflicts. Could you fix it? 🙏 |
|
Obsoleted by #5971 I guess? |
This commit enhances the comment submission functionality with an integrated job restart option, improving workflow efficiency for users managing multiple jobs.
For this
restart_jobhelper added inHelpers.pmand_restartinJob.pmhad to be refactored to encapsulate the logic and the reusability of the feature(restart_jobs) in multiple places within the application.https://progress.opensuse.org/issues/166559