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

Allow searching article content in backend #20083

Merged
merged 7 commits into from May 12, 2018

Conversation

smehrbrodt
Copy link
Contributor

@smehrbrodt smehrbrodt commented Apr 5, 2018

Pull Request for Issue #20072 .

Summary of Changes

Introduced a 'content:' prefix to search the article content

Testing Instructions

Go to Content->Articles in backend. Try searching for article content by prefixing your search with content:, e.g. content:foo.

Expected result

Articles which have the string "foo" in their content should be found.

Documentation Changes Required

Tooltip and maybe help needs to be updated.

Introduced a 'content:' prefix to search the article content
@laoneo
Copy link
Member

laoneo commented Apr 5, 2018

Searching in content should work without prefix. Nobody will know that you have to prefix it with "content:". I would leave that and just include to search in the article content.

@smehrbrodt
Copy link
Contributor Author

Searching in content should work without prefix. Nobody will know that you have to prefix it with "content:". I would leave that and just include to search in the article content.

Ok, have updated the patch accordingly.

@ggppdk
Copy link
Contributor

ggppdk commented Apr 5, 2018

This would be nice,
but it will have terrible performance on websites with a few thousand articles or more
even timeouts if you have 10,000+ articles

the clause LIKE %word% is OK on

  • a varchar 255 title column (or 400 for alias column) with a few backend users

but LIKE %word% on 2 longtext columns ?

And even if you use a real search index, and solve the performance issue
you will hit the UX problem

  • users 90% - 95% of the time would search for words that they want to find in the title

so if you enable this always it would be better to priotirize results and return matched articles having the words in the title first and then matches in the content

or add a selector
Search in title
Search in title and content

@csthomas
Copy link
Contributor

csthomas commented Apr 5, 2018

I suggest to replace prefixes with a new select box as @ggppdk mentioned.

Example:

<select name="searchIn">
  <option value="title">Search in Title</option>
  <option value="content">Search in Content</option>
  <option value="id">Search by Id</option>
  ...
</select>

@ggppdk
Copy link
Contributor

ggppdk commented Apr 5, 2018

Yes a selector exactly like @csthomas suggested will be good UX and will give this possibility to small websites without destroying performance on large websites (as searching in content with LIKE will be optional and not default behaviour)

@brianteeman
Copy link
Contributor

@laoneo at a minimum the search should only search the title by default. If it searches everything by default then it will effect performance. The prefix is the correct method. As is the ability to set the prefix in a select

@mbabker
Copy link
Contributor

mbabker commented Apr 5, 2018

IMO if these quick filters are going to start searching content and not just titles it needs to be consistent across core. I don't believe the intent of these filters was to ever be a full search similar to the frontend com_search and its plugins, and personally I'm not sure that we should make these filters behave as a full search, but if we're going to I'd suggest that the filters should most likely offer searching the same fields that the com_search plugins query at a minimum per component.

@smehrbrodt
Copy link
Contributor Author

Ok then I will go back to the prefix.

Putting all available prefixes in a select box is a good idea, but out of scope for this patch.
That should be done separately, and consistently for all components.

@laoneo
Copy link
Member

laoneo commented Apr 5, 2018

If you guys really want to have an explicit search in the full content, then I'm also in favor of @csthomas solution with a drop down which defines in what to search. We should stop adding hidden features which are not obvious and will never get documented.

@smehrbrodt
Copy link
Contributor Author

If you guys really want to have an explicit search in the full content, then I'm also in favor of @csthomas solution with a drop down which defines in what to search. We should stop adding hidden features which are not obvious and will never get documented.

I see no general problem with using prefixes for searching. I see it as a power user feature which I personally like very much (btw also available here on Github).

But creating a dropdown for this can still be done afterwards to make it more visible.

@Quy
Copy link
Contributor

Quy commented Apr 8, 2018

Update language string:
COM_CONTENT_FILTER_SEARCH_DESC="Search in title and alias. Prefix with ID: or AUTHOR: or CONTENT: to search for an article ID, article author or article content."

@joomla-cms-bot joomla-cms-bot added the Language Change This is for Translators label Apr 9, 2018
@laoneo
Copy link
Member

laoneo commented Apr 9, 2018

The difference between Github and Joomla is that on Github it is properly documented https://help.github.com/articles/searching-issues-and-pull-requests/ which is not the case for Joomla. So, if this feature gets ever into Joomla, only the people participating in this pr, the guy who merged it and you do know it.

Don't get me wrong, I really like the idea, but it should be done properly.

@smehrbrodt
Copy link
Contributor Author

The difference between Github and Joomla is that on Github it is properly documented which is not the case for Joomla.

I updated the tooltip which now describes the 3 available prefixes: id, author and content.
Is there any other places where this needs to be documented?

@brianteeman
Copy link
Contributor

if we stop using everything in joomla that isnt documented then there wont be much left ;)

@@ -118,7 +118,7 @@ COM_CONTENT_FIELDS_TYPE_MODAL_ARTICLE="Article"
COM_CONTENT_FIELDSET_PUBLISHING="Publishing"
COM_CONTENT_FIELDSET_RULES="Permissions"
COM_CONTENT_FIELDSET_URLS_AND_IMAGES="Images and Links"
COM_CONTENT_FILTER_SEARCH_DESC="Search in title and alias. Prefix with ID: or AUTHOR: to search for an article ID or article author."
COM_CONTENT_FILTER_SEARCH_DESC="Search in title and alias. Prefix with ID: or AUTHOR: or CONTENT: to search for an article ID, article author or article content."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you change this please to
+COM_CONTENT_FILTER_SEARCH_DESC="Search in title and alias. Prefix with ID: or AUTHOR: or CONTENT: to search for an article ID, article author or search in article content."

@tonypartridge
Copy link
Contributor

This is what I was talking about the other day when you did the issue, without prefix we also face a user issue. Users are not used to searching full content this could have a mass negative effect on the community.

Prefix is ok... but even I only found out about the ID prefix last week 😭.

Ideally we should leave as is, and add in additional filter options. Multi-select option I.e. title is selected by default, the. Allow description, custom fields, image etc.

@brianteeman
Copy link
Contributor

restarted drone

@brianteeman
Copy link
Contributor

I have tested this item ✅ successfully on f8d42f1


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

1 similar comment
@Quy
Copy link
Contributor

Quy commented May 9, 2018

I have tested this item ✅ successfully on f8d42f1


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

@Quy
Copy link
Contributor

Quy commented May 9, 2018

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label May 9, 2018
@infograf768
Copy link
Member

infograf768 commented May 9, 2018

Taking off RTC as this should also be implemented in the featured model as the change in the string is displayed there too
screen shot 2018-05-09 at 08 07 59

Please complete PR.

@infograf768
Copy link
Member

Waiting for PR to be completed


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

@infograf768 infograf768 removed the RTC This Pull Request is Ready To Commit label May 9, 2018
@smehrbrodt
Copy link
Contributor Author

Taking off RTC as this should also be implemented in the featured model as the change in the string is displayed there too

Good catch, fixed now!

@infograf768
Copy link
Member

I have tested this item ✅ successfully on 234bf55


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

1 similar comment
@brianteeman
Copy link
Contributor

I have tested this item ✅ successfully on 234bf55


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

@infograf768
Copy link
Member

Back to RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label May 9, 2018
@mbabker mbabker added this to the Joomla 3.9.0 milestone May 10, 2018
@mbabker mbabker changed the base branch from staging to 3.9-dev May 12, 2018 17:06
@mbabker mbabker merged commit 70f419a into joomla:3.9-dev May 12, 2018
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label May 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language Change This is for Translators
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants