I have tested this item ✅ successfully on aa225cb

tested on postgresql

I have tested this item ✅ successfully on aa225cb

@roland-d thanks! Working correctly. No bogus entries are created anymore for tags.

RTC

This is wrong. This needs to be togglable. There can be UCM content types that require an asset entry. It just happens that for core extensions this isn't the case (because they have their own assets). So this needs to be controllable and probably even enabled by default (for b/c)

I have tested this item ? unsuccessfully on aa225cb

For reason in comment above

@wilsonge I think you haven't looked at this enough. First of all it is not affecting all UCM content but only tags. Individual tags don't have any ACL settings. Second, the default behavior hasn't changed. Assets tracking is still enabled, now you have the option to turn it off if you want.

It does affect the UCM content (JTableCorecontent maps to the #__ucm_content table) and this is making a pretty huge assumption that everything stored in that table will not have an asset mapped to it.

The table structure meets the criteria for JTable to default assume that #__ucm_content is asset aware and treats it as such. So George is right, this does need a configurable behavior in front of it for developers. On the one site I looked into, the comment about the "asset ID is not ours" is 100% incorrect; this record in the #__ucm_content table has a corresponding asset (right or wrong) in the #__assets table matching that asset ID.

I am not making the assumption that everything stored in the #__ucm_content table is not going to use assets. That is why the change is only in the tags helper.

It is configurable because as a developer you can choose to turn it off when JTable turns it on because there is the asset_id column. So the default behavior hasn't changed. This change is only related to tags.

What are you trying to address here because right now I am not following the issue.

Tested steps 1-4, found a record for my new article in the #__ucm_content table and a corresponding record in the #__assets table. None of this is related to tags related data, but rather the fact that tags brings this crap UCM junk into the code and database. If your issue is the UCM record doesn't have the proper ACL based on the com_content configuration, to me the bug is that the ACL data isn't being properly propagated forward, NOT that an ACL record is being created.

It seems to me your issue is the fact that ACL records are added in general when content is stored to the #__ucm_content table. I would suggest either making this ACL configuration manageable or removing ACL support from the table. This really does seem like a massive hack, needing this setter feels like we are introducing support for incorrectly designed data structures. JTable ONLY declares asset support if an asset_id column exists on a table. Now you are introducing API that states "my table does not follow the system conventions".

For the record, yes, your patch seems to address whatever it is you want it to. But I do not believe this is correct by any measure.

What are you trying to address here because right now I am not following the issue.

Let me try to explain, if it isn't clear I guess we should just close the issue.

When you create an article with tags, upon storing that article JTable creates an assets entry due to the UCM table having the asset_id. Such is all valid behavior. The only thing is, there is no ACL on individual tag entries. So there is no ACL to track, as such the entry in the assets table is invalid and shouldn't be there.

The reason for the switch is because someone else may be using the UCM table and is actually using ACL on their items but for tags we don't do it.

That is what I am trying to achieve here, that tags does not store an invalid entry in the assets table.

It seems to me your issue is the fact that ACL records are added in general when content is stored to the #__ucm_content table.

I have no problem with ACL records added when content is stored in the UCM table as long as this is valid data.

I would suggest either making this ACL configuration manageable or removing ACL support from the table.

I did consider removing the asset_id field from the UCM table, but I was sure, I am going to be manhandled for that as well :)

If the records on the #__ucm_content table were for tags entries, I might feel differently. But the records in that table are based on the source content type and just duplicated there thanks to the UCM structure. So my test record added just for this PR is a com_content.article item, not a com_tags.tag item. Either way the records as inserted now are invalid for a multitude of reasons, but I guess my main thing is these records aren't directly associated to com_tags, they're just placed there because the tags code uses the UCM concept which calls for storing base shared data into that table.

I guess we wasted enough time on this. Time to move on.

It's not wasted time, we just disagree on the root issue and approach I guess. The asset records being inserted right now are absolutely invalid (no configuration, no inheritance from anything). But I'm not so sure we can just say "any record created in the #__ucm_content table by way of the tags component will have no ACL record created by default".

In truth we still need to sit down and look at the overall data structure moving into 4.0. Once upon a time, a long long time ago, while there were still contributors interested in pursuing it, there was a plan to actually make use of the UCM data structure (which has its pros and cons, in all fairness it really isn't the worst thing out there but how it was introduced has made it more enemies than friends). It's gone nowhere in the years since then and right now it's just duplicating data in the database and inserting useless records (the asset stuff). Either it needs to be used if we can come up with a way to implement it without making the migration terrible or find a way to phase it out, and deal with the UCM data structure specific bugs that exist (this asset stuff is UCM specific, just exposed through the way tags stores data).

In truth we still need to sit down and look at the overall data structure moving into 4.0.

Yes, that I agree with. This issue has been around for over 2 years now so another year won't matter I guess.

But I'm not so sure we can just say "any record created in the #__ucm_content table by way of the tags component will have no ACL record created by default".

That is a good point and I think I did not realize that.

I am all for phasing out UCM but that is up to the 4.0 team to decide what they want to do with it.