? Success

User tests: Successful: Unsuccessful:

avatar miguelbgouveia
miguelbgouveia
8 Mar 2016

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.

avatar miguelbgouveia miguelbgouveia - open - 8 Mar 2016
avatar miguelbgouveia miguelbgouveia - change - 8 Mar 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 8 Mar 2016
Labels Added: ?
avatar andrepereiradasilva
andrepereiradasilva - comment - 8 Mar 2016

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

avatar brianteeman brianteeman - change - 8 Mar 2016
Category Search
avatar miguelbgouveia
miguelbgouveia - comment - 9 Mar 2016

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.

avatar andrepereiradasilva
andrepereiradasilva - comment - 9 Mar 2016

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

avatar andrepereiradasilva andrepereiradasilva - test_item - 9 Mar 2016 - Tested unsuccessfully
avatar andrepereiradasilva
andrepereiradasilva - comment - 9 Mar 2016

I have tested this item :red_circle: 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.
avatar miguelbgouveia
miguelbgouveia - comment - 10 Mar 2016

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.

avatar andrepereiradasilva
andrepereiradasilva - comment - 10 Mar 2016

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

avatar miguelbgouveia
miguelbgouveia - comment - 10 Mar 2016

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.
avatar andrepereiradasilva
andrepereiradasilva - comment - 10 Mar 2016

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.

avatar chrisdavenport
chrisdavenport - comment - 10 Mar 2016

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

avatar miguelbgouveia
miguelbgouveia - comment - 10 Mar 2016

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.
avatar miguelbgouveia
miguelbgouveia - comment - 10 Mar 2016

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.

avatar andrepereiradasilva andrepereiradasilva - test_item - 10 Mar 2016 - Tested successfully
avatar andrepereiradasilva
andrepereiradasilva - comment - 10 Mar 2016

I have tested this item :white_check_mark: 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.

avatar chrisdavenport
chrisdavenport - comment - 10 Mar 2016

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

avatar miguelbgouveia
miguelbgouveia - comment - 11 Mar 2016

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?

avatar miguelbgouveia
miguelbgouveia - comment - 11 Mar 2016

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?

avatar andrepereiradasilva
andrepereiradasilva - comment - 11 Mar 2016

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

avatar chrisdavenport
chrisdavenport - comment - 11 Mar 2016

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

avatar joomla-cms-bot
joomla-cms-bot - comment - 15 Mar 2016

This PR has received new commits.

CC: @andrepereiradasilva


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

avatar joomla-cms-bot
joomla-cms-bot - comment - 15 Mar 2016

This PR has received new commits.

CC: @andrepereiradasilva


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

avatar joomla-cms-bot
joomla-cms-bot - comment - 15 Mar 2016

This PR has received new commits.

CC: @andrepereiradasilva


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

avatar joomla-cms-bot
joomla-cms-bot - comment - 15 Mar 2016

This PR has received new commits.

CC: @andrepereiradasilva


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

avatar joomla-cms-bot
joomla-cms-bot - comment - 15 Mar 2016

This PR has received new commits.

CC: @andrepereiradasilva


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

avatar joomla-cms-bot
joomla-cms-bot - comment - 15 Mar 2016

This PR has received new commits.

CC: @andrepereiradasilva


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

avatar miguelbgouveia
miguelbgouveia - comment - 15 Mar 2016

I think I finished the PR @andrepereiradasilva. 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 using the category form (changing in the list it was already working).
  • 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 test should be repeated for contacts and newsfeeds. Just substitute articles with contacts and newsfeeds in the test text.

avatar andrepereiradasilva
andrepereiradasilva - comment - 15 Mar 2016

If i follow your steps exactly for articles/categories your test works.

