apostrophe icon indicating copy to clipboard operation
apostrophe copied to clipboard

3.0: Multiple search issues

Open myovchev opened this issue 3 years ago • 9 comments

Describe the bug

  1. The task node app @apostrophecms/search:index fails with TypeError: self.indexDoc is not a function
  2. Pieces types are not registered programmatically as stated in the source documentation even with explicit searchable: true settings.
  3. I'm unable to include a desired field in the search index even via explicit searchable: true field option (tested with string textarea field type).
  4. The implementation of addSearchTexts disallows any weight configuration, and there are valid cases when a specific area text field might be of type highSearchText. I haven't tested if the rich-text widgets are searchable yet, and the above is obviously not a bug.
  5. The silent option for fields is not documented anywhere and it's unclear how it would affect the search. It's definitely not a part of the search result UI, my guess is it has something to do with API result set? (again not a search related bug report)

Expected behavior

  1. It looks to be a typo. indexDoc is defined in the handlers configuration and not in methods.
  2. I'm unable to find a registered handler for the determineTypes event in the pieces source but only dedicated method self.searchDetermineTypes(types) which in turn is not invoked anywhere (but I might miss something here). The only way to enable a piece for a search is via the explicit types option of the search module.
  3. There is a trace to this feature in the schema module (indexFields method) and string field type has a dedicated index method.

Details

Apostrophe v3.0.1

myovchev avatar Jun 26 '21 14:06 myovchev

Yes, we have some work to do here. The search mechanism is working well enough for inline search in the manage view but we need to go back and QA it from a standpoint of sitewide search. The indexDoc thing definitely sounds like it should have been a method so it could be called from this task and we just missed it.

As for incorporating your own fields directly into the mongodb index, I'm not sure if we'll be reworking that, but I take your point that it would be useful to be able to declare that a specific area's text belongs in highSearchText, especially since that is essential if you want it to be available for autocomplete purposes. This would be secondary to mopping up the clear-cut bugs you pointed out.

boutell avatar Jun 28 '21 15:06 boutell

Thanks Tom, it's fair enough. The most critical issues are related with enabling fields and pieces not added to the search index. The rest could come in v4 :D

myovchev avatar Jun 28 '21 17:06 myovchev

  • [x] The task node app @apostrophecms/search:index fails with TypeError: self.indexDoc is not a function
  • [x] Pieces types are not registered programmatically as stated in the source documentation even with explicit searchable: true settings.

I've resolve those two in an upcoming PR.

@boutell Were you saying that searchable (and silent maybe as well) should not be expected to operate as the code and code comments suggest? They're deprecated A2 features? I'm finding the same thing as @myovchev. Probably the same question for addSearchTexts since it's in the code, though I haven't tested that yet.

abea avatar Dec 02 '21 18:12 abea

Ah, no, searchable and silent should be expected to work, although I haven't reviewed the implementation.

On Thu, Dec 2, 2021 at 1:15 PM Alex Bea @.***> wrote:

  • The task node app @apostrophecms/search:index fails with TypeError: self.indexDoc is not a function
  • Pieces types are not registered programmatically as stated in the source documentation even with explicit searchable: true settings.

I've resolve those two in an upcoming PR.

@boutell https://github.com/boutell Were you saying that searchable (and silent maybe as well) should not be expected to operate as the code and code comments suggest? They're deprecated A2 features? I'm finding the same thing as @myovchev https://github.com/myovchev. Probably the same question for addSearchTexts since it's in the code, though I haven't tested that yet.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/apostrophecms/apostrophe/issues/3193#issuecomment-984876939, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27OKCSKN76YTI3USPE3UO6ZSNANCNFSM47LNKJYQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

--

THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

boutell avatar Dec 02 '21 18:12 boutell

searchable is mostly about opting out as I recall.

On Thu, Dec 2, 2021 at 1:54 PM Tom Boutell @.***> wrote:

Ah, no, searchable and silent should be expected to work, although I haven't reviewed the implementation.

On Thu, Dec 2, 2021 at 1:15 PM Alex Bea @.***> wrote:

  • The task node app @apostrophecms/search:index fails with TypeError: self.indexDoc is not a function
  • Pieces types are not registered programmatically as stated in the source documentation even with explicit searchable: true settings.

I've resolve those two in an upcoming PR.

@boutell https://github.com/boutell Were you saying that searchable (and silent maybe as well) should not be expected to operate as the code and code comments suggest? They're deprecated A2 features? I'm finding the same thing as @myovchev https://github.com/myovchev. Probably the same question for addSearchTexts since it's in the code, though I haven't tested that yet.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/apostrophecms/apostrophe/issues/3193#issuecomment-984876939, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27OKCSKN76YTI3USPE3UO6ZSNANCNFSM47LNKJYQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

--

THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

--

THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

boutell avatar Dec 02 '21 18:12 boutell

I don't know the cause of searchable and silent not working for string fields, areas, etc. as expected, but they should work and I do consider it a bug if they don't. Hoping that was the question.

boutell avatar Dec 02 '21 19:12 boutell

@boutell I'm not sure how it's related, but there is also (maybe legacy) inconsistency I've found while digging around to find a way to add high search text to an article coming from related tags titles. My obvious choice was an event, so I've found doc-type listening to this one ('@apostrophecms/search:index'), which is exactly what I need: https://github.com/apostrophecms/apostrophe/blob/7d728dd4b8443a6649c30e1f33abf7a3dc08f9e5/modules/%40apostrophecms/doc-type/index.js#L222 The thing is this event isn't triggered at all - there is no trace of emit('index') in the search module. Furthermore, search module is directly hooking into doc-type via beforeSave method (which is the problematic function indexDoc() which should become a method because it's used by the index task): https://github.com/apostrophecms/apostrophe/blob/7d728dd4b8443a6649c30e1f33abf7a3dc08f9e5/modules/%40apostrophecms/search/index.js#L100

The summary - currently only the beforeSave event handler of the search module is executed, and the index event listener in doc-type is not. The latter I think is responsible for registering the index data from the doc fields via the schema manager.

myovchev avatar Dec 07 '21 14:12 myovchev

'@apostrophecms/search:index' matches an A2 docSearchIndex event that looks left out of getSearchTexts in A3. Maybe that simply needs to be added back.

@myovchev The indexDoc issue should be resolved in tomorrow's release through my PR #3573

abea avatar Dec 07 '21 15:12 abea

Interesting enough, my second thought was to extend (improve) getSearchTexts in order to add my precious tag titles to the article search index :) It seems if this is a desired behavior of the search index event, the solution is simple. I'll just wait for a repair :)

myovchev avatar Dec 07 '21 16:12 myovchev