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

Filter on multiple Access Levels, Authors, Categories, and Tags #5474

Closed
wants to merge 14 commits into from

Conversation

@jrseliga
Copy link

@jrseliga jrseliga commented Dec 20, 2014

Summary

Allows articles and featured articles to be filtered by multiple access levels, authors, categories, and tags in Administrator.

Current Joomla! behavior

The article/featured articles search (filter) tools allow one choice per input, each acting as a logical AND.

New behavior

  • Each input still acts as the logical AND.
  • Each choice within a input act as the logical OR with all the other choices inside of that input.

Notes

  • The module "Article - Newsflash" has an option to select multiple categories, when for example two categories are selected the module pulls all of the articles from both categories. Since articles can only exist in one category this is the only logical behavior, not only for this module, but for any instance of a multiple select category field. As a result, it could be counter intuitive to implement other multiple select filters in a different fashion.
  • The content model already supported filtering based on multiple categories before this pull request was made (see condition starting at line 244). This pull request just enables it's use in the search filters.
  • Similarly it seemed intuitive to introduce virtually identical code blocks (behavior when multiple are selected) for Author, Access Levels, and Tags.

Testing

Administrator

Articles
  1. Create 3 articles:
    1. Article 1 - Category 1 - Tag 1 - Author 1 - Public
    2. Article 2 - Category 2 - Tag 1 & Tag 2 - Author 2 - Registered
    3. Article 3 - Category 2 - Tag 2 - Author 1 - Super User
  2. View Articles
    1. Use Search Tools
      • Just with a simple structure like this, there is a large number of potential test cases. Please refer to the New behavior section for an overview of the filtering logic
Featured Articles
  1. View Featured Articles
    • Repeat in Featured Articles Search Tools

Site

Currently the only core feature that resembles the use case for this is in the module "Articles - Newsflash". If similar "Search Tools" are developed for filtering a list of articles in the front-end, it could leverage this feature in the same way as the back-end.

Articles - Newsflash (module)
  1. Leave Categories option at "- All Categories -" only
  2. Select one tag up to n tags. Any article with any of the n tags should be displayed.
    • This can be limited by selecting multiple categories.

Acknowledgements

Thank you to @brianteeman and @nikosdion for taking the time to help with my first pull request!

Allow articles to be filtered by multiple tags in Administrator
@brianteeman
Copy link
Member

@brianteeman brianteeman commented Dec 20, 2014

To test create 3 articles and 2 tags
Article 1 - tag 1
Article 2 - tag 1 AND tag 2
Article 3 - tag 2

Then use the search filter for tags to select the articles

Before the patch you can only select a single tag so a filter for tag 1 will return article 1 & 2
After the patch you can select multiple tags so so a filter for tag 1 will return article 1 & 2 AND
a so a filter for tag 1 & 2 will return article 2 & 3


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

@brianteeman
Copy link
Member

@brianteeman brianteeman commented Dec 20, 2014

OOPS My test instructions may wrong

I assumed that searching for two tags would return all articles tagged with BOTH
Instead it selected articles tagged with EITHER.

If that is the intended behaviour then it works - but perhaps users would be expecting to only see items tagged with BOTH

AND

The patch removes the label Select Tags see screenshots whatever the expected behaviour of the filter this needs fixing

BEFORE
screen shot 2014-12-20 at 04 00 47

AFTER
screen shot 2014-12-20 at 04 00 51


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

@nikosdion
Copy link
Contributor

@nikosdion nikosdion commented Dec 20, 2014

@test Successful

Regarding Brian's observations:

  • The search for multiple tags will return articles which belong to any of the tags. Looking for all tags would have a very adverse performance and memory impact.
  • The "Select an option" label can be changed, but it's tricky. See below.

To restore the "Select Tag" label in the field you need to make two changes:

In administrator/components/com_content/views/articles/tmpl/default.php add the line:

JHtml::_('formbehavior.chosen', '.advancedTags', null, array('placeholder_text_multiple' => JText::_('JOPTION_SELECT_TAG')));

ABOVE the JHtml::_('formbehavior.chosen', 'select'); line. The placement of the line matters!

In the filter_articles.xml file you need to add the line class="advancedTags" after multiple="true"

@brianteeman
Copy link
Member

@brianteeman brianteeman commented Dec 20, 2014

Fair enough on point one

Select an option does need to be resolved though or no one will know what
the option is they are selecting

On 20 December 2014 at 12:54, Nicholas K. Dionysopoulos <
notifications@github.com> wrote:

@test https://github.com/test Successful

