Component tags not showing when filter unpublished categories from tagged item patch is used

[ColinM2]

This item relates to commit https://github.com/joomla/joomla-cms/pull/10304 The objective of commit 10304 is to the prevent the display of tagged items that are in unpublished Joomla! categories. However the patch is incomplete and it is creating very serious user problems for several components that use tags in Components. In these cases the tags are used in content that is not a Joomla! article and as a result the content is not assigned to a Joomla! category. This is affecting components such as jDownloads and HWDMediaShare that make good use of tags in their own content.

HWDMediaShare have found a solution see https://hwdmediashare.co.uk/blog/769-joomla-3-6-0-undesirable-change-affecting-joomla-tags-in-hwdmediashare

by changing lines 572 and 573 in libraries/cms/helper/tags.php lines from

// Join over categoris for get only published 
->join('INNER', '#__categories AS tc ON tc.id = c.core_catid AND tc.published = 1')

to

// Join over categories to only get items in published categories, or items in no category  
->join('LEFT', '#__categories AS tc ON tc.id = c.core_catid')->where('(tc.published = 1 OR tc.published IS NULL)')

The added clause OR tc.published IS NULL)') is, I believe, testing for those cases where a Joomla! Category does not exist. That is the tags are being used in other than a Joomla! article or a Joomla! category in a Component which has Content. There may of course be a better or more robust test. The test does however have the major virtue that it works and solves the "challenge".

Steps to reproduce the issue

Install jDownloads, create a jDownloads Category, then create a jDownloads Download. Use jDownloads to add a Tag to the Download. NB you may need to first add the tag to a regular Joomla! article as it cannot be typed in directly as a result of the above commit, which is also a pain. The user documentation on adding a tag to jDownloads is http://www.jdownloads.net/documentations/item/tags-support-in-jdownloads Add jDownloads menu item 'List All Categories'. Do all the from the backend.

From the front end now use the menu item 'List All Categories" to show the Download, click on the tag. The Download is not listed but the article is listed.

Expected result

With the modified version which includes the OR tc.published IS NULL)') the Download is listed.

Actual result

With unmodified version the Download is not listed

System information (as much as possible)

Only applies to latest Joomla 3.6

Additional comments

This is very embarrassing for us!!!

ColinM2

[mbabker]

At no point should a published/state column have a NULL value so to me this proposal is trying to work with an invalid data record.

On Wednesday, September 14, 2016, ColinM2 notifications@github.com wrote:

This item relates to commit #10304 https://github.com/joomla/joomla-cms/pull/10304 The objective of commit 10304 is to the prevent the display of tagged items that are in unpublished Joomla! categories. However the patch is incomplete and it is creating very serious user problems for several components that use tags in Components. In these cases the tags are used in content that is not a Joomla! article and as a result the content is not assigned to a Joomla! category. This is affecting components such as jDownloads and HWDMediaShare that make good use of tags in their own content.

HWDMediaShare have found a solution see https://hwdmediashare.co.uk/blog/769-joomla-3-6-0- undesirable-change-affecting-joomla-tags-in-hwdmediashare

by changing lines 572 and 573 in libraries/cms/helper/tags.php lines from

// Join over categoris for get only published ->join('INNER', '#__categories AS tc ON tc.id = c.core_catid AND tc.published = 1')

to

// Join over categories to only get items in published categories, or items in no category ->join('LEFT', '#__categories AS tc ON tc.id = c.core_catid')->where('(tc.published = 1 OR tc.published IS NULL)')

The added clause OR tc.published IS NULL)') is, I believe, testing for those cases where a Joomla! Category does not exist. That is the tags are being used in other than a Joomla! article or a Joomla! category in a Component which has Content. There may of course be a better or more robust test. The test does however have the major virtue that it works and solves the "challenge". Steps to reproduce the issue

Install jDownloads, create a jDownloads Category, then create a jDownloads Download. Use jDownloads to add a Tag to the Download. NB you may need to first add the tag to a regular Joomla! article as it cannot be typed in directly as a result of the above commit, which is also a pain. The user documentation on adding a tag to jDownloads is http://www.jdownloads.net/documentations/item/tags-support-in-jdownloads Add jDownloads menu item 'List All Categories'. Do all the from the backend.

