Skip to content
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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

alondhe
Copy link
Contributor

@alondhe alondhe commented Jun 14, 2023

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:

image

If set to true but no cores are accessible based on the WebAPI's info endpoint:

image

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.

image

Only is visibe if the app config's useSolrVocabSearch setting is true
@chrisknoll
Copy link
Collaborator

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 /info to tell if solr is enabled, and which cores are enabled, and render the UI based on that. This prevents someone 'forgetting' to set a param and simplifies the functionality.

@@ -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;
Copy link
Collaborator

@chrisknoll chrisknoll Jun 22, 2023

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;
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants