[WIP] Updates the form and configuration #82
Conversation
|
I have most of the settings backported and the app JS/CSS updated to v2.0.
I have not yet touched the block code. |
|
It's also not clear that I am handling the 'hide' options correctly. |
search_api_federated_solr.module
Outdated
| @@ -353,31 +353,31 @@ function search_api_federated_solr_admin($form, &$form_state) { | |||
| '#attributes' => [ | |||
| 'id' => ['site-name-property'], | |||
| ], | |||
| '#default_value' => variable_get('search_api_federated_solr_has_site_name_property') ? 'true' : NULL, | |||
| '#default_value' => 'true', #variable_get('search_api_federated_solr_has_site_name_property'), | |||
There was a problem hiding this comment.
On Drupal 7 we don't have an "easy" way of determining what fields are present, like we do in D8, so I defaulted all of these to 'true' for the time being.
In D8, we derive these values from $type_property = $index_config->get('field_settings.federated_type');
| * Uses the selected index server's backend connector to execute | ||
| * a select query on the index based on request qs params passed from the app. | ||
| */ | ||
| function search_api_federated_solr_proxy() { |
There was a problem hiding this comment.
This is the part that I just don't quite understand. THe D7 and D8 code for SearchAPISolr is just different enough to be confusing.
Since IE does not support RegExp() constructor
| $query_fields = array_values($query_fields_map); | ||
| } | ||
|
|
||
| // If site search is restricted, enforce it here. |
There was a problem hiding this comment.
I think we can only enforce this if site search is restricted and hidden. Otherwise, it's possible (as in UMich's case) that this could be a subsequent search and the site name facet has been unchecked. See the description for the default site name field: https://github.com/palantirnet/search_api_federated_solr/pull/82/files#diff-819816c1f9a4704c4a9e9f3e53e2d832R185
search_api_federated_solr.proxy.inc
Outdated
| } | ||
| foreach ($params['fq'] as $index => $element) { | ||
| if (substr_count($element, 'sm_site_name:') > 0) { | ||
| unset($params['fq'][$index]); |
There was a problem hiding this comment.
Can you walk me through this... is this unsetting the site name if it already exists as a qs param?
There was a problem hiding this comment.
Yes, to ensure we aren't sending more than one value to the server.
search_api_federated_solr.proxy.inc
Outdated
| } | ||
| } | ||
| // @TODO: This value might be configurable? Get from SearchAPI config. | ||
| $params['fq'][] = 'sm_site_name:("' . variable_get('site_name') . '")'; |
There was a problem hiding this comment.
Yes, this should just equal the value of the site name property on the index.
There was a problem hiding this comment.
I just fixed that part in a subsequent commit.
| @@ -73,7 +71,8 @@ dpm($index); | |||
| } | |||
|
|
|||
| // If site search is restricted, enforce it here. | |||
| $restrict_site = variable_get('search_api_federated_solr_set_search_site', 0); | |||
| $restrict_site = variable_get('search_api_federated_solr_set_search_site', 0) | |||
| && variable_get('search_api_federated_solr_hide_site_name', 0); | |||
There was a problem hiding this comment.
@jesconstantine Since this only works with the proxy right now, should we force the proxy to be used in configuration, or just set a note in the UI that the proxy must be enabled?
There was a problem hiding this comment.
The only time it won't work is if the site is not using the proxy AND the search page is browsed to directly, right? I think we can make a note in the UI with that restriction.
I also envision a near future where we only support using the proxy (or a view for autocomplete).
No description provided.