From the front end now use the menu item 'List All Categories" to show the Download, click on the tag. The Download is not listed but the article is listed. Expected result

With the modified version which includes the OR tc.published IS NULL)') the Download is listed. Actual result

With unmodified version the Download is not listed System information (as much as possible)

Only applies to latest Joomla 3.6 Additional comments

This is very embarrassing for us!!!

ColinM2

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/joomla/joomla-cms/issues/12038, or mute the thread https://github.com/notifications/unsubscribe-auth/AAWfoZpPDiLywCjBcBzqE8MzwrYkQ8Trks5qp_1ogaJpZM4J8zWu .

[ColinM2]

@mbabker As I have tried to explain the test is trying to exclude those cases where the content is NOT a Joomla! article type content but is content from a Component. No one is setting a field to NULL. Because the Joomla! category does not exist then the test for NULL works because the Joomla! category being referenced does not exist. It is not claimed that the 'fix' is the best or is sufficiently robust. It is a method of effectively checking if the Joomla! category exists. Someone far better at SQL than myself can surely make a better solution but for an emergency situation facing our users we need something simple even if it is not the purest or best coding. I would suspect that the final solution will involve a more comprehensive change which involves more php and sql code. I am only trying to identify a bug in Joomla! that is preventing users employing tags in non-Joomla! content.

[mbabker]

The red flag to me is your WHERE clause is checking the joined table's published column for NULL values. The categories table should NOT have a NULL value in this column unless the record is broken. So I'm not seeing how this addresses a case where the record doesn't exist; your added filter is against the categories table so it would be filtering whatever is already joined.

Without seeing the full data set and in question I suggest this not be applied to core because as is the filter is now looking for invalid records from the categories table which to me suggests someone is inserting invalid data somewhere or the query in general is bad and needs better rewriting/improvement.

[ColinM2]

I am not contending that the quick fix is a proper Solution, and I fully appreciate your concern about the code as it stands. You obviously know much more than I do in regard to SQL. I can absolutely assure you jDownloads is not inserting invalid data somewhere. How can I tell you that even clearer! Personally I am not an SQL specialist and took the solution from the HWDMediaShare article https://hwdmediashare.co.uk/blog/769-joomla-3-6-0-undesirable-change-affecting-joomla-tags-in-hwdmediashare. Surely the point is that Commit 10304 overlooked the possibility that Joomla! tags are also used in situations where there is no associated Joomla! category, an unintended consequence. Or am I mistaken that Joomla! tags are not allowed in other content? Why are you, a Joomla! member, being so aggressive to me a mere person who is trying to request fixing a faul, that is causing some users distress? Prior to Commit 10304 the tags worked absolutely fine in jDownloads. After that commit they are not working. I am sure you are not suggesting the problem does not exist. Please please why not suggest a proper solution? I am asking for Help.

[mbabker]

