-
Notifications
You must be signed in to change notification settings - Fork 138
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.
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
Adds Solr Vocab status to Configuration page #2869
base: master
Are you sure you want to change the base?
Conversation
Only is visibe if the app config's useSolrVocabSearch setting is true
Hi, @alondhe , we spoke off line, but for the record: let's avoid the need to set a new config param to make the core versions show, and instead depend on the informatino returned from |
Now will show solr vocab information if solrEnabled is true.
…g core information
@@ -120,7 +121,8 @@ define([ | |||
const info = await buildInfoService.getBuildInfo(); | |||
await sourceApi.initSourcesConfig(); | |||
super.onPageCreated(); | |||
this.isSolrVocabEnabled = info.configuration.vocabulary.solrEnabled; | |||
this.isSolrVocabEnabled = info.configuration.vocabulary.solrEnabled && info.configuration.vocabulary.hasOwnProperty("cores"); | |||
this.isSolrVocabCoreAvailable = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the top of your object, you've defined isSolrVocabEnabled as an observable, but on this line (125) you are overwriting it with a boolean. So, if it's not important to have this value observable (ie: have the UI respond to changes in this value) then just declare it to default to 'false'. Or, if you do want the UI to respond to changes in the value of that variable: then you should say this.isSolrVocabCoreAvailable(false)
...btw, just looking at your coe changes, it seems that it is always false...unless I'm missing whre this.isSolrVocabCoreAvailable is set to true somewhere?
this.loading(false); | ||
} | ||
|
||
getSolrVocabCoreName(info) { | ||
if (typeof(info.configuration.vocabulary.cores) != "string") { | ||
this.isSolrVocabCoreAvailable = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think i found where isSolrVocabCoreAvailable is assigned to true here. Note, it's not observable, you're just storing a boolean value. This may work if this all happens when the page loads (ie: the configuration page loads) and there is no change in the isSolrVocabCoreAvailable state while the page is alive. If that's the case, don't make anything observable.
This addresses part of #2586.
Only is visible if the app config's useSolrVocabSearch setting is true. If false (which is by default), then you see:
If set to true but no cores are accessible based on the WebAPI's info endpoint:
If set to true and at least 1 core is available based on the WebAPI's info endpoint, the first core is shown. This matches current functionality, but we should look at being able to pick different cores.