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

Solr search #1882

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

Solr search #1882

wants to merge 9 commits into from

Conversation

pandurx
Copy link
Contributor

@pandurx pandurx commented Jul 11, 2018

This PR is related to the issue in the GCcollab repository.
gctools-outilsgc/gccollab#336

The following files have been changed:

  • please ignore vendor directory, as it reflects changes due to composer install
  • /elgg_solr/lib/hooks.php
  • /elgg_solr/start.php commented the hook to search for tags, no longer able to search tags
  • /elgg_solr/lib/functions.php commented out the enabled content
  • /gcconnex_theme/views/... i needed to temporarily set the search url to use the solr engine
  • /solr_api/start.php

Additional Information

  • need to upgrade apache solr 6.1.0 to apache solr 7.4.0
  • need to install groovy in order to run groovy scripts
  • create api key to access the api required to do indexing
  • using groovy scripts to do indexing on the site through api/database crawl
  • the vendors in the elgg_solr plugin is updated through composer install

@pandurx pandurx requested review from Phanoix and removed request for Phanoix July 11, 2018 19:51
@pandurx pandurx self-assigned this Jul 12, 2018
@pandurx pandurx requested a review from Phanoix July 12, 2018 20:04
Copy link
Contributor

@Phanoix Phanoix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only one part (the last one) that might actually be a bug... one of the things is not like the others.

@@ -514,9 +514,9 @@ function elgg_solr_get_default_fq($params) {
$fq['access'] = $access_query;
}

if (!access_get_show_hidden_status()) {
/*if (!access_get_show_hidden_status()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delete instead?

@@ -36,9 +36,11 @@ function elgg_solr_init() {
elgg_register_plugin_hook_handler('search', 'object', 'elgg_solr_object_search');
elgg_register_plugin_hook_handler('search', 'user', 'elgg_solr_user_search');
elgg_register_plugin_hook_handler('search', 'group', 'elgg_solr_group_search');
elgg_register_plugin_hook_handler('search', 'tags', 'elgg_solr_tag_search');
//elgg_register_plugin_hook_handler('search', 'tags', 'elgg_solr_tag_search');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we may need that later (i'm not sure) it just searches for tags

@@ -11,10 +11,12 @@
*/
function elgg_solr_file_search($hook, $type, $value, $params) {

$set_language = get_language();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing wrong, just sounds weird.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$set_language has been set to $get_language

Copy link
Contributor

@Phanoix Phanoix Jul 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exactly, no problem with the logic or anything just reads a little weird. One might expect set_ and get_ language to both be functions at a quick glance. Up to you if you want to do anything with that.

// get highlighting component and apply settings
$hl = $query->getHighlighting();
$hlfields = array('name', 'username', 'description');
$hlfields = array('text_en');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only 'en' here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed to $hlfields = array("text_{$get_language}");

@Phanoix
Copy link
Contributor

Phanoix commented Jul 23, 2018

This is completely unrelated to / does not affect the connex side and is only for the collab-related solr agent right?

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

Successfully merging this pull request may close these issues.

3 participants