? ? ?

Closed
2 Nov 2015
Medium
Build: staging
Easy Test
# 8132
Diff
tflm84:staging

Success

Success continuous-integration/travis-ci/pr The Travis CI build passed Details
Success JTracker/HumanTestResults Human Test Results: 2 Successful 0 Failed. Details

User tests: Successful: Unsuccessful:

tflm84
23 Oct 2015

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

e0a849e 23 Oct 2015

implement new way to store permissions via single ajax request

1d32ea3 23 Oct 2015

add version tags

tflm84 - open - 23 Oct 2015

tflm84 - change - 23 Oct 2015

Status

New

⇒

Pending

joomla-cms-bot - change - 23 Oct 2015

Labels

Added: ? ?

8e87880 23 Oct 2015

Update store.php

581d40f 23 Oct 2015

Update application.php

bf38a53 23 Oct 2015

Update en-GB.lib_joomla.ini

zero-24 - change - 23 Oct 2015

Milestone

Added:

zero-24 - change - 23 Oct 2015

Labels

Added: ?

a52cdee 23 Oct 2015

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

4c79ac1 23 Oct 2015

delete semicolons and whitespaces in application model

6c38d23 23 Oct 2015

edit jQuery Tag and delete logs

zero-24 - change - 23 Oct 2015

Labels
Easy	No	⇒	Yes

zero-24 - change - 23 Oct 2015

Category

⇒

Administration UI/UX

coolman01 - test_item - 24 Oct 2015 - Tested successfully

coolman01 - comment - 24 Oct 2015

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.}

designbengel - test_item - 24 Oct 2015 - Tested successfully

designbengel - comment - 24 Oct 2015

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 - change - 24 Oct 2015

Status	Pending	⇒	Ready to Commit
Labels

zero-24 - comment - 24 Oct 2015

RTC

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

joomla-cms-bot - change - 24 Oct 2015

Labels

Added: ?

roland-d - change - 24 Oct 2015

Labels

Removed: ?

roland-d - comment - 24 Oct 2015

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 - change - 24 Oct 2015

Labels

Added: ?

83028f9 25 Oct 2015

put javascript in seperated file

joomla-cms-bot - comment - 25 Oct 2015

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 - comment - 25 Oct 2015

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.

dgt41 - comment - 25 Oct 2015

@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 ?

dgt41 - comment - 25 Oct 2015

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');

cfc3585 25 Oct 2015

renamed javascript files

roland-d - change - 31 Oct 2015

Labels

Removed: ?

roland-d - comment - 31 Oct 2015

@tflm84 Can you please update the PR with the proposed changes? On wednesday November 4th beta 1 goes out and this should be done and tested before that time, otherwise it can't be merged I am afraid. Thanks.

joomla-cms-bot - change - 31 Oct 2015

Labels

Added: ?

7bd6760 1 Nov 2015

remove javascript from core.js and add them to views where permissions are used

09544b9 1 Nov 2015

using jquery instead of javascript

tflm84 - comment - 1 Nov 2015

The proposed changes were implemented and can be tested now

Fedik - comment - 1 Nov 2015

@tflm84 please check the result again
what combobox doing in core.js?

455c0f7 1 Nov 2015

clean core.js and core-uncompressed.js

b621ac6 1 Nov 2015

clean core.js

joomla-cms-bot - comment - 1 Nov 2015

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.}

zero-24 - change - 2 Nov 2015

Status	Ready to Commit	⇒	Pending
Labels

721a222 2 Nov 2015

Update store.php

zero-24 - test_item - 2 Nov 2015 - Tested successfully

zero-24 - comment - 2 Nov 2015

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.}

joomla-cms-bot - change - 2 Nov 2015

Labels

Removed: ?

2b66adb 2 Nov 2015

Implement @dgt41's suggestion here.

3b28348 2 Nov 2015

Update rules.php

dgt41 - test_item - 2 Nov 2015 - Tested successfully

dgt41 - comment - 2 Nov 2015

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.}

dce6db5 2 Nov 2015

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

zero-24 - alter_testresult - 2 Nov 2015 - dgt41: Tested successfully

zero-24 - alter_testresult - 2 Nov 2015 - zero-24: Tested successfully

zero-24 - change - 2 Nov 2015

Status	Pending	⇒	Ready to Commit
Labels

zero-24 - comment - 2 Nov 2015

RTC on testing now. Thanks

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

joomla-cms-bot - change - 2 Nov 2015

Labels

Added: ?

roland-d - change - 2 Nov 2015

Status	Ready to Commit	⇒	Pending
Labels

roland-d - comment - 2 Nov 2015

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.}

joomla-cms-bot - change - 2 Nov 2015

Labels

Removed: ?

c12da13 2 Nov 2015

change db connection

8a576c7 2 Nov 2015

change db connection #2

joomla-cms-bot - comment - 2 Nov 2015

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 - comment - 2 Nov 2015

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

mironsavan - test_item - 2 Nov 2015 - Tested successfully

mironsavan - comment - 2 Nov 2015

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 - test_item - 2 Nov 2015 - Tested successfully

mironsavan - comment - 2 Nov 2015

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.}

dgt41 - test_item - 2 Nov 2015 - Tested successfully

dgt41 - comment - 2 Nov 2015

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 - change - 2 Nov 2015

Status	Pending	⇒	Ready to Commit
Labels

zero-24 - comment - 2 Nov 2015

RTC Thanks for you work and the tests ????

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

joomla-cms-bot - change - 2 Nov 2015

Labels

Added: ?

roland-d - change - 2 Nov 2015

Status	Ready to Commit	⇒	Closed
Closed_Date	0000-00-00 00:00:00	⇒	2015-11-02 20:47:40
Closed_By		⇒	roland-d

roland-d - close - 2 Nov 2015

wilsonge - change - 17 Jan 2016

Labels

Removed: ?

infograf768 - comment - 19 May 2016

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

infograf768 - comment - 29 May 2016

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