Conversation
|
Corrects a lot of stuff. Still some Ajax issues I guess. before, for superUser Before, for administrator After, for superuser After for administrator BUT, if I change a permission to Denied for superUser , before save I get
and after save
So, this is a small problem, but it also shows (and I have seen that all over before or after patch), that we have to save the permissions to get the correct saved settings. |
|
@infograf768 The AJAX results after saving are not part of this PR, I am going to make a separate one for that. As for the Denied, if I am not mistaken, this is always overriden by the Super User setting or not? |
The calculated settings after saving are correct. I will test now site login (for example) after and before save. |
|
yep, the display of the permissions is wrong but the super user can login in frontend before and after saving the permissions. |
|
@infograf768 I have not looked at the AJAX results yet, that will be the next PR :) Perpahs we should show the Conflict status if you set this to Denied but it is overruled. |
Well, if the Ajax issue is solved, it should not stay at all in the New settings box, should'nt it? |
|
@infograf768 Even with AJAX, we can't change the dropdown back to what it was as it will appear as nothing worked. We can then show the Conflict status and perhaps a message that the change wasn't saved because of it. |
|
|
||
| // The calculated setting is not shown for the root group of global configuration. | ||
| $canCalculateSettings = ($group->parent_id || !empty($component)); | ||
| $canCalculateSettings = ($group->parent_id || '' !== $component); |
There was a problem hiding this comment.
Why are we removing the empty check over this?
There was a problem hiding this comment.
We are not removing the empty check, we are still checking if it is empty. Since $component is a string you only need to check against an empty string. No need to use empty.
There was a problem hiding this comment.
http://maettig.com/code/php/php-performance-benchmarks.php empty used to have a performance benifit. Not sure if it's still the case in modern php versions. But imho is easier to read....
|
Will this be still updated or can I mark the test as succesful? It is quite urgent to get it in. This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10755. |
|
@infograf768 This PR is ready to go, I am working on the AJAX code now. |
|
I have tested this item ✅ successfully on 93b27b9 This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10755. |
|
if super user cannot be changed can't we disable the select boxes for the super user group? Update: Froget it i saw your other PR now. |
|
I have tested this item ✅ successfully on 93b27b9 This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10755. |
|
@andrepereiradasilva Thanks for the test. I was actually thinking, on the Super User tab, you only need the Super User privilege. The rest is useless basically. Indeed, this would be for another PR. |
|
RTC as we have 2 succesful tests This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10755. |
Summary of Changes
This change fixes the reporting of the ACL settings.
Testing Instructions
Calling @andrepereiradasilva and @infograf768 for special attention :)