Issue observed on creating a NEW article. PR resolves the issue - thanks

Thanks Brian.

@test

This PR resolves reported error.

RTC Thanks

IMHO, this should go in 3.4.2

Bug was introduced with #5230.
While this code works, I'm not that sure it makes much sense. $this->rules is actually always set as it is declared in JTable and thus the check will always return false.
I would suggest to use if (!$this->getRules()) instead if you want to check if there are rules specified.

As the original code was supposed to fix frontend editing where the permission tab is absent this needs to be tested on frontend editing as well.
And probably also on PostgreSQL.

Pinging @wilsonge, @alikon and @nikosdion as those were the testers on the original PR. So I suppose they can still test on PostgreSQL

I agree this would be good to have in 3.4.2 but it's Georges call.

isset returns false for a null variable (which is the case with rules) https://github.com/joomdonation/joomla-cms/blob/patch-15/libraries/joomla/table/table.php#L78 So whether you use that or just check using ! I don't really mind.

I would agree using $this->getRules() would make logical sense - rather than the class var (as it's more future proof).

Agree with Thomas and made the change. However, I think we might still have a possible issue here. I think when there is no permission set for that article (from frontend form ? haven't checked it yet), the permission settings of that article should be inherited from the category it is assigned to, not from the component settings as in the current code. Am I correct?

I don't think so. I'm pretty sure in the backend it's just using the default permissions for the article (which is the ones for com_content)

isset returns false for a null variable (which is the case with rules)

Ah true. Didn't remember that. I stand corrected

I think when there is no permission set for that article (from frontend form ? haven't checked it yet), the permission settings of that article should be inherited from the category it is assigned to, not from the component settings as in the current code. Am I correct?

George is right here. We don't need to inherit the permissions from the category. Otherwise you would have to change all article permissions if the category permissions are changed.
In fact we can just save an empty rule ({}) to use the defaults from either category or component. Our ACL will calculate that on rumtime when needed.

@Bakual I can't check with PostgreSQL any longer since the laptop I used for these tests was stolen and I didn't have the time to set it up on the new one. Sorry :(

Set back to Pending

@wilsonge I checked it again and saw that you are correct. When we create a new article from backend, the default permission settings is set from com_content permission settings, so you are correct.

@Bakual That's what I thought, set it to empty rule so that every permissions is "Inherited" and make it inherit permission from the category it was assigned to. However, as George pointed out, it is not how it works when the article is created from backend, so we will leave it as how it is.

Could @brianteeman and @jwaisner re-test it so that we can get this bug fixed in 3.4.2?

Tested with

• creating an article in frontend: check gets triggered as expected, default permissions applied.
• creating an article in backend: check fails as expected, permissions are saved as given in the form.
• editing an article in frontend: permissions remain unchanged as expected (form doesn't have rules field)
• editing an article in backend: permissions are saved as given in the form.

So I think this one is save for merging. Setting back to RTC.
If someone can run a test on PostgreSQL that would still be great.

tested on MSSQL and Postgresql and works well

Merged - thanks guys :)