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

[4.4] Smart Search: Properly sort filters #42835

Merged
merged 4 commits into from
Mar 5, 2024

Conversation

Hackwar
Copy link
Member

@Hackwar Hackwar commented Feb 19, 2024

Pull Request for Issue #41930.

Summary of Changes

The entries in the filters in Smart Search aren't properly ordered in the frontend.

Testing Instructions

  1. Create a website with some content, especially nested categories. Best solution would be to use the testing sampledata.
  2. Go to the Smart Search search view in the frontend and open the advanced filters
  3. Check for example the category filter

Actual result BEFORE applying this Pull Request

The categories are pretty much random.

Expected result AFTER applying this Pull Request

The categories are displayed properly like in the tree structure.

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@ceford
Copy link
Contributor

ceford commented Feb 20, 2024

I have tested this item ✅ successfully on 55139c2


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

@richard67
Copy link
Member

richard67 commented Feb 20, 2024

@Hackwar Shouldn’t this be fixed in the 4.4-dev branch? The issue was reported for 4.3.

@Hackwar
Copy link
Member Author

Hackwar commented Feb 20, 2024

@richard67 See the last discussion we had about this. Nothing has changed since then.

@richard67
Copy link
Member

@richard67 See the last discussion we had about this. Nothing has changed since then.

@Hackwar So you think you as the JBS Team Lead and a member of the CMS Maintenance Team can do as you like and ignore all our policies?

@Hackwar
Copy link
Member Author

Hackwar commented Feb 20, 2024

I'm not ignoring policies. However I refuse to waste my time arguing if something is a bug and worthy of fixing in Joomla 4.4 and rather take the shortcut of providing that fix to 5.0+. Since the relevant maintainer decided to ignore this for over a week now as well, I'm assuming that it is fine the way I'm doing it.

@Hackwar Hackwar added the PBF Pizza, Bugs and Fun label Feb 20, 2024
@fgsw
Copy link

fgsw commented Feb 21, 2024

I have tested this item ✅ successfully on 55139c2


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

@richard67 richard67 removed the PBF Pizza, Bugs and Fun label Feb 21, 2024
@richard67
Copy link
Member

RTC as it has 2 successful tests. Release managers pls. decide if it should go into 5.0-dev or 4.4-dev.


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Feb 21, 2024
@richard67 richard67 added the RMDQ ReleaseManagerDecisionQueue label Feb 21, 2024
@joomla-cms-bot joomla-cms-bot added the Language Change This is for Translators label Feb 21, 2024
@Hackwar Hackwar changed the base branch from 5.0-dev to 4.4-dev February 21, 2024 20:13
@Hackwar Hackwar changed the title [5.0] Smart Search: Properly sort filters [4.4] Smart Search: Properly sort filters Feb 21, 2024
@joomla-cms-bot joomla-cms-bot added PR-4.4-dev and removed Language Change This is for Translators labels Feb 21, 2024
@Hackwar Hackwar added PBF Pizza, Bugs and Fun and removed RTC This Pull Request is Ready To Commit labels Feb 21, 2024
@stimpsonjcat
Copy link

I've tested this on J5.1. I created some categories and subcategories in content and in contacts. I deliberately named some of the categories in contact and content categories the same.
At first it seemed to work. The sort order in the pull down menu is now correct. Is it intended that the duplicate category names from the two different components are the shown as one?

After some testing the search didn't work for content anymore. Even if I turn of all other search plugins, and leave content the only one turned on. I did rebuild the smart search index. The advanced search does show the content categories, but whatever search word I enter, there's no result. If I turn on search for contacts, the contacts will be found, but no articles will show up.

I revoked the patch, and still the content search doesn't work anymore.


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Feb 24, 2024
@richard67 richard67 removed PBF Pizza, Bugs and Fun PR-4.4-dev labels Feb 24, 2024
@richard67
Copy link
Member

Back to pending after rebase.


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

@adj9
Copy link

adj9 commented Feb 24, 2024

With J 5.1, the display after patching does not change.
I made a category and subcategory

##ACCESSIBILITY IDEA
I searched for the subcategory with its own prefix and found nothing, so I infer that the query does a search with the = operator. You can vary the search query with a
LIKE '%''++'%'?

@Hackwar
Copy link
Member Author

Hackwar commented Feb 24, 2024