But, if instead of changing the status to unpublished, i change the access level of the category to "Super user" it still finds the article in the frontend (i'm not logged in).

image

avatar miguelbgouveia
miguelbgouveia - comment - 28 Mar 2016

@chrisdavenport I found a small issue with your test plan for Smart Search.
In the content item state changes and category state change it should mention that we should test the change when it is made in the list view and also when it is made in the form. This is necessary because there are two different functions that deal with this two ways of changing the state. I try to propose this change in the test plan it self but I have not permissions for that.

avatar miguelbgouveia miguelbgouveia - change - 29 Mar 2016
Title
[com_finder] Error in reindex when category access change
[com_finder] Error in reindex when category state change
avatar joomla-cms-bot
joomla-cms-bot - comment - 29 Mar 2016

This PR has received new commits.

CC: @andrepereiradasilva


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

avatar joomla-cms-bot
joomla-cms-bot - comment - 29 Mar 2016

This PR has received new commits.

CC: @andrepereiradasilva


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

avatar joomla-cms-bot
joomla-cms-bot - comment - 29 Mar 2016

This PR has received new commits.

CC: @andrepereiradasilva


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

avatar joomla-cms-bot
joomla-cms-bot - comment - 29 Mar 2016

This PR has received new commits.

CC: @andrepereiradasilva


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

avatar joomla-cms-bot
joomla-cms-bot - comment - 29 Mar 2016

This PR has received new commits.

CC: @andrepereiradasilva


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

avatar joomla-cms-bot
joomla-cms-bot - comment - 29 Mar 2016

This PR has received new commits.

CC: @andrepereiradasilva


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

avatar miguelbgouveia
miguelbgouveia - comment - 29 Mar 2016

This last commit is to correct the issue that when a content was in the state of archive it was not showing in the results of the smart search. The archived content should be showed as said in the tests plans of @chrisdavenport. The problem of the access level is a more complex one that I also want to correct. I don't know if I, or another person, will have time to correct it in a near future. So, I suggest that we move it to another pull request. This way at least we will have the category state changes correct. What do you think @andrepereiradasilva?

avatar andrepereiradasilva
andrepereiradasilva - comment - 29 Mar 2016

ok by me, but i'm just a volunteer.

avatar miguelbgouveia
miguelbgouveia - comment - 29 Mar 2016

What more actions can I do to this correction be accept in some future version of Joomla?

avatar andrepereiradasilva
andrepereiradasilva - comment - 29 Mar 2016

Two people have to test successfully, them it's up to the maintainers decide when to merge it.

avatar joomla-cms-bot
joomla-cms-bot - comment - 4 Apr 2016

This PR has received new commits.

CC: @andrepereiradasilva


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

avatar joomla-cms-bot
joomla-cms-bot - comment - 4 Apr 2016

This PR has received new commits.

CC: @andrepereiradasilva


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

avatar joomla-cms-bot
joomla-cms-bot - comment - 4 Apr 2016

This PR has received new commits.

CC: @andrepereiradasilva


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

avatar joomla-cms-bot
joomla-cms-bot - comment - 4 Apr 2016

This PR has received new commits.

CC: @andrepereiradasilva


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

avatar joomla-cms-bot
joomla-cms-bot - comment - 4 Apr 2016

This PR has received new commits.

CC: @andrepereiradasilva


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

avatar joomla-cms-bot
joomla-cms-bot - comment - 4 Apr 2016

This PR has received new commits.

CC: @andrepereiradasilva


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

avatar joomla-cms-bot
joomla-cms-bot - comment - 4 Apr 2016

This PR has received new commits.

CC: @andrepereiradasilva


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

avatar joomla-cms-bot
joomla-cms-bot - comment - 4 Apr 2016

This PR has received new commits.

CC: @andrepereiradasilva


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

avatar miguelbgouveia
miguelbgouveia - comment - 4 Apr 2016

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.

avatar superknutsel superknutsel - test_item - 25 Jun 2016 - Tested successfully
avatar superknutsel
superknutsel - comment - 25 Jun 2016

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.

avatar abuechert abuechert - test_item - 2 Aug 2016 - Tested unsuccessfully
avatar abuechert
abuechert - comment - 2 Aug 2016

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.

avatar kevinscheithauer kevinscheithauer - test_item - 2 Aug 2016 - Tested unsuccessfully
avatar kevinscheithauer
kevinscheithauer - comment - 2 Aug 2016

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.

avatar roland-d
roland-d - comment - 2 Aug 2016

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

avatar joomla-cms-bot joomla-cms-bot - change - 22 Aug 2016
Category Search Administration Components Plugins Front End Search
avatar miguelbgouveia
miguelbgouveia - comment - 22 Aug 2016

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

avatar andrepereiradasilva
andrepereiradasilva - comment - 22 Aug 2016

you need to fix the conflicts

avatar miguelbgouveia
miguelbgouveia - comment - 22 Aug 2016

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

avatar andrepereiradasilva
andrepereiradasilva - comment - 22 Aug 2016

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.

avatar zero-24
zero-24 - comment - 22 Aug 2016

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

git checkout
git pull git@github.com:/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

avatar miguelbgouveia
miguelbgouveia - comment - 22 Aug 2016

@zero-24 I try git pull git@github.com:/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.

avatar miguelbgouveia miguelbgouveia - change - 22 Aug 2016
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2016-08-22 14:21:35
Closed_By miguelbgouveia
avatar miguelbgouveia miguelbgouveia - close - 22 Aug 2016
avatar miguelbgouveia miguelbgouveia - change - 22 Aug 2016
Status Closed New
Closed_Date 2016-08-22 14:21:35
Closed_By miguelbgouveia
avatar miguelbgouveia miguelbgouveia - change - 22 Aug 2016
Status New Pending
avatar miguelbgouveia miguelbgouveia - reopen - 22 Aug 2016
avatar mbabker
mbabker - comment - 22 Aug 2016

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.

avatar zero-24
zero-24 - comment - 22 Aug 2016

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

avatar zero-24
zero-24 - comment - 22 Aug 2016

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

avatar mbabker
mbabker - comment - 22 Aug 2016

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.

avatar zero-24
zero-24 - comment - 22 Aug 2016
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 6 Jan 2017

@miguelbgouveia is this PR still for testing?

avatar miguelbgouveia
miguelbgouveia - comment - 6 Jan 2017

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

avatar brianteeman
brianteeman - comment - 21 May 2017

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

avatar brianteeman brianteeman - change - 21 May 2017
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2017-05-21 17:40:52
Closed_By brianteeman
avatar brianteeman brianteeman - close - 21 May 2017
avatar joomla-cms-bot joomla-cms-bot - change - 21 May 2017
Category Search Administration Components Plugins Front End Administration com_finder Front End Plugins Search Components

Add a Comment

Login with GitHub to post a comment