Ajaxifying the Permission Tab by tflm84 · Pull Request #8132 · joomla/joomla-cms

tflm84 · 2015-10-23T12:23:24Z

Introduction

This PR fixes a problem of saving permissions within the Global Configuration, Components, Articles, Modules, and anywhere else where permissions can be configured.
If too many permission changes are made, the request is too large, because all settings are transmitted in one big form.
If this form is too big, some data get lost and are not stored in the database. Even worse, there is no feedback, therefore the user can not recognise that anything went wrong.

Fix

This fix splits the form when the save button is clicked. All inputfields which contain permissions will be disabled and not send during the saving progress.
Now, the permissions will be stored immediately after changing a value via AJAX. This solution leads to smaller forms and consistent storage of the permissions.
If something goes wrong, an error message will be displayed.

How to test this patch

Login as administrator
Go to Global Configuration / Components / Articles / Modules / ...
Switch to permission tab
Change any permissions
A green or red checkmark will light up

Worked as a group on that issue: @icampus, @d03ms

@SInCE

Fix some CS sync the language file and add the correct @SInCE tag

Fedik · 2015-10-23T14:17:22Z

libraries/joomla/form/fields/rules.php

I am sure it can be solved easily with jQuery, and you already use jQuery 😉

coolman01 · 2015-10-24T10:51:18Z

I have tested this item ✅ successfully on 6c38d23

It works as described.
However, if for some reason the user does not want to save all the changes made in e.g. the module, and clicks the "close" button, then the permission changes are already saved. Maybe there could be a message attached to the close button to say "permission chnages saved"

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8132.}

coolcat-creations · 2015-10-24T13:26:17Z

I have tested this item ✅ successfully on 6c38d23

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8132.}

zero-24 · 2015-10-24T14:55:30Z

RTC

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8132.}

roland-d · 2015-10-24T14:57:54Z

I have removed the RTC tag for now because I think the PR should be updated with the feedback given by @Fedik and @DGT41

joomla-cms-bot · 2015-10-25T14:31:13Z

This PR has received new commits.

CC: @coolman01, @designbengel

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8132.}

tflm84 · 2015-10-25T14:31:51Z

Thanks for your feedback, i put that javascript code in a seperated file in media/system/js/. Also i used the escaped version for the error messages.

dgrammatiko · 2015-10-25T14:33:12Z

@tflm84 can you rename media/system/js/permissions.js to media/system/js/permissions.min.js and the media/system/js/permissions-uncompressed.js to media/system/js/permissions.js ?

dgrammatiko · 2015-10-25T14:35:40Z

And use JHtml::_('script', 'system/js/permissions.min.js', false, true, false, false, true);
instead of

        // Add Javascript for permission change
        $document = JFactory::getDocument();
        $document->addScript('../media/system/js/permissions.js');

zero-24 · 2015-11-02T13:13:43Z

I have tested this item ✅ successfully on b621ac6

Works great. Great work here. 👍

I have just send a very mirror CS PR at: tflm84#2 to address a space that is to much in a doc block 😄

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8132.}

dgrammatiko · 2015-11-02T14:03:30Z

libraries/joomla/form/fields/rules.php

I know I am a pain…
but this line needs to be
JHtml::_('script', '/media/system/permissions.min.js', false, true, false, false, true);

Rather a pain than a problem :)

@tflm84 @roland-d we have included the fix here: tflm84#2 so we need just @tflm84 to hit the merge button 😄

thanks for this ;)

dgrammatiko · 2015-11-02T14:19:14Z

I have tested this item ✅ successfully on b621ac6

Also works fine with the change:
JHtml::_('script', 'media/system/js/permissions.min.js', false, false, false, false, true);

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8132.}

Just a very mirror ;) one space to much here

zero-24 · 2015-11-02T14:23:55Z

RTC on testing now. Thanks 😃

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8132.}

roland-d · 2015-11-02T15:47:04Z

administrator/components/com_config/model/application.php

You don't need to instantiate the database connection, it is available via $this->db. The database connector is already loaded in the parent class. Please update the code accordingly.

i have changed this. hope it is fine so ;)

roland-d · 2015-11-02T15:49:09Z

Back to pending as we need one more minor code change.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8132.}

roland-d · 2015-11-02T16:14:12Z

administrator/components/com_config/model/application.php

Uh no, not like this. Remove line 345 and 346. From line 351 and later replace $db with $this->db. Thanks.

joomla-cms-bot · 2015-11-02T16:22:31Z

This PR has received new commits.

CC: @coolman01, @designbengel, @DGT41, @zero-24

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8132.}

roland-d · 2015-11-02T16:23:54Z

Code looks good. Thanks. Can we have one more round of tests please? Thanks.

