Skip to content

Fix the response from the AJAX ACL change to be correct#10757

Merged
wilsonge merged 2 commits intojoomla:stagingfrom
roland-d:fix-ajax-acl-settings
Jun 8, 2016
Merged

Fix the response from the AJAX ACL change to be correct#10757
wilsonge merged 2 commits intojoomla:stagingfrom
roland-d:fix-ajax-acl-settings

Conversation

@roland-d
Copy link
Copy Markdown
Contributor

@roland-d roland-d commented Jun 8, 2016 •
edited by joomla-cms-bot
Loading

Related to #10755

Summary of Changes

This change fixes the feedback given by the AJAX response when changing a permission.

Testing Instructions

  1. Go to System -> Global configuration -> Permission
  2. Click on Super User
  3. Change the permission to Denied
  4. The response is that the permission is Denied which is not true should be Allowed (Super User)
  5. Click on Manager
  6. Change Site Login to Denied
  7. Click on Administrator
  8. Change Site Login to Allowed
  9. The response is that the permission is Allowed which is not true should be Conflict because the Denied of Manager overrules the Administrator setting
  10. Apply the patch
  11. Go to System -> Global configuration -> Permission
  12. Click on Super User
  13. Change the permission to Denied
  14. The response is that the permission is Allowed (Super User). This is correct because the Super User setting overrules this.
  15. Click on Manager
  16. Change Site Login to Denied
  17. Click on Administrator
  18. Change Site Login to Allowed
  19. The response is that the permission is in Conflict. This is correct because the Denied of Manager overrules the Administrator setting of Allowed

Calling @andrepereiradasilva and @infograf768 for special attention :)

@roland-d roland-d added this to the Joomla 3.6.0 milestone Jun 8, 2016
@joomla-cms-bot joomla-cms-bot removed this from the Joomla 3.6.0 milestone Jun 8, 2016
@andrepereiradasilva
Copy link
Copy Markdown
Contributor

andrepereiradasilva commented Jun 8, 2016 •
edited
Loading

9 The response is that the permission is Allowed which is not true should be Conflict because the Denied of Manager overrules the Administrator setting

IMHO "Conflict" is not the right word, should be "Not Allowed (parent rule)" or something like that.

@roland-d
Copy link
Copy Markdown
Contributor Author

roland-d commented Jun 8, 2016

@andrepereiradasilva Again, I agree with you but the current implementation doesn't support this. My goal is to fix the problems first and then improve what we have.

@andrepereiradasilva
Copy link
Copy Markdown
Contributor

andrepereiradasilva commented Jun 8, 2016

ok right!

@infograf768
Copy link
Copy Markdown
Member

infograf768 commented Jun 8, 2016

siesta time, will look later.. ;)

@andrepereiradasilva
Copy link
Copy Markdown
Contributor

andrepereiradasilva commented Jun 8, 2016

Just an additional comment.
Before doing

18 Change Site Login to Allowed

The administrator "Site Login" caculated permission is "Not Allowed (Locked)".
And when changed to "Allowed" we get "Conflict".

So, if i understand correctly, we have two text strings for the same thing ... that is rather confusing.
I guess this was nothing to do with this PR, but this could be better.

@andrepereiradasilva
Copy link
Copy Markdown
Contributor

andrepereiradasilva commented Jun 8, 2016

I have tested this item ✅ successfully on 458adc0

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10757.

@roland-d
Copy link
Copy Markdown
Contributor Author

roland-d commented Jun 8, 2016

@andrepereiradasilva You now see the quicksand I have been playing with :)

@infograf768
Copy link
Copy Markdown
Member

infograf768 commented Jun 8, 2016

I have tested this item ✅ successfully on 458adc0

OK here. I can't believe how bad ACL has gone...

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10757.

@infograf768
Copy link
Copy Markdown
Member

infograf768 commented Jun 8, 2016

RTC,

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10757.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jun 8, 2016
@andrepereiradasilva
Copy link
Copy Markdown
Contributor

andrepereiradasilva commented Jun 8, 2016

@roland-d you're makign a great work solving all this issues! Thanks!

@roland-d
Copy link
Copy Markdown
Contributor Author

roland-d commented Jun 8, 2016

@infograf768 At least we are fixing it :)

@andrepereiradasilva Thanks.

@infograf768
Copy link
Copy Markdown
Member

infograf768 commented Jun 8, 2016

You are! I can't believe no one else tries. 😺

@infograf768
Copy link
Copy Markdown
Member

infograf768 commented Jun 8, 2016

back to siesta

@brianteeman brianteeman added this to the Joomla 3.6.0 milestone Jun 8, 2016
@wilsonge wilsonge merged commit d1ac470 into joomla:staging Jun 8, 2016
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jun 8, 2016
@gwsdesk gwsdesk mentioned this pull request Jan 6, 2017
Closed
@roland-d roland-d deleted the fix-ajax-acl-settings branch January 29, 2017 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

Joomla 3.6.0

Development

Successfully merging this pull request may close these issues.

6 participants

@roland-d @andrepereiradasilva @infograf768 @brianteeman @wilsonge @joomla-cms-bot