Joomla! Issue Tracker | Joomla! CMS #10459 - small regression: [com_menus] items view: use empty for all menu instead of *

? ? ?

Fixed in Code Base
23 May 2016
Medium
Build: staging
# 10459
Diff
andrepereiradasilva:dont-use-asterisk

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:

andrepereiradasilva
14 May 2016

Summary of Changes

The "All Menu Items" was implemented in #10190 (which used empty as the value for the "- Select Menu -" option), but after the menus ACL was implemented in #9814 (which changed to * the value for the "- Select Menu -" option).

So, this PR goes back to (empty) as the value for the "- Select Menu -" option.

This allows to use /administrator/index.php?option=com_menus&view=items&menutype=URL (without a 403) as a fallback (in case the menu type is not found or something). This URL lists the all menu items list view.

Testing Instructions

Use latest staging
Go to /administrator/index.php?option=com_menus&view=items&menutype= you will get a 403 error
Now apply patch
Go to /administrator/index.php?option=com_menus&view=items&menutype= you'll be redirect to "All Menu Items" with no 403 error
Navigate through the menus and create/Edit menu items and check if you're properly redirected after without issues
Test batch and other operations also to confirm all is ok.

@infograf768 @bembelimen since you're the original creators of the two PR mentioned please test if all work as it should.

Observations

After this PR if you use:

/administrator/index.php?option=com_menus&view=items you'll be redirect to your current state selected menu item ("All Menu Items" if none).
/administrator/index.php?option=com_menus&view=items&menutype= you'll be redirect to "All Menu Items"
/administrator/index.php?option=com_menus&view=items&menutype=validmenutype you'll be redirect to your valid menu type
/administrator/index.php?option=com_menus&view=items&menutype=invalidmenutype you'll get a error the menu is not found
/administrator/index.php?option=com_menus&view=items&menutype=validmenutypebutnopermission you'll get a error not autorized

146375b 14 May 2016

use empty for all menu instead of *

7000037 14 May 2016

codestyle and fix colspan

andrepereiradasilva - open - 14 May 2016

andrepereiradasilva - change - 14 May 2016

Status

New

⇒

Pending

joomla-cms-bot - change - 14 May 2016

Labels

Added: ?

andrepereiradasilva - change - 14 May 2016

The description was changed

andrepereiradasilva - change - 14 May 2016

The description was changed

andrepereiradasilva - change - 14 May 2016

The description was changed

andrepereiradasilva - change - 14 May 2016

The description was changed

ac541f6 14 May 2016

we need an empty menutype here

andrepereiradasilva - change - 14 May 2016

The description was changed

andrepereiradasilva - change - 14 May 2016

Title

…