mironsavan · 2015-11-02T16:37:04Z

I have tested this item ✅ successfully on 8a576c7

Works as expected!

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8132.}

mironsavan · 2015-11-02T16:37:27Z

I have tested this item ✅ successfully on 8a576c7

I have tested this succcessfull on my test site with some different combinations and it works for me

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8132.}

dgrammatiko · 2015-11-02T20:18:47Z

I have tested this item ✅ successfully on 8a576c7

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8132.}

zero-24 · 2015-11-02T20:31:59Z

RTC Thanks for you work and the tests 👍

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8132.}

infograf768 · 2016-05-19T08:16:10Z

This has broken "Inherited", in the sense that choosing inherited always calculates as "Denied"
#10552

infograf768 · 2016-05-29T06:34:32Z

We also have other bugs for Permissions.
See this one:

tflm84 added 2 commits October 23, 2015 13:38

implement new way to store permissions via single ajax request

e0a849e

add version tags

1d32ea3

joomla-cms-bot added Language Change This is for Translators PR-staging labels Oct 23, 2015

zero-24 added 3 commits October 23, 2015 15:02

Update store.php

8e87880

Update application.php

581d40f

Update en-GB.lib_joomla.ini

bf38a53

zero-24 mentioned this pull request Oct 23, 2015

Fix some CS sync the language file and add the correct @since tag tflm84/joomla-cms#1

Merged

zero-24 added this to the Joomla! 3.5.0 milestone Oct 23, 2015

zero-24 added the Feature label Oct 23, 2015

Merge pull request #1 from zero-24/patch-1

a52cdee

Fix some CS sync the language file and add the correct @SInCE tag

Fedik reviewed Oct 23, 2015
View reviewed changes

tflm84 added 2 commits October 23, 2015 16:43

delete semicolons and whitespaces in application model

4c79ac1

edit jQuery Tag and delete logs

6c38d23

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Oct 24, 2015

roland-d removed the RTC This Pull Request is Ready To Commit label Oct 24, 2015

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Oct 24, 2015

put javascript in seperated file

83028f9

renamed javascript files

cfc3585

zero-24 mentioned this pull request Nov 2, 2015

Just a very mirror ;) one space to much here tflm84/joomla-cms#2

Merged

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Nov 2, 2015

dgrammatiko reviewed Nov 2, 2015
View reviewed changes

zero-24 added 2 commits November 2, 2015 15:06

Implement @DGT41's suggestion here.

2b66adb

Update rules.php

3b28348

Merge pull request #2 from zero-24/patch-1

dce6db5

Just a very mirror ;) one space to much here

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Nov 2, 2015

roland-d reviewed Nov 2, 2015
View reviewed changes

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Nov 2, 2015

change db connection

c12da13

roland-d reviewed Nov 2, 2015
View reviewed changes

change db connection #2

8a576c7

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Nov 2, 2015

roland-d closed this in 88bd35c Nov 2, 2015

Fedik mentioned this pull request Nov 7, 2015

3.5beta1 - Permission editing is broken #8321

Closed

Fedik mentioned this pull request Nov 21, 2015

Slow Menu Manager due to unresponsive script media/jui/js/jquery.min.js:2 #8515

Closed

infograf768 mentioned this pull request Nov 26, 2015

Proposal to correct some language strings values #8541

Merged

wilsonge removed the RTC This Pull Request is Ready To Commit label Jan 17, 2016

Uh oh!

Conversation

tflm84 commented Oct 23, 2015

Introduction

Fix

How to test this patch

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coolman01 commented Oct 24, 2015

Uh oh!

coolcat-creations commented Oct 24, 2015

Uh oh!

zero-24 commented Oct 24, 2015

Uh oh!

roland-d commented Oct 24, 2015

Uh oh!

joomla-cms-bot commented Oct 25, 2015

Uh oh!

tflm84 commented Oct 25, 2015

Uh oh!

dgrammatiko commented Oct 25, 2015

Uh oh!

dgrammatiko commented Oct 25, 2015

Uh oh!

zero-24 commented Nov 2, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dgrammatiko commented Nov 2, 2015

Uh oh!

zero-24 commented Nov 2, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

roland-d commented Nov 2, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joomla-cms-bot commented Nov 2, 2015

Uh oh!

roland-d commented Nov 2, 2015

Uh oh!

mironsavan commented Nov 2, 2015

Uh oh!

mironsavan commented Nov 2, 2015

Uh oh!

dgrammatiko commented Nov 2, 2015

Uh oh!

zero-24 commented Nov 2, 2015

Uh oh!

infograf768 commented May 19, 2016

Uh oh!

infograf768 commented May 29, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants