[com_finder] Error in reindex when category state change

[miguelbgouveia]

Pull Request for Issue # .

Summary of Changes

Testing Instructions

When the category state of a content change it was not propagated to the index of the smart search.

[andrepereiradasilva]

@miguelbgouveia can you make more clear test instrutions?
like step 1, 2. 3. how it behaves before, how it behaves after?

For instance, i've never used com_finder before and have no idea what this PR is supposed to do and how to test it.

[miguelbgouveia]

The com_finder is used to create a index of words in order to use it in smart search way. We start writing some letters in the search filed and the smart search show some possible words that the user can chose. The words are from the previous index created.

To index is created based in the data associated to the com_finder. For example: articles, contacts, etc, can be associated to the smart search. For each type of item we have to activate the corresponding plugin.

When there is some change in the items to index there is some events that fire and that are responsible to actualize the index. One of that cases is when we change the state of a category of one of those elements. The problem founded here was that when we change the state of category of a item that is in the index, the function that handle this change was not working correctly. This function should reindex all the items related to the category but by mistake it was taken the wrong identifier of the items. Instead of taken the item identifier to reindex, it was taken the category identifier.

To test this you should create, for example, an article with the state published, with a specific word in is title (for example 'word_to_index') and associate it to a category that also as the state state published. Then try to find the article using the word 'word_to_index' in the main search field. It should show the article in the search results. Now try to unpublished the article and make the same search. You will see that the article still appear but it shouldn't. After make this correction and repeating all the steps described before, the article with the unpublished state will not appear this time.

Note: You should change the article category state in the edit form in the administration site. You have to also change the category access beside with the state change in order to fire the reindex of the items. This is another problem with this component that I will try to correct later.

[andrepereiradasilva]

ok thanks for the explanation and test instructions, will test when i have time

[andrepereiradasilva]

I have tested this item 🔴 unsuccessfully on cd5ee81

Steps done:

• Created a category "Category com_finder" (Public and published)
• Created an article "Article com_finder word_to_index" (Public and published)
• Enabled the "Content - Smart Search" plugin
• Pressed "Index" in smart search component (com_finder)
• Created a menu item "Finder" to Smart Search
• Used previously created "Finder" menu item in the frontend and tried to search for "word_to_index" and the article appeared.
• Applied this patch
• Changed the article "Article com_finder word_to_index" to unpublished state
• Used previously created "Finder" menu item in the frontend and tried to search for "word_to_index" and the article still appeared in the results...

  ---

  _{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9346.}

[miguelbgouveia]

The result of your test are very strange because you are testing the change state of the article and I only change the change state of the category of the article. To test my pull request you should change the state of the category "Category com_finder" from published to unpublished and not the state of the article. Nevertheless, I will simulate your test, but it must pass because was not the part of the code that I change.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9346.}

[andrepereiradasilva]

sorry you are right, i was with cache enabled ....

[miguelbgouveia]

The test should be:

• Created a category "Category com_finder" (Public and published)
• Created an article "Article com_finder word_to_index" (Public and published)
• Enabled the "Content - Smart Search" plugin
• Pressed "Index" in smart search component (com_finder)
• Created a menu item "Finder" to Smart Search
• Used previously created "Finder" menu item in the frontend and tried to search for "word_to_index" and the article appeared.
• Applied this patch
• Changed the category "Category com_finder" to unpublished state
• Used previously created "Finder" menu item in the frontend and tried to search for "word_to_index" and the article should not appear in the results.

  ---

  _{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9346.}

[andrepereiradasilva]

Used 3.5.0 RC for the tests.

i think i see what you mean now, you have to change the category published status to unpublished on the edit page to detect the problem (not the button or checkbox in the list view, that already worked fine), but with or without your patch the behaviour is the same, in other words, doesn't remove it from the search results list.

[chrisdavenport]

Whilst this PR certainly looks correct, fixing it might serve to highlight a performance issue. If you publish/unpublish a category containing a lot of articles (where "a lot" depends on the server specifications) or many particularly large articles (with a similarly vague definition of "many" and "particularly large") you could be waiting a long time for a response. In some cases the PHP process may time-out before it completes.

I'm not saying we shouldn't fix it though. :-)

[miguelbgouveia]

Try to change the access filed at the same time as you change the state to unpublished. I think in this way may patch will work.

As I said before: "You should change the article category state in the edit form in the administration site. You have to also change the category access beside with the state change in order to fire the reindex of the items. This is another problem with this component that I will try to correct later."

In resume, my patch only resolve part of the problem.

So the tests should be:

• Created a category "Category com_finder" (Public and published)
• Created an article "Article com_finder word_to_index" (Public and published)
• Enabled the "Content - Smart Search" plugin
• Pressed "Index" in smart search component (com_finder)
• Created a menu item "Finder" to Smart Search
• Used previously created "Finder" menu item in the frontend and tried to search for "word_to_index" and the article appeared.
• Applied this patch
• Changed the category "Category com_finder" to unpublished state and at the same time change the access field (the two changes should go in the same transaction).
• Used previously created "Finder" menu item in the frontend and tried to search for "word_to_index" and the article should not appear in the results.

[miguelbgouveia]

You right @chrisdavenport. But I think who use this plugin must have the information that the system could lose in performance. But that doesn't mean that we should not resolve the errors.

[andrepereiradasilva]

I have tested this item ✅ successfully on cd5ee81

It solves the part of the reindex when changing a category state AND access level (at the same time) in edit view according to comments above.

IMHO, a PR like this should solve the whole issue, in other words, also solve the fact that is not reindexing when we only change the state of the category (in edit view).

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9346.}