Not trying to be aggressive by any means. But the suggested change raised a major red flag to me because as I said it added a filter for a value that cannot exist through normal use of Joomla and that to me suggests a deeper rooted issue; whether it works or not doesn't really matter to me because it's just wrong (the SQL schema defines the column as not allowing null values and the field is tied to the published/state selector you see in the admin UI which doesn't have an empty or null value).

You're correct that it does raise an issue for content that does not have an associated category. I could most likely reproduce this scenario with my own code which supports tags without using the core category system. That part needs to be fixed. My only advocation is that whatever change is made from here does NOT use a filter for a condition that cannot exist in normal operation of the system.

[alikon]

supposing that you are using Mysql than as per #__categories table definition the published field published tinyint(1) NOT NULL DEFAULT 0, CANNOT BE NULL https://github.com/joomla/joomla-cms/blob/staging/installation/sql/mysql/joomla.sql#L212

so if something should be fixed it should be on the extensions side

[ColinM2]

@mbabker thanks I understand. It would seem then that the effect of the 'alleged fix' is just the same as if the code had been removed. It also seems to divert attention from the real problem! What is certain is that before the Commit it was OK but after it was not OK.
So if modifying the filter code is not the way to fix it then it needs to skip the filter section if the Joomla! category does not exist. The code concerned is in function getTagItemsQuery. This is not called directly by jDownloads. So how do you suggest I proceed? Maybe I should just scrub this issue and start it agasin without the "fix". Please advise.

@alikon Yes it has been made clear that the "fix" I found is not proper. However the code concerned is not called directly from jDownlopad. Please see above. Thanyou

[mbabker]

I don't remember the tags workflows anymore but IIRC the method building this query is called as an inner dependencies of one of the other methods.

Essentially the PHP side of the code needs to be aware if the extension uses categories to decide whether it should add a SQL filter. Off hand I don't remember if we have an API for that, but you might be able to fake that with a call to JCategories::getInstance() since it seems it's a requirement for an extension to have a subclass of this to use categories properly. If it returns a boolean false then category support doesn't exist, if it returns an object then category support is there and the extra join/where should be added to the query.

[sovainfo]

So, now it is clear that not all tagged items are of a content type that supports categories, maybe it is time to revert https://github.com/joomla/joomla-cms/pull/10304 !!!!!

Clearly you can't create an sql statement with a condition that calls php code! You can do that when the condition can be evaluated before reading any rows. You can't do that when you need the information from the rows. Which means you need to process the rows in php and filter them. You can't have an sql statement that does that for you!

It would have been possible if the information had been available in the database. So, if #__content_types had an attribute support_categories, which would have been set to yes for core content_types. Then you would be able to select any item that doesn't support categories and those that do support it with a published category. Which happens to be the information request here!

[sovainfo]

@ColinM2 ->join('LEFT', '#__categories AS tc ON tc.id = c.core_catid AND tc.published = 1') Would give you the same! Note the change from INNER to LEFT. It makes the categories part optional, exactly what you think you need. Obviously, it is not the correct solution. It does select those items not supporting categories. It also selects rows without a published category. You still need php processing the result set. Only category supporting content types require a published category.

@mbabker

At no point should a published/state column have a NULL value so to me this proposal is trying to work with an invalid data record.

No, it is not! The condition is on the result set, not on the table itself! As mentioned, there is no need for the condition, just change the join type from INNER to LEFT. The proposal is considered bad SQL! (condition is redundant).

The red flag to me is your WHERE clause is checking the joined table's published column for NULL values. The categories table should NOT have a NULL value in this column unless the record is broken. So I'm not seeing how this addresses a case where the record doesn't exist; your added filter is against the categories table so it would be filtering whatever is already joined.

No, it is not! Again the IS NULL checks whether there is a valid category in the result set! Nothing to do with the #__category table ! Again, the condition is redundant!

My only advocation is that whatever change is made from here does NOT use a filter for a condition that cannot exist in normal operation of the system.

As mentioned, the condition is redundant but doesn't filter for something that cannot exist in normal operation. The condition applies to the result set, not to the table #__categories. It just checks whether it joined a valid category! A very normal construct on relational databases.

@alikon Sounds like you are missing the point as well. Again, nothing to do with content in #__categories. It is about checking whether there is a valid category or not. Both are acceptable for tagged items, obviously depending on the content type of the item.

[phproberto]

I can confirm this issue. It's funny because @svenbluege warned about the possible issue but it was ignored.

https://github.com/joomla/joomla-cms/pull/10304#issuecomment-220887814

[sovainfo]

Not really ignored. The question was wrongly interpreted and @svenbluege didn't react.

[svenbluege]

@sovainfo Actually your answer was fine for me since I assumed you have more knowledge here than me

[mbabker]

@sovainfo maybe I read something wrong but seeing tc.published IS NULL to me jumped out as "join where the category record's published value is null", which cannot happen. Sure it also addresses the case where the data just doesn't exist, but it still feels wrong having a SQL statement written that way.

And honestly I still have to look every time I do a join to remember what each one does, so I didn't even think to change the join type.

[ColinM2]

First appologies to one and all for clouding the issue with bad SQL - I should know better

@mbabker Thank you for pointing out the possible use of JCategories::getInstance() https://github.com/joomla/joomla-cms/blob/3.6.2/libraries/legacy/categories/categories.php#L126 We will look into this. @sovainfo thank you for your comments. I have formed the impression that this 'challenge' cannot be solved by the use of SQL alone, which is what 10304 is doing. Reverting #10304 and then using supplementary php to check the tag type would seem to be a better avenue. If the tag type is not Joomla! content then it should 'pass'. If it is Joomla! content then to determine if it is in a published category. This I would suggest may be best in a 'callable' function so that a Component or other extension could make any supplementary assessment of whether the tag should be 'seen'. I will also try your suggestion and report back as maybe it could be all done with SQL

[sovainfo]

@svenbluege It wasn't me that responded to your question. It was @roland-d, who also made the mistake to merge the PR. It is not the first time no proper impact analysis was done. Obviously, the changed behaviour needs to be tested. In addition it should be tested there are no negative side effects.

@mbabker It depends on where you see that condition: ->join('INNER', '#__categories AS tc ON tc.id = c.core_catid AND tc.published IS NULL) you are correct ->join('LEFT', '#__categories AS tc ON tc.id = c.core_catid AND tc.published = 1 ) ->where('tc.published IS NULL') you are wrong. The second construct gives you those items that don't have a published category. A common situation when you unpublish a category.

[mbabker]

Now that I'm reading this whole thread again, all I can say is pass the coffee. I completely missed the changed join type from the word go.

[sovainfo]

@ColinM2 See no reason for you to apologize. You pointed out a bug and mentioned what you thought was a solution. Turns out they are both wrong!

Doing it in SQL requires the attribute in #__content_types, which is already INNER joined. the condition would be ->where('support_categories = 0 or (support_categories = 1 and tc.published = 1)')

This would give you items of content with published category and HWDMediaShare items.

[ColinM2]

@sovainfo Thanks for comments. Have been away a few days. I am uncertain of the complete SQL line, or lines, that should be in a revised commit. You noted that the where clause should be ->where('support_categories = 0 or (support_categories = 1 and tc.published = 1)') but what is the main part? The only one that works for me is the very first one which caused some 'discussion'. Where do we go from here?

[sovainfo]

I will also try your suggestion and report back as maybe it could be all done with SQL

Read this as you were going to add attribute support_categories to #__content_types, with 1 for core rows and 0 for your own extension. The SQL statement should be created as: ->join('LEFT', #__categories AS tc ON tc.id = c.core_catid AND tc.published = 1 ) ->where('support_categories = 0 or (support_categories = 1 and tc.published = 1)') to allow for both situations: when the tag is on a content type supporting categories it needs to have a published category.

[sovainfo]

That still leaves the question of @svenbluege unanswered. According to the table definitions the value 0 is allowed in catid. Which means content can be added without being categorized (catid = 0). Sofar, haven't found a way of doing that in frontend or backend, an existing category is proposed. Would consider the default category uncategorized for that particular content type more appropriate in the data model instead of 0.

Then the answer would be: Only when the content type supports categories it is required, otherwise the attribute catid shouldn't exist or be NULL.

[ColinM2]

As background I am supporting jDownloads component. There exist Categories, which are essentially a directory plus some database info, and Downloads that are the file to be downloaded plus some database info such as description and so on. In table zzz_content_types (where zzz stands for local database prefix) we had already added 2 type_id entries of 10000, for jd Downloads, and 10001 for jd Categories These id's are also in the zzz_ucm_content table in core_type_id, in the zzz_ucm_base in ucm_type_id, and in the zzz_contentitem_tag_map table in type_id.

I could not detect in the SQL statement where support_categories is 'located' so I assummed, incorrectly as it turns out, that support_categories would include the relevant info from one or other of those tables and used the SQL below. ->join('LEFT', '#__categories AS tc ON tc.id = c.core_catid') ->where('(support_categories = 10000 or support_categories = 10001) or (support_categories = 1 and tc.published = 1)')

This produced no results at all. Or of course it could be my poor detail knowledge of SQL - which is improving slowly! I also tried as below but that also failed to produce anything ->join('LEFT', '#__categories AS tc ON tc.id = c.core_catid') ->where('support_categories <> 1 or (support_categories = 1 and tc.published = 1)')

Could I trouble you to advise please where else I should modify the SQL to include the support_categories or maybe we are part in error in not locating the ids in another table.

This does however raise the issue that perhaps selection may need to be 'component' specific.

[sovainfo]

add attribute support_categories to #__content_types

means just what it says! Same thing with

The SQL statement should be created as: ->join('LEFT', #__categories AS tc ON tc.id = c.core_catid AND tc.published = 1 ) ->where('support_categories = 0 or (support_categories = 1 and tc.published = 1)')

where the only thing you need to change is the prefix: #_

[ColinM2]

Thank you for your patience. I missed that you were suggesting adding another attribute (in my terms an extra 'column') to the #__categories table.

The actual information that is wanted, namely the tag type id is already in tables #content_types (type_id}, #contentitem_tag_map (type_id again), #__ucm_content table (core_type_id) and #__ucm_base (ucm_type_id).

By a long chalk I am not sufficiently adept in sql to know how to use one of those tables to achieve the objective. Indeed is it even possible?

It does strike me however that a component or other extension should be able to call a function that specifies which content types should be included, or perhaps excluded, in the final list. Such added selectivity would also add another dimension to the use of tags.

[sovainfo]

I missed that you were suggesting adding another attribute (in my terms an extra 'column') to the #__categories table.

Not suggesting that, please read more carefully! The correct place for that column is in #12047

The actual information that is wanted, namely the tag type id is already in tables #content_types (type_id}, #contentitem_tag_map (type_id again), #__ucm_content table (core_type_id) and #__ucm_base (ucm_type_id).

Sounds like you are completely missing the point!

By a long chalk I am not sufficiently adept in sql to know how to use one of those tables to achieve the objective. Indeed is it even possible?

Given exact instructions on what to do. Nothing to do with sql. Yes, it is possible!

It does strike me however that a component or other extension should be able to call a function that specifies which content types should be included, or perhaps excluded, in the final list. Such added selectivity would also add another dimension to the use of tags.

Completely different topic, unrelated to requiring a published category

[ColinM2]

Yes I do not understand the detail. All I know is that there is a problem and that the original suggestion was a quick 'fix' in our case. I hope someone will do something about it as otherwise it will impact on several components who are adding tags to their content.
It would seem I am causing more confusion than intended so I will now leave it to my betters code-wise. Thank you for your help and your obvious perception of the problem. With regard to the suggestion it is a different topic so I will raise it separately.

[ColinM2]

We have now done some more research on this problem. It relates to Components that like Joomla! have Categories.
jDownloads has categories that are defined in table xxx_jdownloads_categories. In jDownloads the contents of a jD-category are Downloads. Joomla! categories are specified in table xxx_categories and hold various types of content There are 13 Joomla! defined content types and we have added two correctly structured jDownloads types as indicated in the attached pic of the 'xxx_content_types' table.

Tags from jDownloads appear in the same tag list as Joomla! articles and so on.

The present problem line is in /libraries/cms/helper/tags.php circa line 582. This was added in 3.6.2. ->join('INNER', '#__categories AS tc ON tc.id = c.core_catid AND tc.published = 1')

This assumes that only Joomla! has a category structure, which is of course rather a simplistic view of the entire Joomla! world.

To illustrate one of the problems assume we have a Joomla! Category with a certain id and that there is a jD-Category which has the same id, a situation that is most likely to occur. Further suppose there is a Download in the that jd-Category that has a Tag.

Normally the tag will appear and its reference link will point to the Download. But if the Joomla! category with the same id is unpubished then the Tag will still be listed but it will have no link even though there is valid content (a Download).

If the "problem" line is commented out then all is well but obviously the original reason for adding the line still remains. So the problem is really the hard coded #categories part. This must be flexible so that we could in our case get a line like this: ->join('INNER', '#jdownloads_categories AS tc ON tc.id = c.core_catid AND tc.published = 1') would suit jDownloads but is clearly that is no good for Joomla! in general.

Presently the entry for the Downloads as below tells it to use table "#__jdownloads_files" but that does not tell where the jd-Category is located. Similarly of course the article type does not specify where its category info is located. That is both an Article and a Download have a cat_id field but not the name of the table for the category. So it would seem that to give a flexible approach then the content_items table needs to hold extra information about the relevant 'category or perhaps the location of a routine to call or some other way of automating the checking properly without assuming it is a Joomla! category.

Could someone please sort this problem.

[Ssupreme]

I have the same problem as I use JTag for a custom component which doesn't have any category. I don't mind getting read off INNER JOIN #_categories AS tc ON tc.id = c.core_catid AND tc.published = 1 and as I don't keep unpublished junk but for now I stick with ->join('LEFT', '#__categories AS tc ON tc.id = c.core_catid AND tc.published = 1') . It works for now.

[jameswadsworth]

I have the same problem my custom component which uses its own category system and not the joomla one. After version Joomla 2.6.2 the tags items didn't show in the frontend, because core_catid gets set to 0.

[phproberto]

I still can't believe https://github.com/joomla/joomla-cms/issues/5814 hasn't been reverted. It breaks B/C with 3rd part extension developers.

Revert and fix core view issue in another way would be the expected behaviour. Ignore and fuck extension devs is the J way.

[phproberto]

Please check patch in https://github.com/joomla/joomla-cms/pull/13737

[alikon]

@phproberto IMHO for 3rd part extension developers should be mandatory to do tests on RC/beta/alpha/staging ect .... it seems they don't ..... so don't complain about the J way, we are only human and usually make mistakes, like i did
i'm sorry for not testing against all the 3rd extensions

[phproberto]

It's not about testing it against 3rd part or not being able to do mistakes. I've done a lot of them. But when a B/C issue is found revert should be done inmediatelly until a better fix is found. And probably a big red tag like "B/C break".

We should stop blaming 3rd part devs for not testing latest versions. Can you imagine Microsoft saying that issues are caused because devs do not test programs against it? Do you see that in other CMS?

Joomla should have a better testing system and it's our fault not having it. NEVER 3rd part extensions.

[alikon]

Joomla should have a better testing system and it's our fault not having it.

all softwares should have a better testing system (if automated sounds better to me) to prevent B/C and all other kinds of new releases related issues ...... some jpeople are working hard on it

i'm only asking for more help from 3dp developers cause we are not a closed source software, and we are mainly based on volunteers

i'm sorry to disagree with you on this

[Bakual]

i'm only asking for more help from 3dp developers cause we are not a closed source software, and we are mainly based on volunteers

I'm going to say most of the devs here are actually also 3rd party devs and test with their own extensions as well. At least I do. The only thing I do "wrong" is that I first test with core and only if core is fine I start testing with my own extension 😄

[phproberto]

i'm only asking for more help from 3dp developers cause we are not a closed source software, and we are mainly based on volunteers

Asking for help is not what you did:

IMHO for 3rd part extension developers should be mandatory to do tests on RC/beta/alpha/staging ect it seems they don't ..... so don't complain about the J way,

Translation: you haven't tested my code so do not complain.

Instead of: sorry. We have reverted the patch causing the issue. We will release a new version as soon as possible. Thanks for reporting it.

I will complain about the J way every time I see it's stupid. That's how you improve things. Not ignoring your users and blaming them for your bugs.

[alikon]

ok, it's clear we have different opinion can we move on and close this issue as we have a pr #13737 to test ?

[jameswadsworth]

thanks @phproberto for the patch

[ghost]

did #13737 resolve this Issue?

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12038.}

[phproberto]

Yes. We can close this.

[joomla-cms-bot]

Set to "closed" on behalf of @franz-wohlkoenig by The JTracker Application at issues.joomla.org/joomla-cms/12038

[ghost]

closed as Issue is solved, thanks @phproberto

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12038.}