silverstripe / silverstripe-fulltextsearch

Adds external full text search engine support to Silverstripe
BSD 3-Clause "New" or "Revised" License
44 stars 83 forks source link

Incorrect docs on ShowInSearch #237

Closed chillu closed 4 years ago

chillu commented 5 years ago

https://github.com/silverstripe/silverstripe-fulltextsearch/blob/65b780f629ec31168902217d734079a14727896f/docs/en/03_configuration.md#adding-dataobjects says:

getShowInSearch() is required to get the record to show in search, since all results are filtered by ShowInSearch.

There's no functionality in the module to make this work. It's part of the CWP module. We should either document this as a recipe to implement in your search index, or (my preference) actually implement it in the default search index definitions we mention to developers.

See CWP implementation:

https://github.com/silverstripe/cwp-search/blob/057c873e8de4713b22ee72077bfe7ed41b0879cc/src/Solr/CwpSolrIndex.php#L27

PR

Extract from PR (merged): https://github.com/silverstripe/silverstripe-fulltextsearch/pull/275/files#diff-1959cad5d7c9eed2e0117a24d3f4da7eR175

brynwhyman commented 4 years ago

We should run this one past some of our internal teams to assert what impact this might have on existing implementations.

chillu commented 4 years ago

We should run this one past some of our internal teams to assert what impact this might have on existing implementations.

I've flagged this for SAs and PDs today: https://silverstripeltd.slack.com/archives/C052GD0J0/p1584589306051700

emteknetnz commented 4 years ago

Have updated docs https://github.com/silverstripe/silverstripe-fulltextsearch/pull/275/files#diff-1959cad5d7c9eed2e0117a24d3f4da7eR150

Cheddam commented 4 years ago

Resolved in #275.