From fe44f3eef04cd6fddbccb9d8c88755c8d0fb5801 Mon Sep 17 00:00:00 2001 From: tfennelly Date: Thu, 15 Oct 2015 09:35:02 +0100 Subject: [PATCH] Fixed build history pagination/search issues Fix HistoryPageFileTest tests Fix int parsing in JavaScript Fix some JS glitches --- .../jenkins/widgets/HistoryPageEntry.java | 5 ++- .../jenkins/widgets/HistoryPageFilter.java | 16 ++++--- .../widgets/HistoryPageFilterTest.java | 43 +++++++++++-------- .../main/webapp/scripts/hudson-behavior.js | 18 +++++--- 4 files changed, 48 insertions(+), 34 deletions(-) diff --git a/core/src/main/java/jenkins/widgets/HistoryPageEntry.java b/core/src/main/java/jenkins/widgets/HistoryPageEntry.java index 8a1078ef5f..b3aeaecedb 100644 --- a/core/src/main/java/jenkins/widgets/HistoryPageEntry.java +++ b/core/src/main/java/jenkins/widgets/HistoryPageEntry.java @@ -59,7 +59,10 @@ public class HistoryPageEntry { return ((Queue.Item) entry).getId(); } else if (entry instanceof Run) { Run run = (Run) entry; - return (Integer.MIN_VALUE + run.getNumber()); + return (Long.MIN_VALUE + run.getNumber()); + } else if (entry instanceof Number) { + // Used for testing purposes because of JENKINS-30899 and JENKINS-30909 + return (Long.MIN_VALUE + ((Number) entry).longValue()); } else { return Run.QUEUE_ID_UNKNOWN; } diff --git a/core/src/main/java/jenkins/widgets/HistoryPageFilter.java b/core/src/main/java/jenkins/widgets/HistoryPageFilter.java index b86e4f53e7..6101916958 100644 --- a/core/src/main/java/jenkins/widgets/HistoryPageFilter.java +++ b/core/src/main/java/jenkins/widgets/HistoryPageFilter.java @@ -227,18 +227,20 @@ public class HistoryPageFilter { } private void addQueueItem(Queue.Item item) { - updateNewestOldest(item.getId()); - queueItems.add(new HistoryPageEntry(item)); + HistoryPageEntry entry = new HistoryPageEntry(item); + queueItems.add(entry); + updateNewestOldest(entry.getEntryId()); } private void addRun(Run run) { - updateNewestOldest(run.getQueueId()); - runs.add(new HistoryPageEntry(run)); + HistoryPageEntry entry = new HistoryPageEntry(run); + runs.add(entry); + updateNewestOldest(entry.getEntryId()); } - private void updateNewestOldest(long queueId) { - newestOnPage = Math.max(newestOnPage, queueId); - oldestOnPage = Math.min(oldestOnPage, queueId); + private void updateNewestOldest(long entryId) { + newestOnPage = Math.max(newestOnPage, entryId); + oldestOnPage = Math.min(oldestOnPage, entryId); } private boolean add(T entry) { diff --git a/core/src/test/java/jenkins/widgets/HistoryPageFilterTest.java b/core/src/test/java/jenkins/widgets/HistoryPageFilterTest.java index 61d89f9da3..239a606bd5 100644 --- a/core/src/test/java/jenkins/widgets/HistoryPageFilterTest.java +++ b/core/src/test/java/jenkins/widgets/HistoryPageFilterTest.java @@ -70,7 +70,7 @@ public class HistoryPageFilterTest { itemList.addAll(newQueueItems(3, 4)); // want to make sure the list items are ordered by id in descending order - Assert.assertEquals(1, HistoryPageEntry.getEntryId(itemList.get(0))); + Assert.assertEquals(HistoryPageEntry.getEntryId(1), HistoryPageEntry.getEntryId(itemList.get(0))); historyPageFilter.add(itemList); Assert.assertEquals(4, HistoryPageEntry.getEntryId(itemList.get(0))); @@ -82,9 +82,9 @@ public class HistoryPageFilterTest { Assert.assertEquals(4, historyPageFilter.queueItems.get(0).getEntryId()); Assert.assertEquals(4, historyPageFilter.newestOnPage); Assert.assertEquals(3, historyPageFilter.queueItems.get(1).getEntryId()); - Assert.assertEquals(2, historyPageFilter.runs.get(0).getEntryId()); - Assert.assertEquals(1, historyPageFilter.runs.get(1).getEntryId()); - Assert.assertEquals(1, historyPageFilter.oldestOnPage); + Assert.assertEquals(HistoryPageEntry.getEntryId(2), historyPageFilter.runs.get(0).getEntryId()); + Assert.assertEquals(HistoryPageEntry.getEntryId(1), historyPageFilter.runs.get(1).getEntryId()); + Assert.assertEquals(HistoryPageEntry.getEntryId(1), historyPageFilter.oldestOnPage); } /** @@ -107,7 +107,7 @@ public class HistoryPageFilterTest { Assert.assertEquals(12, historyPageFilter.queueItems.get(0).getEntryId()); Assert.assertEquals(12, historyPageFilter.newestOnPage); - Assert.assertEquals(10, historyPageFilter.runs.get(0).getEntryId()); + Assert.assertEquals(HistoryPageEntry.getEntryId(10), historyPageFilter.runs.get(0).getEntryId()); } /** @@ -126,8 +126,8 @@ public class HistoryPageFilterTest { Assert.assertEquals(true, historyPageFilter.hasDownPage); Assert.assertEquals(5, historyPageFilter.runs.size()); - Assert.assertEquals(10, historyPageFilter.newestOnPage); - Assert.assertEquals(6, historyPageFilter.oldestOnPage); + Assert.assertEquals(HistoryPageEntry.getEntryId(10), historyPageFilter.newestOnPage); + Assert.assertEquals(HistoryPageEntry.getEntryId(6), historyPageFilter.oldestOnPage); } /** @@ -165,8 +165,8 @@ public class HistoryPageFilterTest { // Should only be 3 runs on the page (oldest 3) Assert.assertEquals(3, historyPageFilter.runs.size()); - Assert.assertEquals(3, historyPageFilter.newestOnPage); - Assert.assertEquals(1, historyPageFilter.oldestOnPage); + Assert.assertEquals(HistoryPageEntry.getEntryId(3), historyPageFilter.newestOnPage); + Assert.assertEquals(HistoryPageEntry.getEntryId(1), historyPageFilter.oldestOnPage); } /** @@ -184,8 +184,8 @@ public class HistoryPageFilterTest { Assert.assertEquals(true, historyPageFilter.hasDownPage); Assert.assertEquals(5, historyPageFilter.runs.size()); - Assert.assertEquals(7, historyPageFilter.newestOnPage); - Assert.assertEquals(3, historyPageFilter.oldestOnPage); + Assert.assertEquals(HistoryPageEntry.getEntryId(7), historyPageFilter.newestOnPage); + Assert.assertEquals(HistoryPageEntry.getEntryId(3), historyPageFilter.oldestOnPage); } /** @@ -220,8 +220,8 @@ public class HistoryPageFilterTest { Assert.assertEquals(false, historyPageFilter.hasDownPage); Assert.assertEquals(5, historyPageFilter.runs.size()); - Assert.assertEquals(5, historyPageFilter.newestOnPage); - Assert.assertEquals(1, historyPageFilter.oldestOnPage); + Assert.assertEquals(HistoryPageEntry.getEntryId(5), historyPageFilter.newestOnPage); + Assert.assertEquals(HistoryPageEntry.getEntryId(1), historyPageFilter.oldestOnPage); } /** @@ -239,8 +239,8 @@ public class HistoryPageFilterTest { Assert.assertEquals(true, historyPageFilter.hasDownPage); Assert.assertEquals(5, historyPageFilter.runs.size()); - Assert.assertEquals(8, historyPageFilter.newestOnPage); - Assert.assertEquals(4, historyPageFilter.oldestOnPage); + Assert.assertEquals(HistoryPageEntry.getEntryId(8), historyPageFilter.newestOnPage); + Assert.assertEquals(HistoryPageEntry.getEntryId(4), historyPageFilter.oldestOnPage); } /** @@ -260,8 +260,8 @@ public class HistoryPageFilterTest { Assert.assertEquals(true, historyPageFilter.hasDownPage); Assert.assertEquals(5, historyPageFilter.runs.size()); - Assert.assertEquals(10, historyPageFilter.newestOnPage); - Assert.assertEquals(6, historyPageFilter.oldestOnPage); + Assert.assertEquals(HistoryPageEntry.getEntryId(10), historyPageFilter.newestOnPage); + Assert.assertEquals(HistoryPageEntry.getEntryId(6), historyPageFilter.oldestOnPage); } private List newQueueItems(long startId, long endId) { @@ -283,9 +283,9 @@ public class HistoryPageFilterTest { private HistoryPageFilter newPage(int maxEntries, Long newerThan, Long olderThan) { HistoryPageFilter pageFilter = new HistoryPageFilter(maxEntries); if (newerThan != null) { - pageFilter.setNewerThan(newerThan); + pageFilter.setNewerThan(HistoryPageEntry.getEntryId(newerThan)); } else if (olderThan != null) { - pageFilter.setOlderThan(olderThan); + pageFilter.setOlderThan(HistoryPageEntry.getEntryId(olderThan)); } return pageFilter; } @@ -318,5 +318,10 @@ public class HistoryPageFilterTest { public long getQueueId() { return queueId; } + + @Override + public int getNumber() { + return (int) queueId; + } } } diff --git a/war/src/main/webapp/scripts/hudson-behavior.js b/war/src/main/webapp/scripts/hudson-behavior.js index 62e4884099..04aa378231 100644 --- a/war/src/main/webapp/scripts/hudson-behavior.js +++ b/war/src/main/webapp/scripts/hudson-behavior.js @@ -1897,6 +1897,7 @@ function updateBuildHistory(ajaxUrl,nBuild) { } } + var updateBuildsRefreshInterval = 5000; function updateBuilds() { if(isPageVisible()){ if (bh.headers == null) { @@ -1947,13 +1948,12 @@ function updateBuildHistory(ajaxUrl,nBuild) { createRefreshTimeout(); } }); - } else { + } else { // Reschedule again createRefreshTimeout(); } } - var updateBuildsRefreshInterval = 5000; var buildRefreshTimeout; function createRefreshTimeout() { cancelRefreshTimeout(); @@ -1968,7 +1968,6 @@ function updateBuildHistory(ajaxUrl,nBuild) { createRefreshTimeout(); checkAllRowCellOverflows(); - window.setTimeout(updateBuilds, updateBuildsRefreshInterval); onBuildHistoryChange(function() { checkAllRowCellOverflows(); @@ -2005,10 +2004,10 @@ function updateBuildHistory(ajaxUrl,nBuild) { return buildHistoryPage.getAttribute('page-has-down') === 'true'; } function getNewestEntryId() { - return parseInt(buildHistoryPage.getAttribute('page-entry-newest')); + return buildHistoryPage.getAttribute('page-entry-newest'); } function getOldestEntryId() { - return parseInt(buildHistoryPage.getAttribute('page-entry-oldest')); + return buildHistoryPage.getAttribute('page-entry-oldest'); } function updatePageParams(dataTable) { buildHistoryPage.setAttribute('page-has-up', dataTable.getAttribute('page-has-up')); @@ -2106,8 +2105,13 @@ function updateBuildHistory(ajaxUrl,nBuild) { loadPage({'newer-than': getNewestEntryId()}); }); pageDown.observe('click', function() { - cancelRefreshTimeout(); - loadPage({'older-than': getOldestEntryId()}); + if (hasPageDown()) { + cancelRefreshTimeout(); + loadPage({'older-than': getOldestEntryId()}); + } else { + // wrap back around to the top + loadPage(); + } }); togglePageUpDown();