-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[Smart Search] Fix for tags issue #10122 #12745
Conversation
Hi Chris, Brian, Please excuse my ignorance, but what does this actually mean in terms of obtaining a solution to this problem. |
It fixes it. Apply PR, purge, re-index, problem gone. (Actually, you probably don't need to purge). |
Hi again Chris, You need to treat me as someone totally ignorant! This is the first time I find myself in GitHub. Where do I find the "PR" and how do I apply it? At the moment, I only see your Summary of Changes. Alan |
No problem Alan. PR = "pull request", which is essentially just a proposal for a bunch of changes to the code. You can see the changes I'm proposing to fix this specific problem by clicking on "Files changed" near the top of this page. The easiest way to test a PR is to use the patch tester component. It makes it really simple for someone, even with no coding knowledge, to test proposed changes to the code. There is documentation on using the patch tester and how to report your results, here: https://docs.joomla.org/Testing_Joomla!_patches Once we have at least two tests that confirm that the PR fixes the problem (and doesn't cause any other problems!) it will be flagged as RTC = "Ready to commit". All being well, the changes will then be merged into the codebase and included in the next Joomla release. Please feel free to ask questions, and welcome to the Joomla community. It's great to have you on board. 😄 |
I have tested this item ✅ successfully on a0fdae6 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12745. |
Hi again Chris, Thank you for your explanation. I installed the latest stable release of Joomla in my local XAMPP installation and tested that Smart Search gave no result for a new tag after indexing. I then applied the patch, made a new tag, applied it to an article and indexed again. Effectively, Smart Search then produces a result. What I don't understand is that the result of the Smart Search is the tag and not the article. If I then click on the tag, I get the article(s). Is this normal Joomla behaviour? If so, then it appears to be working correctly. Actually, however, if I do an advanced search only on articles, it does not bring up the article(s). I should add that I'm using the Smart Search module. Am I perhaps doing something wrong? Alan |
I have tested this item ✅ successfully on a0fdae6 Successfully tested - I had to re-index the content after applying the PR This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12745. |
RTC This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12745. |
I have been testing this in Joomla 3.6.5 and it works fine until I use a custom filter; I then edited the filter until it was identical to the default; the result was the same; tags were not found that were found with the default filter. It's beyond me at present to explore why this should be... |
@peterstewart Please open a new issue with detailed instructions on how to reproduce the problem. Thanks. |
@chrisdavenport |
@peterstewart on top of this Page change to Tab "Issues", there on right green Button "New issue". |
you can also use this link: https://github.com/joomla/joomla-cms/issues/new |
Pull Request for Issue #10122 .
Summary of Changes
Smart Search was not retrieving com_tags items in search results. This turned out to be due to the publish_down date being set to the current date whenever a tag is saved. Hence, Smart Search would not retrieve any com_tags items because it saw them as no longer published.
As far as I can tell, the publish_up and publish_down fields are not actually used in the com_tags component. At least, they don't appear anywhere in the UI so there's no way to change them. Not sure whether they should simply be removed from com_tags or implemented properly. Anyway, this PR fixes the issue by simply ignoring those fields when constructing the search index.
I have also taken the opportunity of adding a call to FinderIndexerHelper::getContextExtras which will be required if anyone ever needs to extend the index for com_tags items. I think the new com_fields makes use of it, for example.
Testing Instructions
See #10122 which describes the problem with screenshots.
Documentation Changes Required
None.