dev/joomla#35: J4 permission fix

[monishdeb]

No description provided.

[vingle]

This patch improves the behavior (no error on permissions screen, normal notices & tabs now appear), but does not list, or let the user set, any permissions. So – unless I'm missing another part of this – this doesn't fix the broken permissions in J4 – Civi permissions still can't be set.

[monishdeb]

@vingle I have now taken a different approach to fix the J4 permission block issue. Here's a demo after the latest change:

Can you please reapply the patch and try again?

[vingle]

Hi @monishdeb – that's fixed it for Joomla 4.0. I also tested a couple of basic permission changes (not a full permission test) and they worked as expected.

However, Joomla 3 is now giving me an error when trying to load this screen:

An error has occured. 
0 Class "JFormFieldRules" not found 
/Users/admin/Sites/localhost/joomla3/administrator/components/com_civicrm/civicrm/joomla/libraries/joomla/form/fields/civiperms.php:8 

NB - I'm just adding this patch to an existing J3/J4+Civi – am not building it from buildkit.

[monishdeb]

Thanks @vingle for your feedback. I'll update the patch for J3 and let you know about this.

[monishdeb]

@vingle can you please review the updated PR?

[vingle]

Thanks @monishdeb and @JoeMurray

• Works on J4 {{tick}}
• Works on J3 {{tick}}
• All the permissions still do exactly what they say they will do {{not tested, wd be a big job}}

In other words - yes, this works. But would advise Joomla+Civi users to double check any custom permissions still behave as expected as there's been a change to the code around them. I did a few spot tests on the last version of this and they did work as expected, but as permissions have a big cost associated when they break and there's many of them, maybe wise to flag more tests are worthwhile.

[JoeMurray]

Pinged in MM Joomla channel that additional testing would be appreciated.

[vingle]

On the latest J4 and Civi this is now giving an error again. I'm not sure what changed - I've reapplied this PR and still get:

An error has occurred.

Failed opening required '/home/sitename/public_html/libraries/joomla/form/fields/rules.php' 
(include_path='.:/opt/alt/php80/usr/share/pear:/opt/alt/php80/usr/share/php:/usr/share/pear:/usr/share/php') 

[JoeMurray]

@monishdeb can you work on this again?

[monishdeb]

@vingle I am unable to replicate the error on my J4 setup. To further safeguard the file inclusion I had earlier thought of adding an additional condition like:

- if (!version_compare(JVERSION, '4.0.0', 'ge')) {
+ if (file_exists(JPATH_SITE . '/libraries/joomla/form/fields/rules.php')) {

but then I think version_compare would be enough in this case. Can you please ensure after reapplying the patch if this condition is present in your /home/sitename/public_html/administrator/components/com_civicrm/civicrm/joomla/libraries/joomla/form/fields/civiperms.php file?

[seamuslee001]

@monishdeb is there a reason not to do this instead because it is kind of like your say if not greater than this instead of if less than this

Suggested change

	if (!version_compare(JVERSION, '4.0.0', 'ge')) {
	if (version_compare(JVERSION, '4.0.0', 'lt')) {

[monishdeb]

nope just copy paste code :) but yeah would be good to use that, I have (force)updated the PR with file_exists condition now.

[vingle]

That's fixed it - thank you!

[JoeMurray]

@seamuslee001 after you are satisfied and PR merged please close this and the associated issue. Thx.

[seamuslee001]

merging as per @vingle 's testing