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

[Smart Search] Fix for tags issue #10122 #12745

Merged
merged 1 commit into from
Nov 15, 2016
Merged

Conversation

chrisdavenport
Copy link
Contributor

@chrisdavenport chrisdavenport commented Nov 4, 2016

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.

@absillitoe
Copy link

Hi Chris, Brian,

Please excuse my ignorance, but what does this actually mean in terms of obtaining a solution to this problem.

@chrisdavenport
Copy link
Contributor Author

It fixes it. Apply PR, purge, re-index, problem gone. (Actually, you probably don't need to purge).
Don't forget to report your test results. We need at least two positive tests before the PR can be merged.

@absillitoe
Copy link

absillitoe commented Nov 4, 2016

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

@chrisdavenport
Copy link
Contributor Author

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. 😄

@alikon
Copy link
Contributor

alikon commented Nov 6, 2016

I have tested this item ✅ successfully on a0fdae6

followed the Testing Instructions described at #10122


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12745.

@absillitoe
Copy link

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

@brianteeman
Copy link
Contributor

I have tested this item ✅ successfully on a0fdae6

Confirmed the issue as described before the PR

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.

@brianteeman
Copy link
Contributor

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12745.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Nov 15, 2016
@brianteeman brianteeman added this to the Joomla 3.7.0 milestone Nov 15, 2016
@rdeutz rdeutz merged commit af8fbde into joomla:staging Nov 15, 2016
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Nov 15, 2016
@peterstewart
Copy link

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...

@chrisdavenport chrisdavenport deleted the ss-tags branch April 15, 2017 21:02
@chrisdavenport
Copy link
Contributor Author

@peterstewart Please open a new issue with detailed instructions on how to reproduce the problem. Thanks.

@peterstewart
Copy link

@chrisdavenport
sorry for the delay; very busy with the job that this issue arose on and had to find a workround (abandoned my custom filter). I'm new here and can't figure out how to 'open a new issu'; I mudsst be missing something

@ghost
Copy link

ghost commented Apr 21, 2017

@peterstewart on top of this Page change to Tab "Issues", there on right green Button "New issue".

@peterstewart
Copy link

That's what I expected, but no button appeared when I tried it...
image

@ghost
Copy link

ghost commented Apr 21, 2017

Thats meant:
bildschirmfoto 2017-04-21 um 11 02 56

@zero-24
Copy link
Contributor

zero-24 commented Apr 21, 2017

you can also use this link: https://github.com/joomla/joomla-cms/issues/new

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

Successfully merging this pull request may close these issues.

None yet

8 participants