Regarding Brian's observations:

  • The search for multiple tags will return articles which belong to
    any of the tags. Looking for all tags would have a very adverse performance
    and memory impact.
  • The "Select an option" label can be changed, but it's tricky. See
    below.

To restore the "Select Tag" label in the field you need to make two
changes:

In administrator/components/com_content/views/articles/tmpl/default.php
add the line:

JHtml::('formbehavior.chosen', '.advancedTags', null, array('placeholder_text_multiple' => JText::('JOPTION_SELECT_TAG')));

ABOVE the JHtml::_('formbehavior.chosen', 'select'); line. The
placement of the line matters!

In the filter_articles.xml file you need to add the line
class="advancedTags" after multiple="true"


Reply to this email directly or view it on GitHub
#5474 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

@jrseliga
Copy link
Author

@jrseliga jrseliga commented Dec 20, 2014

Both vs Either

The intended behavior is for it to select articles tagged with either.

I used the core module "Articles - Newsflash" as the precedent for the decision. There is a field that allows for selecting multiple categories, when for example, two categories are selected it also displays articles in either. I assumed that would be consistent across all instances of multiple selections in core features, so I made the tags filter consistent.

Select Tag Label

There are two reasons I removed this. (In writing this, I saw the response by @nikosdion. Not sure if his solution addresses these, will test.)

  1. When a user picks a tag with the label there, that generic label persists when the page reloads. This is default behavior for when tag (and even category) form fields are set to allow for multiple (see Articles - Newsflash module's category option again to confirm).
    before

    If it's not there when a user picks a tag, they will see this after making a selection, which seemed more intuitive to me.
    after

  2. When the label is there it appears like any other tag in the field. When you expand Search Tools and see a tag labeled "Select Tag" and you click the "x" to remove that (with no other tags selected), in an effort to clear the Tag field and start adding your selection, the page reloads to match the change you just made. The problem being, that the page reloads with the same "Select Tag" option in the field again. As I mentioned in item 1, this is default behavior for form fields that can be set to allow multiples, so I didn't see a choice. The problem is kind of hard to describe, but testing it is simple.

    1. At line 78 of the XML (included in this pull request) append
      <option value="">JOPTION_SELECT_TAG</option>
    2. Administrator > Articles
    3. Be sure to "Clear" any filters in Search Tools
    4. Remove "Select Tag"
    5. Page reloads with "Select Tag" again.
@nikosdion
Copy link
Contributor

@nikosdion nikosdion commented Dec 20, 2014

Please try the code I kindly provided. It has NONE of the drawbacks you mentioned. As you will see if you do read my reply very carefully I did NOT add the tag. I completely sidestepped it by tricking Chosen into providing a different label when you use the special class name "advancedTags" on the field.

If you are interested in the underlying mechanics of my solution: the label is enforced on the field by Chosen. In default.php we already load Chosen, telling it to hijack all tags. However, since there are no options provided, the default JHtmlFormBehavior::chosen lines 68:71 kick in and apply the default label JGLOBAL_SELECT_SOME_OPTIONS. The solution to that is using a different instance of Chosen, with a different selector and a different label. That's why there are two parts in my solution. FIRST: You need to apply a custom class name to the field. This is done in filter_articles.xml by adding the class= line. SECOND: You need to add a custom Chosen instance against the advancedTags classname and with a custom label. This is what the line I told you to add in default.php does. However, you can't add it anywhere you please! The multiple Chosen instances run in the same order they are loaded. This is why I told you that it's extremely important to place the new line ABOVE the existing behavior.chosen line. Please try the two super simple changes I gave you and try to understand why it all works when put together. About an hour ago I had no idea how to fix that issue either. I read the code, tried to understand it and work through the solution :)

@jrseliga
Copy link
Author

@jrseliga jrseliga commented Dec 20, 2014

As @nikosdion suggested, making those changes resolves the issue of the label.

Thanks for the explanation, as soon as I read your solution I understood what it would do. I'm just not sure I would have been able to figure it out myself, at least not as quickly as you did. Thanks!

As I mentioned in my tweet to you guys this was my first pull request, not only for Joomla, but ever. With that said, how do I handle making these changes? Do I create a separate pull request for /tmpl/default.php and reference this pull request? What do I do about updating the XML file in this pull request?

@jrseliga
Copy link
Author

@jrseliga jrseliga commented Dec 20, 2014

From what I can tell, I just need to make the appropriate changes and push them to the "multiple-tags-filter" branch and this pull request will be automatically updated?

@nikosdion
Copy link
Contributor

@nikosdion nikosdion commented Dec 20, 2014

I've spent eight years hacking around Joomla!, I'm sure it usually takes me less than most developers to figure out where to look for a solution :D That said, I'm not a magician. I'll tell you my secret sauce: start from the symptom and read the code upwards to the root cause. Since this is your first PR ever, please let me tell you how I figured out the solution.

When I saw the problem with the label I first looked at the XML file. It uses the "tag" field. I knew that a field of type "X" corresponds to the class "JFormFieldX" so I took a look at JFormFieldTag. I followed it up the class tree but there was nothing there for me to see. Hm...

Let's take a look at the page source. The field is hidden and replaced with a chzn-something one and the select field now has a class that's nowhere else to be found, "chzn-done". Where does that come from? (The rest of this step I actually skipped because I already knew where it would lead me – I had been here before!) Searching the Joomla! source for this string shows me that it's added by chosen.jquery.js and chosen.jquery.min.js. All right, let's look for "chosen.jquery" in the Joomla! source code. Oh, right, it comes from JHtmlFormbehavior::chosen! Let's read the code. This is where I saw where the label comes from. A-ha! I need to change that. Where in my view do I load this form behavior? I searched administrator/components/com_content/views/articles for "behavior.chosen". Why "behavior.chosen"? Because I knew that JHtmlFoo::Bar is always loaded with a call to JHtml::_('foo.bar'). You could have eventually figured it out yourself by following the code back to JHtml itself. Anyway. I found that this comes in the default.php file. So my question was how do I change the label for just this one instance? Looking back at the JHtmlFormbehavior::chosen code I saw that I needed a. a new CSS selector and b. add some stuff to the $options array. A bit of trial and error and here I am looking to the causal observer as a magician who just pulled a rabbit out of his tall hat. Regarding how to change the code in your PR, that's easier than you think. Your PR is simply a request to merge the changes you made to the multiple-tags-filter branch on YOUR repository to Joomla!'s staging branch. All you need to do is make more changes to your multiple-tags-filter branch. GitHub will magically update your PR. Hey, now you know how to pull rabbits out of your hat too!

Implements solution by @nikosdion to fix issue with default field label
introduced by allowing selection of multiple tags
@jrseliga
Copy link
Author

@jrseliga jrseliga commented Dec 20, 2014

Thanks for taking the time to explain your thought process, understanding how goes much further than just being told what needs changed.

In the scheme of things I know this is minor but I used class="advancedMultiple" instead of class="advancedTags" as this solution isn't specific to just tags. For example it would be applicable to a category field since it can be set to allow multiple selections also.

@nikosdion
Copy link
Contributor

@nikosdion nikosdion commented Dec 20, 2014

Actually, you need one formbehavior.chosen call for each custom label. That's why I named the class advancedTags: the label "- Choose tag -" only applies to tags, not any other field like Categories.

Can not use ambigious class declaration as this is specific to only tags
@jrseliga
Copy link
Author

@jrseliga jrseliga commented Dec 20, 2014

Right, over thinking on my part. Fixed.

@jrseliga
Copy link
Author

@jrseliga jrseliga commented Dec 20, 2014

I'm not sure how the process plays out for this to be officially merged, but I realized that this also needs to be added to the featured view as well so that tag filtering is consistent in all of com_content. I will make those changes and push them shortly.

Allow featured articles to be filtered by multiple tags in Administrator
@jrseliga
Copy link
Author

@jrseliga jrseliga commented Dec 20, 2014

Featured Articles multiple tags filtering can now be tested. Also updated original comment with detailed instructions and explanation.

@jrseliga jrseliga changed the title Support multiple tags filtering in Administrator ContentModelArticles Article/Featured Article multiple tags filtering in Administrator Dec 20, 2014
@brianteeman
Copy link
Member

@brianteeman brianteeman commented Dec 20, 2014

Awesome stuff - thanks for your contribution its a great one for your first
(and hopefully not the last)

On 20 December 2014 at 16:28, jrseliga notifications@github.com wrote:

Featured Articles multiple tags filtering can now be tested. Also updated
original comment with detailed instructions and explanation.


Reply to this email directly or view it on GitHub
#5474 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

@jrseliga
Copy link
Author

@jrseliga jrseliga commented Dec 20, 2014

The front-end model doesn't even allow filtering on a single tag, let alone multiple, I'll build that into this pull request before it's officially merged, should be done sometime tomorrow.

@ggppdk
Copy link
Contributor

@ggppdk ggppdk commented Dec 20, 2014

Quoting:
The search for multiple tags will return articles which belong to any of the tags. Looking for all tags would have a very adverse performance and memory impact.

-- I struggled with this a year ago, trying to have acceptable performance but then i gave up, if you have any suggestion on how to make a query require -ALL selected - tags instead of - ANY selected - in large web-site (thousands of tags and 10000+ items) it would be great


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

@brianteeman
Copy link
Member

@brianteeman brianteeman commented Dec 20, 2014

@test all good now


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

@sovainfo
Copy link
Contributor

@sovainfo sovainfo commented Dec 21, 2014

Suggest to join on type_id instead of type_alias. Should allow for using the index.

@ggppdk :

SELECT title, count(tag_id) as tags 
FROM `j336_content` 
JOIN `j336_contentitem_tag_map` ON type_id = 1 AND id = content_item_id  
WHERE tag_id IN (3,4) 
GROUP BY title 
HAVING tags = 2

What does your EXPLAIN say?

@ggppdk
Copy link
Contributor

@ggppdk ggppdk commented Dec 21, 2014

@sovainfo
thanks for reply !!,
this is similar to the query i am using, also tried placing the COUNT in the HAVING clause

  • Using your query to require ALL tags
    I get on the DB table: _contentitem_tag_map
    --> Using where; Using index; Using temporary; Using filesort
  • Query to require ANY tags (GROUP BY is removed and DISTINCT is used instead):
    I get on the DB table: _contentitem_tag_map
    --> Using where; Using index; Using temporary;

The difference is that the --require ALL tags-- uses filesort

This pull request should be performance safe, because it does not use GROUP BY/COUNT/HAVING to require ALL -selected- Tags that has the above mentioned performance problem

@sovainfo
Copy link
Contributor

@sovainfo sovainfo commented Dec 21, 2014

But it is still using type_alias instead of type_id !

Allow articles/featured articles to be filtered by tag(s) in
Administrator
@jrseliga
Copy link
Author

@jrseliga jrseliga commented Dec 21, 2014

@brianteeman @nikosdion Pushed tag(s) filtering to the front-end model.

@jrseliga jrseliga changed the title Article/Featured Article multiple tags filtering in Administrator/Site Filter on multiple Access Levels, Authors, Categories, and Tags Mar 5, 2015
jrseliga added 2 commits Mar 5, 2015
@wilsonge wilsonge added this to the Joomla! 3.5.0 milestone Mar 11, 2015
@paternax
Copy link

@paternax paternax commented Mar 14, 2015

Several combinations of selections were tested successfully. But deleting the filter entries in the search tool doesn't reset the filtering in the articles list. It has to use the clear button to reset the list.


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

@knoby2015
Copy link

@knoby2015 knoby2015 commented Mar 14, 2015

it´s ok but when i set all in normal, there will not switch to all articles. the filter is ok only the set of zeoro is not possible


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

@FrankyDE
Copy link

@FrankyDE FrankyDE commented Mar 14, 2015

bug found :-(
the selection of the tags works great, but it does not "reload" when you deselect the tags
Example: Tag: Red: correct articles are displayed.

5474 - select_tagscorrect

When you delete the Tag Selection the article view is not refreshed ... normally then all articles should be displayed again.

5474 - select_tagscorrectnorefresch

Sorry, as I like the idea very much and I find it cool and pretty much straight forward.

By the way I do not know what the expected behavior would be, I expected:
tag1 AND tag2 AND tag3
but the selection appears to be
tag1 OR tag2 OR tag3

Any opinions on that?

@brianteeman
Copy link
Member

@brianteeman brianteeman commented Mar 14, 2015

@FrankyDE that is something that @phproberto and myself have been discussing - the ability to choose if you want it to be AND or OR


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

@rmittl
Copy link

@rmittl rmittl commented Mar 14, 2015

The selecting tags works great and also the resfreshing. The refreshing doesn't works, if I deselect all tags.

I expected the the OR between the tags.


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

@jrseliga
Copy link
Author

@jrseliga jrseliga commented Mar 14, 2015

@FrankyDE there was a bit of discussion above about giving the choice between AND or OR. @nikosdion had some reservations on the idea due to how inefficient the query already is, and the implications it might have for large sites.

@jrseliga
Copy link
Author

@jrseliga jrseliga commented Mar 14, 2015

@ALL Perhaps I'm not understanding the problem when resetting a filter group. I made a simple demonstration of how it functions.

@ggppdk
Copy link
Contributor

@ggppdk ggppdk commented Mar 29, 2015

Some people above stated that
-- "deselecting all values"
from a multi-value filter does not "refresh" / list all articles

the reason is because a select-multiple when having no values behaves like a checkbox-set, aka -NO VALUE- is submited (?? by most / all browsers ??), so the filter maintains its session state

The solution is to detect that form was submited:
(*example code)

Client side
<input type="hidden" name="form_submited" value="1"/>

Server side:
$form_submited = JRequest::getVar('form_submited');
$FILTERNAME = $form_submited ?
JRequest::getVar('FILTERNAME') :
$app->getUserStateFromRequest( $option.'.'.$view.'.FILTERNAME', 'FILTERNAME', false, 'array' );
if ( $form_submited ) $app->setUserState( $option.'.'.$view.'.FILTERNAME', $FILTERNAME);

Please use above JInput instead of JRequest, etc, adapt to the actual code, as the above is just an example

-- the above solution is simple, if form was submitted then do not use session for getting the filter value, instead only use the posted data, and then set session data

@ggppdk
Copy link
Contributor

@ggppdk ggppdk commented Mar 30, 2015

Hello

i will not agree or disagree with this, possibly your suggestion is best

but mostly what i did is

  1. explain --why/what-- is happening, identifying the problem
  2. used a common (indepedent to joomla) suggested approach, of detecting if form was submitted and not using the session

-- i don't say it is a best choise, but i don't think it is not as harmful as you imply

about adding an extra option to the multiselect, i am not sure if this best, since

  • browsers
  • and chosen/select2 JS libs
    treat it differently than how single select "empty" value is treated

i think it will cause more work and also can you tell how to make this "none" value invisible without hacking choosen JS ? e.g. via CSS ,

or we will need some extra JS to do this
or does Joomla / choosen already have this feature ??

@jrseliga
Copy link
Author

@jrseliga jrseliga commented Mar 30, 2015

I've tested this across Firefox (Windows/Linux), Chrome (Windows/Linux), and IE 10 and am not experiencing any issues with deselecting. Perhaps this is a browser issue? IE 9 or older?

@nikosdion
Copy link
Contributor

@nikosdion nikosdion commented Mar 30, 2015

Please ignore my replies (I removed them now). A mail filter fail made me think they were about a completely different project. Receiving too much email can do that... Sorry!

@jrseliga
Copy link
Author

@jrseliga jrseliga commented Jun 4, 2015

Circling back to this, does anyone have more information on the environment their experiencing the issue with? Again I made a demonstration for more clarity on how it should be working.

@ggppdk
Copy link
Contributor

@ggppdk ggppdk commented Jun 8, 2015

Indeed,
you clearly show it to work, sorry i don't have time to test,
can someone else do it ?

@Bakual
Copy link
Contributor

@Bakual Bakual commented Jun 9, 2015

This will also need some tests on PostgreSQL and MSSQL databases to make sure nothing breaks there.

@ggppdk
Copy link
Contributor

@ggppdk ggppdk commented Jul 19, 2015

I have tested firefox/chrome/ie11, seems to works properly, when clearing via clear button and by removing all selected options too.

Also article manager URLs seem to work properly

...&filter[access]=1

...&filter[access][]=1&filter[access][]=2

...&filter[somefiltername]=aaaa
'access' filter and other filters are cleared
thus #6916, which links from catergory manager towards the article manager seem to work properly

@wilsonge wilsonge removed this from the Joomla! 3.5.0 milestone Nov 6, 2015
@roland-d
Copy link
Contributor

@roland-d roland-d commented May 8, 2016

@alikon can you take a look if this is OK for postgres and mssql? Thank you.

@jrseliga Can you please fix the merge conflicts so we can get it ready for 3.6? Thank you.


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

@brianteeman
Copy link
Member

@brianteeman brianteeman commented Aug 5, 2016

@jsreliga will you be updating this to fix the merge conflicts. If we dont get a reply then this will be closed in a few weeks.


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

@ggppdk
Copy link
Contributor

@ggppdk ggppdk commented Aug 5, 2016

@jrseliga

Circling back to this, does anyone have more information on the environment their experiencing the issue with? Again I made a demonstration for more clarity on how it should be working.

There were some bogus PHP versions (with suhosin ?),
that were not submiting select multiple as empty array, instead the array did not exist at all in the posted data

@roland-d
Copy link
Contributor

@roland-d roland-d commented Aug 5, 2016

I have someone working on this, will know next Friday if it is finished or not.

@brianteeman
Copy link
Member

@brianteeman brianteeman commented Aug 8, 2016

Closed as we have a NEW pr #11517

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

You can’t perform that action at this time.