@stimpsonjcat Your issues are unrelated to this PR. This PR only touches the order of the entries in the filters.

@adj9 That is already an option in the component configuration of Smart Search. Also,if you only have 2 categories, there is nothing which could change, since they aren't out of order at that moment.

@stimpsonjcat
Copy link

stimpsonjcat commented Feb 24, 2024

I did test it again. It seemed that my articles had disappeared on the pizza-bugs-and-fun server.
I did test it again in 5.1 alpha 3 and retested on the testserver of today. It works as expected. ✅

The only feature request I would suggest: if you have a top level category (i.e. red) and and a sublevel category (i.e. light red), I would expect, as an enduser, if I search in the top-level I would also find results in the sublevel.

@richard67
Copy link
Member

I did test it again. It seemed that my articles had disappeared on the pizza-bugs-and-fun server. I did test it again in 5.1 alpha 3 and retested on the testserver of today. It works as expected. ✅

@stimpsonjcat Could you use the blue "Test this" button in the issue tracker https://issues.joomla.org/tracker/joomla-cms/42835 to submit your test result? Otherwise it is not counted. Thanks in advance.

@stimpsonjcat
Copy link

I have tested this item ✅ successfully on 4a8e9a1

Created multiple categories and subcategories in content and in contacts.
Added some articles and contacts. Some contacts have the same word in the name as in the article. (I've written an article on Stefanie and added a contact named Stefanie)
In the frontend searched on Stefanie and clicked on the advanced search. The category dropdown now shows the categories in the correct alphabetical order and hierarchy. The categories with the same names in content and contacts are shown as one category.


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

@eddiekonczal
Copy link

I have tested this item ✅ successfully on 4a8e9a1


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

@Quy
Copy link
Contributor

Quy commented Feb 24, 2024

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Feb 24, 2024
@laoneo laoneo added the Updates Requested Indicates that this pull request needs an update from the author and should not be tested. label Mar 4, 2024
Co-authored-by: Allon Moritz <allon.moritz@digital-peak.com>
@MacJoom
Copy link
Contributor

MacJoom commented Mar 4, 2024

Just to be safe: can we have another test here?

@laoneo
Copy link
Member

laoneo commented Mar 4, 2024

Back to pending


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

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 4, 2024
@laoneo laoneo removed the Updates Requested Indicates that this pull request needs an update from the author and should not be tested. label Mar 4, 2024
@ceford
Copy link
Contributor

ceford commented Mar 4, 2024

No good! With the patch applied I see the list in order but part of the Sample Data Articles is repeated.


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

@Hackwar
Copy link
Member Author

Hackwar commented Mar 4, 2024

Why do you want another test for this?

@Hackwar
Copy link
Member Author

Hackwar commented Mar 4, 2024

@ceford can you please be a bit more specific? What do you see repeated where?

@ceford
Copy link
Contributor

ceford commented Mar 4, 2024

image

@stimpsonjcat
Copy link

I'm getting errors in Joomla 5.1.0-alpha4:

Warning: Undefined property: stdClass::$id in /administrator/components/com_finder/src/Indexer/Result.php on line 576

Warning: Attempt to read property "state" on array in /administrator/components/com_finder/src/Indexer/Result.php on line 577

Warning: Attempt to read property "access" on array in /administrator/components/com_finder/src/Indexer/Result.php on line 578

@richard67
Copy link
Member

I'm getting errors in Joomla 5.1.0-alpha4:

@stimpsonjcat How is this related to this PR here which is clearly made for 4.4-dev and not 5.1-dev?

@ceford
Copy link
Contributor

ceford commented Mar 4, 2024

I have tested this item ✅ successfully on ded674e

Oops - my previous comments were for J5. J4 Seems to work fine.


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

@laoneo
Copy link
Member

laoneo commented Mar 5, 2024

As we added only a type to a function argument, one test is enough. So merging it as it was RTC before.

@laoneo laoneo merged commit ab3d32c into joomla:4.4-dev Mar 5, 2024
3 checks passed
@laoneo
Copy link
Member

laoneo commented Mar 5, 2024

Thanks!

@laoneo laoneo added this to the Joomla! 4.4.4 milestone Mar 5, 2024
@Hackwar Hackwar deleted the 5.0-finder-filter branch March 9, 2024 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug PBF Pizza, Bugs and Fun PR-4.4-dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.