Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove limit option in next&previous tab UI #1658

Merged
merged 2 commits into from
May 28, 2018
Merged

Remove limit option in next&previous tab UI #1658

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2274e52
Remove limit option in next&previous tab UI
mitiao May 23, 2018
14fcf37
Start the display with the current job
mitiao May 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 15 additions & 10 deletions assets/javascripts/job_next_previous.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
function setupJobNextPrevious() {
var params = parseQueryParams();
var table = $('#job_next_previous_table').DataTable({
ajax: {
url: $('#job_next_previous_table').data('ajax-url'),
data: function ( d ) {
d.limit = $('#limit_num').val();
if (typeof params.previous_limit != 'undefined') {
d.previous_limit = params.previous_limit.toString();
}
if (typeof params.next_limit != 'undefined') {
d.next_limit = params.next_limit.toString();
}
}
},
paging: true,
Expand All @@ -28,7 +34,14 @@ function setupJobNextPrevious() {
},
{targets: 2, render: renderBuild},
{targets: 3, render: renderFinishTime},
]
],
initComplete: function (settings, json) {
// Seems an issue in case of displayStart is not an integer multiple of the pageLength
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, wouldn't it still be easier to take your var page as displayStart = page * pageLength? I find changing the table in initComplete a little problematic - as initComplete is called after the table was drawn, meaning it would blink

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i didn't see blinking during initComplete called, from console no repeating draw shown.
test instance: http://10.67.18.160:3000/tests/72

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

options can't be changed after initialized, i wonder to know a better way to pass value to option during in initialisation?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. So we would have to calculate this earlier - which would break lazy load.

Your test instance doesn't work for me and I'm in the middle of something else, so I can't test locally. So having to trust you that there is no blink :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to work without blinking, also when 'Next & previous results' is the initial tab.

// Caculate and start the page with current job
var current_index = json.data.map(function(n) {return n.iscurrent;}).indexOf(1);
var page = Math.min(Math.max(0, Math.floor(current_index / table.page.len())), table.page.info().pages);
table.page(page).draw('page');
}
});
$('#job_next_previous_table').on('draw.dt', function (){
setupLazyLoadingFailedSteps();
Expand Down Expand Up @@ -144,12 +157,4 @@ function triggerJobNextPrevious() {
setupJobNextPrevious();
}
}

$('#limit_num').on('change', function() {
if ($.fn.dataTable.isDataTable('#job_next_previous_table')) {
var table = $('#job_next_previous_table').DataTable();
table.destroy();
}
setupJobNextPrevious();
});
}
7 changes: 4 additions & 3 deletions lib/OpenQA/Schema/ResultSet/Jobs.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,17 +417,18 @@ sub _cancel_or_deprioritize {

sub next_previous_jobs_query {
my ($self, $job, $jobid, %args) = @_;
my $limit = $args{limit};
my @params;
my $p_limit = $args{previous_limit};
my $n_limit = $args{next_limit};

my @params;
push @params, 'done';
push @params, OpenQA::Schema::Result::Jobs::INCOMPLETE_RESULTS;
for (1 .. 2) {
for my $key (OpenQA::Schema::Result::Jobs::SCENARIO_WITH_MACHINE_KEYS) {
push @params, $job->get_column($key);
}
}
push @params, $jobid, $limit, $jobid, $limit, $jobid;
push @params, $jobid, $n_limit, $jobid, $p_limit, $jobid;

my $jobs_rs = $self->result_source->schema->resultset('JobNextPrevious')->search(
{},
Expand Down
6 changes: 4 additions & 2 deletions lib/OpenQA/WebAPI/Controller/Test.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,8 +281,10 @@ sub job_next_previous_ajax {

my $job = $self->get_current_job;
my $jobid = $job->id;
my $limit = $self->param('limit') || 10;
my $jobs_rs = $self->db->resultset("Jobs")->next_previous_jobs_query($job, $jobid, limit => $limit);
my $p_limit = $self->param('previous_limit') // 400;
my $n_limit = $self->param('next_limit') // 100;
my $jobs_rs = $self->db->resultset("Jobs")
->next_previous_jobs_query($job, $jobid, previous_limit => $p_limit, next_limit => $n_limit);
my (@jobs, @data);
my $latest = 1;
while (my $each = $jobs_rs->next) {
Expand Down
35 changes: 28 additions & 7 deletions t/ui/16-tests_job_next_previous.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,6 @@ is(scalar @{$driver->find_elements('#job_next_previous_table #job_result_99946')
is(scalar @{$driver->find_elements('#job_next_previous_table #job_result_99945')}, 1, 'found previous job 99945');
is(scalar @{$driver->find_elements('#job_next_previous_table #job_result_99944')}, 1, 'found previous job 99944');

# select limit from list for both of next and previous to render results table
$driver->find_element_by_xpath("//select[\@id='limit_num']/option[2]")->click();
wait_for_ajax();
is(scalar @{$driver->find_elements('#job_next_previous_table tbody tr', 'css')},
4, 'job next and previous with limit list selected of 99947');
is(scalar @{$driver->find_elements('#job_next_previous_table #job_result_99944')}, 1, 'found previous job 99944');

#check build links to overview page
$driver->find_element_by_link_text('0091')->click();
is(
Expand Down Expand Up @@ -196,5 +189,33 @@ $job99947 = $driver->find_element('#job_next_previous_table #job_result_99947');
@tds = $driver->find_child_elements($job99947, 'td');
is((shift @tds)->get_text(), 'C&L', '99947 is current and the latest job');

# check limit with query parameters of job next & previous
$driver->get('/tests/99947?previous_limit=1#next_previous');
wait_for_ajax();
is(scalar @{$driver->find_elements('#job_next_previous_table tbody tr', 'css')},
2, 'only 1 previous of 99947 and itself shown');
is(scalar @{$driver->find_elements('#job_next_previous_table #job_result_99947')}, 1, 'found current job 99947');
is(scalar @{$driver->find_elements('#job_next_previous_table #job_result_99946')}, 1, 'found previous job 99946');

$driver->get('/tests/99944?next_limit=1#next_previous');
wait_for_ajax();
is(scalar @{$driver->find_elements('#job_next_previous_table tbody tr', 'css')},
3, 'only 1 next, current and the latest shown');
is(scalar @{$driver->find_elements('#job_next_previous_table #job_result_99944')}, 1, 'found current job 99944');
is(scalar @{$driver->find_elements('#job_next_previous_table #job_result_99945')}, 1, 'found only 1 next job 99945');
is(scalar @{$driver->find_elements('#job_next_previous_table #job_result_99947')}, 1, 'found the latest job 99947');

$driver->get('/tests/99945?previous_limit=0&next_limit=0#next_previous');
wait_for_ajax();
is(scalar @{$driver->find_elements('#job_next_previous_table tbody tr', 'css')}, 2,
'only current and the latest shown');
is(scalar @{$driver->find_elements('#job_next_previous_table #job_result_99945')}, 1, 'found current job 99945');
is(scalar @{$driver->find_elements('#job_next_previous_table #job_result_99947')}, 1, 'found the latest job 99947');

$driver->get('/tests/latest?previous_limit=1&next_limit=1#next_previous');
wait_for_ajax();
is(scalar @{$driver->find_elements('#job_next_previous_table tbody tr', 'css')},
1, 'job next and previous of the latest job - 99981');

kill_driver();
done_testing();
16 changes: 3 additions & 13 deletions templates/test/job_next_previous.html.ep
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,7 @@
</div>
<div class="col-6 col-md-4">
<div class="float-right">
<span>Limit </span>
<select id="limit_num">
<option value="10">10</option>
<option value="20">20</option>
<option value="50">50</option>
<option value="100">100</option>
<option value="400">400</option>
</select>
<span>for both next &amp; previous</span>
<% my $content = '
<p>
Select the limit number to customize job next & previous results search. The number is limited for both valid (eg. passed, failed and softfailed)
next and previous results of current job. The limit is 10 by default.
</p>
<p>
Next & previous results table presents in following form:
<ul>
Expand All @@ -38,6 +25,9 @@
<li>...</li>
<li>...</li>
</ul>
</p>
<p>
By default up to 400 previous jobs and 100 next jobs would be shown in the table.
</p>';
%>
%= help_popover 'Help for Next & previous results' => $content, undef, undef, 'top'
Expand Down