This is an automated email from the ASF dual-hosted git repository. dsmiley pushed a commit to branch branch_10x in repository https://gitbox.apache.org/repos/asf/solr.git
commit 0e2cb4df216d1006a5a0a7b00622baa1898030cc Author: NextBrickINC.com <[email protected]> AuthorDate: Sun Sep 27 10:28:22 2026 -0700 SOLR-18347: Fix four Admin UI defects found by Selenium (#4818) Co-authored-by: rayshrey <[email protected]> Co-authored-by: Shrey Narayan <[email protected]> Co-authored-by: Claude Opus 5 <[email protected]> Co-authored-by: Eric Pugh <[email protected]> (cherry picked from commit c526efab696bd097088f277a03b0d69b8c92398f) --- .../unreleased/solr-18347-admin-ui-defects.yml | 9 +++++++++ dev-docs/admin-ui-tests.md | 11 ----------- .../org/apache/solr/handler/PingRequestHandler.java | 15 +++++++-------- .../apache/solr/handler/PingRequestHandlerTest.java | 4 ++++ .../solr/webapp/AdminUiCollectionScreensTest.java | 4 ++-- .../solr/webapp/AdminUiCollectionsScreenTest.java | 10 ++++++++-- .../solr/webapp/AdminUiCoreAdminStandaloneTest.java | 4 ++-- .../org/apache/solr/webapp/AdminUiSmokeTest.java | 4 +--- .../org/apache/solr/webapp/AdminUiTestBase.java | 6 ------ solr/webapp/web/js/angular/app.js | 4 +++- .../web/js/angular/controllers/collections.js | 6 +++--- .../web/js/angular/controllers/core-overview.js | 21 +++++++++++++++------ solr/webapp/web/partials/collections.html | 3 ++- 13 files changed, 56 insertions(+), 45 deletions(-) diff --git a/changelog/unreleased/solr-18347-admin-ui-defects.yml b/changelog/unreleased/solr-18347-admin-ui-defects.yml new file mode 100644 index 00000000000..00801618637 --- /dev/null +++ b/changelog/unreleased/solr-18347-admin-ui-defects.yml @@ -0,0 +1,9 @@ +# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc +title: Fix four Admin UI defects found by Selenium testing - a null core selection in the shared menu, the ping status of a core with no healthcheck file, silent failures on collection reload, and an enable-ping control offered when it cannot work. +type: fixed # added, changed, fixed, deprecated, removed, dependency_update, security, other +authors: + - name: Shrey Narayan + nick: NextbrickInc +links: + - name: SOLR-18347 + url: https://issues.apache.org/jira/browse/SOLR-18347 diff --git a/dev-docs/admin-ui-tests.md b/dev-docs/admin-ui-tests.md index 73ad2a9f65b..3a5d070ea5d 100644 --- a/dev-docs/admin-ui-tests.md +++ b/dev-docs/admin-ui-tests.md @@ -55,17 +55,6 @@ deliberately does not do. - Every test cluster in the JVM registers a log-watcher appender under the same name in the shared log4j config, so a later cluster's watcher can be blind; the events-viewer test detects this via the API and skips itself. -- The shared menu code logs a benign - `TypeError: Cannot read properties of null (reading 'name')` from - `$scope.showCore` while the per-collection menu resolves (filtered in the - console-error assertion; tracked in - [SOLR-18347](https://issues.apache.org/jira/browse/SOLR-18347)). -- The core overview ping widget answers 503 when the configset has no - healthcheck file (allowed in the affected tests; tracked in - [SOLR-18347](https://issues.apache.org/jira/browse/SOLR-18347)). -- The Schema Designer's backend transiently fails its own prep/analyze calls - with "version mismatch, retry" and recovers via its retry dialog; its API - errors are excluded from the console-error assertion. - ASF Jenkins jobs do not pass `-Ptests.selenium=true`, so these tests do not run there (a nightly job could opt in if its build nodes have a browser). In CI they run via the GitHub Actions workflow diff --git a/solr/core/src/java/org/apache/solr/handler/PingRequestHandler.java b/solr/core/src/java/org/apache/solr/handler/PingRequestHandler.java index 8333ab3c0f5..d26c298fc50 100644 --- a/solr/core/src/java/org/apache/solr/handler/PingRequestHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/PingRequestHandler.java @@ -89,8 +89,9 @@ import org.slf4j.LoggerFactory; * <ul> * <li>If the health check file exists, the handler will execute the delegated query and return * status as described above. - * <li>If the health check file does not exist, the handler will return an HTTP error even if the - * server is working fine and the delegated query would have succeeded + * <li>For a ping request, if a configured health check file does not exist, the handler will + * return an HTTP error even if the server is working fine and the delegated query would have + * succeeded. The <code>action=status</code> request instead reports the file state. * </ul> * * <p>This health check file feature can be used as a way to indicate to some Load Balancers that @@ -104,8 +105,9 @@ import org.slf4j.LoggerFactory; * <li><code>http://.../ping?action=enable</code> - creates the health check file if it does not * already exist * <li><code>http://.../ping?action=disable</code> - deletes the health check file if it exists - * <li><code>http://.../ping?action=status</code> - returns a status code indicating if the - * healthcheck file exists ("<code>enabled</code>") or not ("<code>disabled</code>") + * <li><code>http://.../ping?action=status</code> - returns the healthcheck state as <code>enabled + * </code> (configured file exists), <code>disabled</code> (configured file does not exist), + * or <code>not_configured</code> (no healthcheck file is configured) * </ul> * * @deprecated This handler is deprecated and will be removed in a future release. For load balancer @@ -220,10 +222,7 @@ public class PingRequestHandler extends RequestHandlerBase implements SolrCoreAw break; case STATUS: if (healthcheck == null) { - SolrException e = - new SolrException( - SolrException.ErrorCode.SERVICE_UNAVAILABLE, "healthcheck not configured"); - rsp.setException(e); + rsp.add("status", "not_configured"); } else { rsp.add("status", isPingDisabled() ? "disabled" : "enabled"); } diff --git a/solr/core/src/test/org/apache/solr/handler/PingRequestHandlerTest.java b/solr/core/src/test/org/apache/solr/handler/PingRequestHandlerTest.java index d676c007cad..a6a2f972172 100644 --- a/solr/core/src/test/org/apache/solr/handler/PingRequestHandlerTest.java +++ b/solr/core/src/test/org/apache/solr/handler/PingRequestHandlerTest.java @@ -86,6 +86,10 @@ public class PingRequestHandlerTest extends SolrTestCaseJ4 { rsp = makeRequest(handler, req("action", "ping")); assertEquals("OK", rsp.getValues().get("status")); + + rsp = makeRequest(handler, req("action", "status")); + assertEquals("not_configured", rsp.getValues().get("status")); + assertNull(rsp.getException()); } public void testEnablingServer() throws Exception { diff --git a/solr/webapp/src/test/org/apache/solr/webapp/AdminUiCollectionScreensTest.java b/solr/webapp/src/test/org/apache/solr/webapp/AdminUiCollectionScreensTest.java index f758d46e11b..b4e82fb5457 100644 --- a/solr/webapp/src/test/org/apache/solr/webapp/AdminUiCollectionScreensTest.java +++ b/solr/webapp/src/test/org/apache/solr/webapp/AdminUiCollectionScreensTest.java @@ -101,7 +101,7 @@ public class AdminUiCollectionScreensTest extends AdminUiTestBase { openPage(coreName + "/core-overview", By.id("dashboard")); waitForPageContains("Num Docs"); waitForPageContains(Integer.toString(NUM_DOCS)); - // the ping widget answers 503 when the configset has no healthcheck file - assertNoSevereConsoleErrors("/admin/ping"); + waitForPageContains("Ping request handler is not configured with a healthcheck file."); + assertNoSevereConsoleErrors(); } } diff --git a/solr/webapp/src/test/org/apache/solr/webapp/AdminUiCollectionsScreenTest.java b/solr/webapp/src/test/org/apache/solr/webapp/AdminUiCollectionsScreenTest.java index 8dad9b0be8a..e8e44ad6ebd 100644 --- a/solr/webapp/src/test/org/apache/solr/webapp/AdminUiCollectionsScreenTest.java +++ b/solr/webapp/src/test/org/apache/solr/webapp/AdminUiCollectionsScreenTest.java @@ -142,8 +142,8 @@ public class AdminUiCollectionsScreenTest extends AdminUiTestBase { @Test public void testReloadCollectionViaUi() throws Exception { - // reloading resets the core's start time; that proves the action end-to-end, - // unlike the UI success indicator which only flashes for a second + // Reloading resets the core's start time, and the success state remains visible long enough + // for a user to notice instead of disappearing after one second. String coreName = coreNameOnNode0(COLLECTION); Object startTimeBefore = coreStartTime(coreName); @@ -152,6 +152,12 @@ public class AdminUiCollectionsScreenTest extends AdminUiTestBase { waitUntil( "core start time should change after reload", () -> !startTimeBefore.equals(coreStartTime(coreName))); + WebElement reload = waitFor(By.cssSelector("#reload.success")); + assertEquals("Reloaded", reload.getText()); + Thread.sleep(1500); + assertTrue( + "reload success should remain visible", reload.getAttribute("class").contains("success")); + assertEquals("Reloaded", reload.getText()); assertNoSevereConsoleErrors(); } diff --git a/solr/webapp/src/test/org/apache/solr/webapp/AdminUiCoreAdminStandaloneTest.java b/solr/webapp/src/test/org/apache/solr/webapp/AdminUiCoreAdminStandaloneTest.java index 1707bc9f2a3..fb5d8f05b66 100644 --- a/solr/webapp/src/test/org/apache/solr/webapp/AdminUiCoreAdminStandaloneTest.java +++ b/solr/webapp/src/test/org/apache/solr/webapp/AdminUiCoreAdminStandaloneTest.java @@ -63,8 +63,8 @@ public class AdminUiCoreAdminStandaloneTest extends AdminUiStandaloneTestBase { openPage("swapa/core-overview", By.id("dashboard")); waitFor(By.cssSelector("#core-menu .query")); waitFor(By.cssSelector("#core-menu .replication")); - // the ping widget answers 503 when the configset has no healthcheck file - assertNoSevereConsoleErrors("/admin/ping"); + waitForPageContains("Ping request handler is not configured with a healthcheck file."); + assertNoSevereConsoleErrors(); } @Test diff --git a/solr/webapp/src/test/org/apache/solr/webapp/AdminUiSmokeTest.java b/solr/webapp/src/test/org/apache/solr/webapp/AdminUiSmokeTest.java index 5dfc5b2a451..a2005916018 100644 --- a/solr/webapp/src/test/org/apache/solr/webapp/AdminUiSmokeTest.java +++ b/solr/webapp/src/test/org/apache/solr/webapp/AdminUiSmokeTest.java @@ -91,9 +91,7 @@ public class AdminUiSmokeTest extends AdminUiTestBase { coreName + "/plugins", By.id("plugins"), coreName + "/segments", By.id("segments")); screens.forEach(this::smoke); - // the ping widget on the overview answers 503 when no healthcheck file is configured, - // as is the case for the _default configset - smoke(coreName + "/core-overview", By.id("dashboard"), "/admin/ping"); + smoke(coreName + "/core-overview", By.id("dashboard")); } private void smoke(String route, By anchor) { diff --git a/solr/webapp/src/test/org/apache/solr/webapp/AdminUiTestBase.java b/solr/webapp/src/test/org/apache/solr/webapp/AdminUiTestBase.java index a0f6469aa17..77674cadc4d 100644 --- a/solr/webapp/src/test/org/apache/solr/webapp/AdminUiTestBase.java +++ b/solr/webapp/src/test/org/apache/solr/webapp/AdminUiTestBase.java @@ -533,12 +533,6 @@ public abstract class AdminUiTestBase extends SolrCloudTestCase { Arrays.stream(allowedSubstrings) .noneMatch(allowed -> entry.getMessage().contains(allowed))) .filter(entry -> !entry.getMessage().contains("favicon.ico")) - // benign race in the shared menu code: showCore() fires with a null core - // while the per-collection menu resolves after navigation - .filter( - entry -> - !(entry.getMessage().contains("reading 'name'") - && entry.getMessage().contains("showCore"))) .toList(); assertTrue("Severe browser console errors: " + severe, severe.isEmpty()); } diff --git a/solr/webapp/web/js/angular/app.js b/solr/webapp/web/js/angular/app.js index 27a2f458682..1bbaa4b9163 100644 --- a/solr/webapp/web/js/angular/app.js +++ b/solr/webapp/web/js/angular/app.js @@ -653,7 +653,9 @@ solrAdminApp.controller('MainController', function($scope, $route, $rootScope, $ } $scope.showCore = function(core) { - $location.url("/" + core.name + "/core-overview"); + if (core) { + $location.url("/" + core.name + "/core-overview"); + } } $scope.showCollection = function(collection) { diff --git a/solr/webapp/web/js/angular/controllers/collections.js b/solr/webapp/web/js/angular/controllers/collections.js index d7a14b3dcf3..1c93b088b5e 100644 --- a/solr/webapp/web/js/angular/controllers/collections.js +++ b/solr/webapp/web/js/angular/controllers/collections.js @@ -230,15 +230,15 @@ solrAdminApp.controller('CollectionsController', alert("No collection selected."); return; } + $scope.reloadSuccess = false; + $scope.reloadFailure = false; CollectionsV2.reloadCollection($scope.collection.name, {}, function(error, data,response) { $timeout(function() { if (error) { $scope.reloadFailure = true; - $timeout(function() {$scope.reloadFailure=false}, 1000); - $location.path("/~collections"); + ApiErrorHandler.handle(response); } else { $scope.reloadSuccess = true; - $timeout(function() {$scope.reloadSuccess=false}, 1000); } }); }); diff --git a/solr/webapp/web/js/angular/controllers/core-overview.js b/solr/webapp/web/js/angular/controllers/core-overview.js index 4c97e6d12b0..7ea076b432a 100644 --- a/solr/webapp/web/js/angular/controllers/core-overview.js +++ b/solr/webapp/web/js/angular/controllers/core-overview.js @@ -56,14 +56,23 @@ function($scope, $rootScope, $routeParams, Luke, CoreInfo, Update, Replication, $scope.refreshPing = function() { Ping.status({core: $routeParams.core}, function(data) { - if (data.error) { - $scope.healthcheckStatus = false; - if (data.error.code == 503) { - $scope.healthcheckMessage = 'Ping request handler is not configured with a healthcheck file.'; - } - } else { + // Three states, and they are not interchangeable. "enabled" and "disabled" both mean a + // healthcheck file is configured, so toggleHealthcheck() works and the widget shows the + // lit / unlit control. "not_configured" means there is no healthcheck file at all, and + // enable/disable would answer 503 - so set a message, which hides the toggle rather than + // offering a control that cannot work. + delete $scope.healthcheckMessage; + if (data.status == "enabled" || data.status == "disabled") { $scope.healthcheckStatus = data.status == "enabled"; + } else { + $scope.healthcheckStatus = false; + $scope.healthcheckMessage = data.status == "not_configured" + ? 'Ping request handler is not configured with a healthcheck file.' + : 'Unexpected ping status: ' + data.status; } + }, function(error) { + $scope.healthcheckStatus = false; + $scope.healthcheckMessage = error.data && error.data.error ? error.data.error.msg : 'Unable to read ping status.'; }); }; diff --git a/solr/webapp/web/partials/collections.html b/solr/webapp/web/partials/collections.html index a05ebe7198f..8973d31562c 100644 --- a/solr/webapp/web/partials/collections.html +++ b/solr/webapp/web/partials/collections.html @@ -128,7 +128,8 @@ limitations under the License. <button id="delete-alias" ng-class="{action,disabled:!isPermitted(permissions.COLL_EDIT_PERM)}" ng-disabled="!isPermitted(permissions.COLL_EDIT_PERM)" ng-click="toggleDeleteAlias()" ng-show="collection && collection.type === 'alias'"><span>Delete alias</span></button> <button id="reload" ng-click="reloadCollection()" ng-show="collection && collection.type === 'collection'" ng-disabled="!isPermitted(permissions.COLL_EDIT_PERM)" - ng-class="{success: reloadSuccess, warn: reloadFailure, disabled:!isPermitted(permissions.COLL_EDIT_PERM)}"><span>Reload</span></button> + ng-class="{success: reloadSuccess, warn: reloadFailure, disabled:!isPermitted(permissions.COLL_EDIT_PERM)}" + aria-live="polite"><span>{{reloadSuccess ? 'Reloaded' : (reloadFailure ? 'Reload failed' : 'Reload')}}</span></button> <div class="action delete" ng-show="showDelete">