[com_menus] items view: use empty for all menu instead of * (solves #10458)

[com_menus] items view: use empty for all menu instead of * (solves #10458 - 403 error part)

andrepereiradasilva - change - 14 May 2016

The description was changed

brianteeman - change - 14 May 2016

Category

⇒

Administration

andrepereiradasilva - change - 14 May 2016

The description was changed

andrepereiradasilva - change - 14 May 2016

The description was changed

andrepereiradasilva - change - 14 May 2016

The description was changed

andrepereiradasilva - change - 14 May 2016

Title

…

[com_menus] items view: use empty for all menu instead of * ~~(solves #10458 - 403 error part)~~

[com_menus] items view: use empty for all menu instead of *

andrepereiradasilva - change - 14 May 2016

Title

…

[com_menus] items view: use empty for all menu instead of * ~~(solves #10458 - 403 error part)~~

[com_menus] items view: use empty for all menu instead of *

andrepereiradasilva - change - 14 May 2016

The description was changed

andrepereiradasilva - change - 14 May 2016

The description was changed

Webdongle - comment - 14 May 2016

@andrepereiradasilva
I am confused about the test instructions.

Before applying the patch all test from 2 - 8 work as expected.

If you remove steps 1 and 2 from the test instructions
Then after the Test instructions you have
Expected result
describe what should be seen

Actual result
describe the error

Then .. After applying patch describe what error is fixed.

It would be easier to test. Because at the moment I am unable to reproduce the error this is supposed to fix.

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

andrepereiradasilva - change - 14 May 2016

The description was changed

andrepereiradasilva - comment - 14 May 2016

ok sorry, test instructions updated, now i think it's easier

andrepereiradasilva - change - 14 May 2016

The description was changed

Webdongle - test_item - 14 May 2016 - Tested successfully

Webdongle - comment - 14 May 2016

I have tested this item successfully on ac541f6

Patch works

FULL SUCCESS

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

infograf768 - test_item - 15 May 2016 - Tested successfully

infograf768 - comment - 15 May 2016

I have tested this item successfully on ac541f6

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

infograf768 - change - 15 May 2016

Title

…

[com_menus] items view: use empty for all menu instead of *

small regression: [com_menus] items view: use empty for all menu instead of *

Status

Pending

⇒

Ready to Commit

joomla-cms-bot - change - 15 May 2016

Title

…

[com_menus] items view: use empty for all menu instead of *

small regression: [com_menus] items view: use empty for all menu instead of *

infograf768 - comment - 15 May 2016

RTC. Has to go into 3.6.0 as this is a new bug introduced by #9814

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

joomla-cms-bot - change - 15 May 2016

Labels

Added: ?

bembelimen - comment - 15 May 2016

Hello,
/administrator/index.php?option=com_menus&view=items&menutype=existing-menu-without-access-rights
Expected result: 403
Current result: show all items

bembelimen - test_item - 15 May 2016 - Tested unsuccessfully

bembelimen - comment - 15 May 2016

I have tested this item unsuccessfully on ac541f6

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

brianteeman - change - 15 May 2016

Status	Ready to Commit	⇒	Pending
Labels

brianteeman - comment - 15 May 2016

Oops didnt see the failed test -set back to pending

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

joomla-cms-bot - change - 15 May 2016

Labels

Removed: ?

Webdongle - comment - 15 May 2016

@bembelimen

/administrator/index.php?option=com_menus&view=items&menutype=existing-menu-without-access-rights

What menu "menu-without-access-rights" ? If a user has access to the Menus >>> Manage >>> Menus view then they have access to all menus.

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

brianteeman - comment - 15 May 2016

There is a new acl feature in 3.6

andrepereiradasilva - comment - 15 May 2016

@bembelimen thanks for checking that. Will check.

infograf768 - comment - 15 May 2016

i tested tha acl and it was working fine. here.

a86f184 15 May 2016

solve acl issues

joomla-cms-bot - comment - 15 May 2016

This PR has received new commits.

CC: @bembelimen, @infograf768, @Webdongle

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

joomla-cms-bot - change - 15 May 2016

Labels

Added: ?

andrepereiradasilva - comment - 15 May 2016

@bembelimen please check now.
The code changes were this: a86f184

andrepereiradasilva - change - 15 May 2016

The description was changed

andrepereiradasilva - change - 15 May 2016

The description was changed

andrepereiradasilva - comment - 15 May 2016

BTW @bembelimen there are some questions i notice in the menus ACL:

When you have a user that can't do anything (all denied) for a particular menu type:
updated see comment below

andrepereiradasilva - comment - 15 May 2016

Update: Sorry i had "Access Administration Interface" set to Denied for that menu.

But still that is one issue (when all set to denied). that user can view/edit that menu type in Menu Types Manage (bug?)

BurtNL - test_item - 17 May 2016 - Tested successfully

BurtNL - comment - 17 May 2016

I have tested this item successfully on a86f184

I have applied this patch on the latest staging together with #10460.
It is working as described. So this test is successfull.

I have not tested the new ACL feature.

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

bembelimen - comment - 17 May 2016

Hello @andrepereiradasilva

But still that is one issue (when all set to denied). that user can view/edit that menu type in Menu Types Manage (bug?)

The access rights are for the item(s) view only, not for the menu type stuff itself, there work the regular ACL settings

andrepereiradasilva - comment - 18 May 2016

The access rights are for the item(s) view only, not for the menu type stuff itself, there work the regular ACL settings

@bembelimen
I see it was implemented that way, but IMHO doesn't make much sense a user can delete the entire menu and don't have access to delete a menu item in the same menu.

I made some changes (in new PR) to implement, at least part, of the ACL also in the menu types list view.

infograf768 - test_item - 22 May 2016 - Tested successfully

infograf768 - comment - 22 May 2016

I have tested this item ✅ successfully on a86f184

Except that we do not get a 403 but a 500 with message "You are not authorised to view this resource", the changes do now work when the menutype has no access rights when using:

/administrator/index.php?option=com_menus&view=items&menutype=existing-menu-without-access-rights

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

infograf768 - change - 22 May 2016

Status

Pending

⇒

Ready to Commit

infograf768 - comment - 22 May 2016

RTC

Thanks

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

joomla-cms-bot - change - 22 May 2016

Labels

Added: ?

brianteeman - change - 22 May 2016

Milestone

Added:

infograf768 - comment - 22 May 2016

Conflicts concern only one file.

7c9b6c3 22 May 2016

fix conflicts 1

joomla-cms-bot - comment - 22 May 2016

This PR has received new commits.

CC: @bembelimen, @BurtNL, @infograf768, @Webdongle

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

9935400 22 May 2016

Merge branch 'staging' into dont-use-asterisk

joomla-cms-bot - comment - 22 May 2016

This PR has received new commits.

CC: @bembelimen, @BurtNL, @infograf768, @Webdongle

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

8e294e3 22 May 2016

fix conflicts final

joomla-cms-bot - comment - 22 May 2016

This PR has received new commits.

CC: @bembelimen, @BurtNL, @infograf768, @Webdongle

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

andrepereiradasilva - comment - 22 May 2016

conflicts fixed

infograf768 - test_item - 22 May 2016 - Tested successfully

infograf768 - comment - 22 May 2016

I have tested this item ✅ successfully on 8e294e3

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

roland-d - change - 23 May 2016

Status	Ready to Commit	⇒	Pending
Labels

roland-d - comment - 23 May 2016

Taking RTC off as we only have one successful test so far

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

joomla-cms-bot - change - 23 May 2016

Labels

Removed: ?

bembelimen - test_item - 23 May 2016 - Tested successfully

bembelimen - comment - 23 May 2016

I have tested this item ✅ successfully on 8e294e3

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

andrepereiradasilva - comment - 23 May 2016

@roland-d RTC now?

thanks for testing @infograf768 @bembelimen

brianteeman - change - 23 May 2016

Status	Pending	⇒	Ready to Commit
Labels

brianteeman - comment - 23 May 2016

Back to rtc

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

joomla-cms-bot - change - 23 May 2016

Labels

roland-d - comment - 23 May 2016

I am looking into it

roland-d - change - 23 May 2016

Status	Ready to Commit	⇒	Fixed in Code Base
Closed_Date	0000-00-00 00:00:00	⇒	2016-05-23 14:09:15
Closed_By		⇒	roland-d
Labels	Added: ?

roland-d - comment - 23 May 2016

Thanks everybody

Add a Comment

Older
Newer

Joomla! Issue Tracker - CMS

[#10459] - small regression: [com_menus] items view: use empty for all menu instead of *

Summary of Changes

Testing Instructions

Observations

Add a Comment