NextbrickInc commented on PR #4818:
URL: https://github.com/apache/solr/pull/4818#issuecomment-5548800068

   @epugh — congrats on 
[apache/solr#4819](https://github.com/apache/solr/pull/4819)
    landing, and thanks for flagging up front that you'd pull the Schema 
Designer fixes in. Good to see them exercising the V2 client.
   
   I've merged main and pushed 81dfd6cb6. The Schema Designer changes are yours 
in main now, so schema-designer.js, schema-designer.html and 
AdminUiSchemaDesignerTest.java are out of this PR entirely and the changelog 
entry is narrowed to four fixes — which also answers @janhoy's point about this 
being overloaded. GitHub reports the branch mergeable again.
   
   One merge detail worth knowing: AdminUiTestBase conflicted because we each 
removed a different console-error filter. SOLR-18399 ships the ui-grid fonts, 
so main dropped that one; this branch fixes the null showCore in app.js, so it 
dropped that one. Both are obsolete, so the merge drops both — the 
console-error assertion now runs unfiltered apart from favicon.ico.
   
   The four fixes, each with its test:
   
   PingRequestHandler — action=status on a core with no healthcheck file threw 
a 503 SolrException instead of reporting state. Now returns status: 
"not_configured". Covered by PingRequestHandlerTest.
   app.js — the core selector's ng-change could fire showCore() with a null 
model while the shared menu was still resolving, throwing TypeError on 
core.name. Guarded.
   core-overview.js — handles enabled / disabled / not_configured explicitly, 
so an unknown status no longer renders an enable-ping control that can't work. 
This is the change you and Jan asked for, in 9b5545d.
   collections.js + collections.html — a failed collection reload silently 
redirected to the collections list; now it goes through ApiErrorHandler. The 
button reports Reloaded / Reload failed via aria-live instead of a 1s colour 
flash.
   
   Local verification on the merged tree: solr:webapp:test 
-Ptests.selenium=true — 53 tests across 18 suites, 0 failures, 0 skipped. 
PingRequestHandlerTest — 6 tests, 0 failures. ./gradlew tidy clean.
   
   The workflows need a maintainer approval to run. If the Crave job goes red 
again, it's the 40-minute job timeout it hit on the previous run, not a test 
failure — gradle check and the Admin UI browser tests are the meaningful 
signals.
   
   One ask while you're in these files: changelog/unreleased/SOLR-18152-...yml 
lists only your name, and its title covers the analyze fix that came from this 
branch. Please add me to
   
   authors:
   
   - name: Eric Pugh
   - name: Shrey Narayan
   
   
   Nothing outstanding on my side — branch is pushed, PR is mergeable, all 
tests pass.
   
   This is conflict-free, green locally, and scoped to four independent fixes. 
Please approve and merge when you get a chance. Thanks . 
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to