[nick-boss-tech]

🤖 AI text below 🤖 (reviewed by Nick Shanin)

Jira: https://issues.apache.org/jira/browse/SOLR-16398

Follow-up to the split-shard UI findings.

What happens today

Navigation refreshes can clear the collection selection too early or display state from an older route.

At the pre-change base, resetMenu clears the model before the collection and alias requests finish. Older requests can also complete after a newer navigation and overwrite its menu. Alias overview copies that model once, before loading completes, so its details can remain empty or stale. These paths predate the split-shard migration.

What this change does

Only the latest refresh updates the menu, and alias details follow the requested alias.

• Preserve the collection selection and options until both requests succeed, then publish the rebuilt lists and matching selection together. A successful root-page or no-match refresh clears the selection. A current list/alias failure preserves the old selection and reports the error.
• Ignore obsolete core, system, collection and alias callbacks. Starting another refresh or changing routes invalidates pending callbacks.
• Update alias overview when the refreshed model changes, accepting only an alias matching its route and clearing the view when it disappears.
• Keep the empty-input guard inside showCollection, protecting every call to the handler.

Implementation: menu refresh and alias overview.

Proof

JavaScript syntax checks pass; Linux behavioral and build verification is pending.

Verification at 200ff5e

On Windows, Node v22.17.0 parses both changed production JavaScript files and all five embedded Selenium script blocks. Scoped git diff --check passes.

AdminUiMenuRefreshTest adds nine tests using the bundled Angular/Chosen code, production selector attributes, actual alias template and controlled API callback ordering.

Linux execution of those tests, the two neighboring collection-screen suites, tidy, the webapp check and root check -x test is pending. Fail-before comparisons against the original production JavaScript are also pending. No passing browser or build result is claimed for this head yet.

Limits

This changes legacy UI navigation and display; no collection API migration behavior changes.

The tests use controlled callback ordering. Direct empty-handler coverage does not establish that deleting a model automatically fires ng-change on every page load.

Changelog: SOLR-16398-admin-ui-showcollection.yml.

AI-generated description. Nick Shanin directed the work and takes responsibility for this contribution.

[epugh]

are there other options?

I went back, and this is what Claude said when I first asked about it:

So this isn't really "SOLR-16398 introduced a bug and patched it" — it's a pre-existing latent crash in the global nav controller that the author happened to trip over while testing their split-shard changes in the UI, and fixed with a one-line symptom guard rather than a root-cause fix or a separate ticket.

The more correct fix would address why currentCollection is unset at all during that window — e.g.:

• Don't delete $scope.currentCollection until the new list has loaded and failed to contain a match, instead of unconditionally clearing it upfront, so the dropdown doesn't flash to blank/undefined on every navigation.
• Or guard at the ng-change call site in index.html instead of inside showCollection, so other callers of the collection-selector aren't left exposed to the same unset-during-load window.

Are there other potential issues that need fixing in the same vein?

[nick-boss-tech]

🤖 AI text below 🤖 (reviewed by Nick Shanin)

Thanks, Eric. I followed the approach of keeping the selection until the refreshed lists are ready, and found two related problems:

• Responses from an earlier navigation could finish late and replace the current menu. Refresh callbacks now ignore obsolete requests, including after a route change.
• Alias overview copied the selection before loading completed and never updated that copy. It now follows the refreshed alias matching its route, including changes to its targets and properties, and clears its details when that alias disappears.

The empty-input guard stays in showCollection, protecting every call to the handler. A guard at the ng-change call site would not fix those shared-state problems. Current collection/alias request failures preserve the previous selection while reporting the error.

The changes and regression tests are in this diff.

Linux verification at 200ff5e

• JavaScript syntax checks, full tidy with no tracked changes, and :solr:webapp:check -x test pass on Linux with JDK 21 and Gradle 9.7.0.

• AdminUiMenuRefreshTest compiles and discovers nine tests, but all nine were skipped because the Selenium group was disabled. This host had no usable Chrome installation, so no browser test executed.

• The neighboring browser suites, both comparisons against the original production code, and root check -x test have not run. Browser behavior and fail-before results remain unverified.

• The upstream Admin UI Tests run currently reports action_required.

The tests use controlled callback ordering with the bundled Angular/Chosen implementation. The empty-input test calls the handler directly; it does not establish that deleting a model automatically fires ng-change on every page load.

[epugh]

Slightly scared by this .executeScript, I did a quick poke around and didn't see other Selenium tests doing this, but maybe it's because of what we are trying to test?

I know it may be heresay, but I almost want to keep the code changes, but skip this test as more effort/plumbing than it's worth...

[epugh]

no worries about the menuGeneration number becoming crazy right? THis resetMenu is when we cahnge things, it doesn't happen in the background right?

[nick-boss-tech]

🤖 AI text below 🤖 (reviewed by Nick Shanin)

Thanks, Eric. I understand the concern about the test setup. JavascriptExecutor is already used in AdminUiSqlScreenTest. Here it controls callback order so the tests can deliver an old response after a newer navigation while exercising the bundled Angular/Chosen code. I’d prefer to retain that coverage if it proves reliable in CI, but I’m happy to remove the test afterward if you feel the maintenance cost outweighs its value.

On menuGeneration: yes, it advances on route changes and explicit menu refreshes, with no periodic timer incrementing it. Reaching JavaScript’s integer precision limit would require an infeasible number of refreshes during one browser session.

I fixed the CI error. Could you trigger it again please?

[epugh]

done

[nick-boss-tech]

I saw the test was green and I removed it now.