[chrisdavenport]

Not sure if you will find it useful or not, but some years ago I tried to put together a test plan for Smart Search which would establish the expected behaviour. Your PR reminded me of it.

https://docs.joomla.org/Smart_Search_content_change_test_plan

[miguelbgouveia]

I create this PR because is was an error with an easy correction. I known that there is more work to do to correct all the management of category state change in the smart search. The problem is having time and knowledge to do the work, but I can try.

In your opinion @andrepereiradasilva, if I resolve the others issues you think is better to add new code to this PR or create another one?

[miguelbgouveia]

Thanks @chrisdavenport for the information. But this test plan was accepted by the majority of the joomla community or it is some plan that where not been validated? Is a good idea use this plan to guide me to the process of the smart search correction?

[andrepereiradasilva]

IMHO for the edit published state case problem to this PR. Other call have their PR.

[chrisdavenport]

@miguelbgouveia I wouldn't say it has any "official" status, whatever that means, but if you were to run through it and find a discrepancy I think it would be worth opening an issue in the tracker so it can be discussed.

But as @andrepereiradasilva said, let's not cloud this issue. This PR just needs one more test.

[joomla-cms-bot]

This PR has received new commits.

CC: @andrepereiradasilva

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9346.}

[joomla-cms-bot]

This PR has received new commits.

CC: @andrepereiradasilva

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9346.}

[joomla-cms-bot]

This PR has received new commits.

CC: @andrepereiradasilva

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9346.}

[joomla-cms-bot]

This PR has received new commits.

CC: @andrepereiradasilva

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9346.}

[joomla-cms-bot]

This PR has received new commits.

CC: @andrepereiradasilva

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9346.}

[joomla-cms-bot]

This PR has received new commits.

CC: @andrepereiradasilva

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9346.}

[joomla-cms-bot]

This PR has received new commits.

CC: @andrepereiradasilva

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9346.}

[miguelbgouveia]

This lasted changes is to correct a new founded problem. When we have a content belonging to a category that have more than one hierarchy level, when the state of the categories of the upper levels are changed this change have to be propagated to the contents belonging to the lower levels categories.

To test this we can create an hierarchy of categories with tree levels. For example: category 1, category 1.1 and category 1.1.1, all with the state equal to publish. Then create a content with the state equal to publish and in the category 1.1.1. The content must be created with a specific characters in the title, for example xyz. If we insert the xyz characters using the smart search the content should appear in the results. Changing the category 1 state to unpublished and searching again by the xyz characters the content now should not appear in the search results. We should test this scenario using the two category state editing mods. Changing the category sate in the list of categories and using the category edit form.

[superknutsel]

I have tested this item ✅ successfully on fcfc51e

Tested succesfully with articles and contacts and can confirm that the article/contact is no longer presented in the search results after the patch.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9346.}

[arbue]

I have tested this item 🔴 unsuccessfully on fcfc51e

Tested @icampus

followed the test steps with article and contact categories. Pre-Patch search finds the article/contact. After apply patch and changing the state to unpublished the article/contact still appears in the search results. Only after a reindexing it disappears from the search results.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9346.}

[kevinscheithauer]

I have tested this item 🔴 unsuccessfully on fcfc51e

I have tested this issue @icampus PBF with the proposed change and I get an error. I followed the suggested steps but I get an error "Warning: Header may not contain more than a single header, new line detected in C:\xampp\htdocs\bugtesting\libraries\joomla\application\web.php on line 961" when changing the category to unpublished after applying the patch.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9346.}

[roland-d]

@miguelbgouveia Can you update your PR please so the conflicts are fixed and we can test again? Thanks

[miguelbgouveia]

Now I think is ready to test. The problem was added in the last commit that I was made.

[andrepereiradasilva]

you need to fix the conflicts

[miguelbgouveia]

@andrepereiradasilva, how can I see where are the conflicts?

[andrepereiradasilva]

You can do it in a git cli interface, Github for Windows. Search the web.

Or you can manually check the recent history of the files you changed to see what changed and replicate those changes.

The bottom line is you need to fix conflicts and update your miguelbgouveia:patch-3 branch to latest staging.

[zero-24]

Do you know how to use git e.g. a local client via command line?

git checkout
git pull [email protected]:/joomla/joomla-cms.git staging
[now git tryes to sync and show the conflicted files]
[now you need to fix the conflicts]
Git commit -am 'fix conflicts'
Git push origin

[miguelbgouveia]

@zero-24 I try git pull [email protected]:/joomla/joomla-cms.git staging but I get this error:

Permission denied (publickey).
fatal: could not read from remote repository.

Please make sure you have the correct access rights and the repository exists.

[mbabker]

Use https://github.com/joomla/joomla-cms.git for the address instead of the git@ address. The former allows anyone to read from a repo, the latter requires write access.

[zero-24]

Did you enter you password correct? Maybe you can also try git pull upstream staging

[zero-24]

Are you sure @mbabker as it works for me without push acccess to the upstream cms repo.

[mbabker]

If you've got your full SSH environment set up right then generally it will. For read only operations though, you really don't need it and can get away with the https URL.

[zero-24]

https://help.github.com/articles/which-remote-url-should-i-use/

Thanks just found it.

[ghost]

@miguelbgouveia is this PR still for testing?

[miguelbgouveia]

I'm not more working with joomla CMS and I can't verify the correctness of this PR.

[brianteeman]

As this PR has been abandoned by the submitter and it is not testable or mergable I am going to have to close it at this time. It can always be reopened if updated