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

Projects
None yet
10 participants
@smehrbrodt
Copy link
Contributor

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.

Fix #20072 Allow searching article content in backend
Introduced a 'content:' prefix to search the article content
@laoneo

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor Author

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

Ok, have updated the patch accordingly.

@ggppdk

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor

brianteeman commented Apr 5, 2018

@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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Contributor Author

smehrbrodt commented Apr 5, 2018

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor Author

smehrbrodt commented Apr 6, 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.

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

This comment has been minimized.

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

@laoneo

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor Author

smehrbrodt commented Apr 9, 2018

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

This comment has been minimized.

Copy link
Contributor

brianteeman commented Apr 9, 2018

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

This comment has been minimized.

@brianteeman

brianteeman Apr 9, 2018

Contributor

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

This comment has been minimized.

Copy link
Contributor

tonypartridge commented Apr 12, 2018

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

This comment has been minimized.

Copy link
Contributor

brianteeman commented May 8, 2018

restarted drone

@brianteeman

This comment has been minimized.

Copy link
Contributor

brianteeman commented May 8, 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.

1 similar comment
@Quy

This comment has been minimized.

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

This comment has been minimized.

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 label May 9, 2018

@infograf768

This comment has been minimized.

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

This comment has been minimized.

Copy link
Member

infograf768 commented May 9, 2018

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 label May 9, 2018

@smehrbrodt

This comment has been minimized.

Copy link
Contributor Author

smehrbrodt 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

Good catch, fixed now!

@infograf768

This comment has been minimized.

Copy link
Member

infograf768 commented May 9, 2018

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

This comment has been minimized.

Copy link
Contributor

brianteeman commented May 9, 2018

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

This comment has been minimized.

Copy link
Member

infograf768 commented May 9, 2018

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

@mbabker mbabker added PR-3.9-dev and removed PR-staging labels May 12, 2018

@mbabker mbabker merged commit 70f419a into joomla:3.9-dev May 12, 2018

2 of 4 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
Hound No violations found. Woof!
continuous-integration/drone/pr the build was successful
Details

@joomla-cms-bot joomla-cms-bot removed the RTC label May 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.