-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[com_fields] Fields aren't updated for Smart Search (finder) on item save #13338
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
Conversation
|
Both interfaces should be consistent with the loaded plugin groups and any inconsistency should be considered a minor bug. So it should include the system group. |
|
Okay, I think I've fixed the problem. |
|
I've now added some code to add certain field types to search taxonomies where appropriate. This code should really be moved to the individual field plugins once that PR has been merged, but for the time being it's in a big switch statement. For example, checkbox, list and radio fields are, as you would expect, added to a taxonomy (called a "content map" in the UI) with the name of the field and the relevant value(s). This will appear as a drop-down in the default module and component Smart Search views. Similarly, colour, integer, media, telephone, timezone, sql, user and usergroups fields are also added to taxonomies. We'll need to figure out how to do the translations, which don't currently work. |
|
I have tested this item 🔴 unsuccessfully on 42f4e48 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13338. |
|
@Hackwar Is this something you are addressing in your rewrite of com_finder? This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13338. |
|
@Hackwar gently reminder. |
|
Closing in favour of #36747 . |
Pull Request for Issue #12829 .
Summary of Changes
This doesn't fully fix the issue, but hopefully it's a step in the right direction.
I've added an
$extensionargument to the onPrepareFinderContent handler and theFinderIndexerHelper::getContentExtrasmethod becauseFieldsHelper::getFieldsneeds to know the extension name in order to retrieve the correct fields. Perhaps there's a better way? There's also an implied equivalence between the finder layout name and the second part of the field context variable. Is that assumption always valid? I'm not sure.I've added a prefix of "jfield_" to the field name saved in the index so as to avoid possible collisions between user-defined custom field aliases and existing fields in the content item.
The code assumes that all custom fields have values that are searchable strings (I don't know whether that's true) and that the terms will have the same weight as body text. I think it would be advisable to allow the user to determine whether a particular field is searchable and perhaps for some field types not to be searchable at all. At the moment, even numeric fields are going to be indexed. If a field is searchable, it would also be good to have the option of determining which weight multiplier group it belongs to.
As far as I can tell, the only reason this doesn't work right now is that the
FieldsHelper::getFieldscall in the onPrepareFinderContent method is returning the old custom field values rather than the new ones. Consequently, you need to save changes to content items twice before the index will correctly reflect the new field values. I thought at first that this might be because of the order in which the plugins fire, but that doesn't appear to be case asPlgSystemFields::onContentAfterSaveis called beforePlgContentFinder::onContentAfterSave(which cascades down to the onPrepareFinderContent handler).Note: The CLI indexer will not currently work with the onPrepareFinderContent handler in a system plugin because system plugins are not loaded by the CLI indexer. Personally, I would suggest moving onPrepareFinderContent into a finder plugin so all the finder events are handled within the same plugin group and I'll happily provide a PR to do that if folks agree. Otherwise, I'll do a PR to load the system plugins in the CLI indexer.
Testing Instructions
See #12829
Documentation Changes Required
None.