Inheritable menu item property

[Napoleon-BlownApart]

Pull Request for Issue #13682.

I have implemented an Inheritable property for menu items. The property determines if higher access levels inherit the menu item. An inheritable menu item will display in the menu for all user groups who inherit visibility of the menu item's set access level. A menu item set to un-inheritable will only be seen by users who belong to the access level group that is set in the menu item.

For example:
With a "Register" menu link (whose access is set to Public with Inheritable NO), a public visitor to the site will be able to register, but once they have activated their account and been upgraded to the Registered user group, they will not see the "Register" link while they remain logged in (assuming the user is not assigned to the Public User Group on the User:Edit-Assigned User Groups screen).
Had the Inheritable property been left ON (default), the menu item's behaviour would have been standard Joomla behaviour, and the Registered user would still be able to see the "Register" link (unless a separte menu was created for registered users or some other hack was implemented).

The Inheritable property is a binary switch. Its setting can be controlled on
1 Menus:Edit Item -> Details screen using the standard binary toggle switch.
2 Menus: Items screen using a button similar to the 'Status -> Publish' button.

From various posts I have read concerning this issue, I believe this to be a simple, elegant, unobtrusive solution to this particular problem. It would be nice if a future release of Joomla incorporated this change, or something similar. For justification of this enhancement, I refer the reader to the Issue post I published #13682

The details of the changes are in the git-diff below.

Cheers,
Nap

Summary of Changes

The changes made apply to the Staging Branch of the Joomla repository as of 24 Jan 2017.
My changes are located in the 'inheritable' branch.
There are two commits; the first is the Menu:Edit Item implementation, the second implements the Menu:Items summary screen changes.

From the Menu:Items screen, you cannot set the Default page to un-inheritable (like you cannot un-publish the home page). I haven't tested this at the Menu:Edit Item level.

SQL changes have been applied for all three database types: MySQL, PostGRE, and SQLAzure.

Testing Instructions

As a novice Joomla contributor/user, these should be tested further in regards to side-effects of extensions and templates.

Menus:Edit Item
Works fine for me.

Menu:Items admin management
Works fine for me.

Documentation Changes Required

The code I've added is documented in compliance with Doxygen. The '@SInCE field for methods has been set to 2017-01-25'.

[Napoleon-BlownApart]

hmmmm. This is my 1st Pull Request.
continuous-integration/travis-ci/pr seems to still be in progress, the other checks have been successful.
What's next?

[Bakual]

The Travis one takes long to finish. That's normal.
Now it has finished, there are some codestyle issues with your PR. Check the link https://travis-ci.org/joomla/joomla-cms/jobs/195476952 and scroll down. It shows you each file and line where an error is.

[Napoleon-BlownApart]

Yes, I noticed what you explained.
When I make the fixes and commit them, I just do another pull request? What about all the comments I put in?

[Napoleon-BlownApart]

I've fixed a number of errors, but I'm not sure what to do with ones like this:

if ($item->home)
{
    ....
}

Ther error message for the above is:

 78 | ERROR | Expected "if (...)\n...{...}\n...else\n"; found "if
    |       | (...)\n...{...}\n...else "

Should I add == true to the if statement? What about if statement is something like:

if (empty(self::$instances[$client]))
{
  ...

This is a bit pedantic, not to mention that I didn't write those lines yet they were in the repo.
Also, it's only happening in this file. Other files where that style of if statement is being used seem to pass.

[Bakual]

When I make the fixes and commit them, I just do another pull request? What about all the comments I put in?

Just push them like you did, it will trigger Travis again. You can do that as many times you want 😄

if ($item->home)
{
....
}

There is a space at the end of the first line. That's probably the cause.

This is a bit pedantic, not to mention that I didn't write those lines yet they were in the repo.

It should only show errors for lines you changed something.

[Napoleon-BlownApart]

if ($item->home)
{
....
}

There is a space at the end of the first line. That's probably the cause.

Thanks for your assistance.
Line 78 is in the constructor method for the JMenu class, and I certainly didn't change it. No spaces anywhere in the entire method, only tabs to set the indents. I also added == true to the if statement in line 78, but am still getting the error.

[Bakual]

Try to fix the others first, maybe Travis choked somehow.
There is a space at the end of line 276 and the } else { on line 283 needs to be

}
else
{

[Napoleon-BlownApart]

After two editors (Notepad++ and UEStudio), there are no extraneous spaces. The else you referred to, I reformatted it to follow the 3 line style. :(

[Napoleon-BlownApart]

Well, the long list is gone (DOS formatting issue I think) but the rest are bogus error and should be warnings really. I'm done.

[Bakual]

Well, the long list is gone (DOS formatting issue I think) but the rest are bogus error and should be warnings really. I'm done.

Travis is just enforcing the codestyle rules we have (see http://joomla.github.io/coding-standards/).

If they not pass, this PR will never be accepted.
If you need assistance, I'm sure someone will help you with fixing them. Maybe @zero-24 has some spare time?

[Napoleon-BlownApart]

Ok, I will try getting help. :) Thanks for yours.

[Napoleon-BlownApart]

I've installed PHPCodesniffer. I'll see how I do tomorrow.

[Napoleon-BlownApart]

Could someone help me with running PHPCS? I'm getting this error:

PHP Fatal error:  Interface 'PHP_CodeSniffer_Sniff' not found in /Users/me/Downloads/Joomla/joomla-cms/build/phpcs/Joomla/Sniffs/Classes/InstantiateNewClassesSniff.php on line 20

when I run the command:

phpcs --report=full --extensions=php -p --standard=build/phpcs/Joomla .

[Napoleon-BlownApart]

From the root folder of the Joomla repository, when I run (as per the Joomla! Coding Standards Manual) CodeSniffer, I get an error. Considering I'm trying to fix documentation issues, this is demoralising. The command:

phpcs --report=checkstyle --report-file=build/logs/checkstyle.xml --standard=build/phpcs/Joomla .

Generates this error:

ERROR: The specified report file path "build/logs/checkstyle.xml" points to a non-existent directory

Including /path/to// in the command makes no difference.

It took less time to write the code than it has to figure out how to include the right number of spaces in the documentation.

[Bakual]

I can't help you with setting up codesniffer. I always rely on Travis. Much simpler 😄

Anyway, you seem to have figured out most issues. The only one left is a line lenght where you can just wrap it.

[Napoleon-BlownApart]

Thanks Bakual! Without your encouragement, I think I may have given up. Yes, I split the SELECT statement into an additional line.

I gave up on CodeSniffer for the moment, but I would like to get it working in the end.

Wallah, it's passed!!! :)

[Napoleon-BlownApart]

@Bakual, what do you mean by "The update files are missing."

Also, I haven't tested the repo version, but would really like too! I've been working with the files I have in my 3.6.5 Stable-Full install.
How do I install (or create the zip installer) from the repo?

[Napoleon-BlownApart]

Travis needs a break! I am running PHP v5.3 and it works on my system yet Travis failed. I did not make any changes to the file:

.....S.............................FPHP Fatal error:  Call to a member function getActive() on a non-object in /home/travis/build/joomla/joomla-cms/libraries/cms/application/site.php on line 325

How do I make the system perform another check when there is no code I wish to alter? Do I need to make a 'dummy' change just to trigger Travis/Jenkins?

[Napoleon-BlownApart]

@infograf768
Higher User Groups inherit from lower User Groups: An Editor inherits viewing privileges from the Author. That is what I mean by 'higher group types'.

[Bakual]

what do you mean by "The update files are missing

Look at https://github.com/joomla/joomla-cms/tree/staging/administrator/components/com_admin/sql/updates/mysql. It contains the files which are applied during an update. There are similar forlders for the other databases.
Just create a new file following the pattern. So 3.7.0-2017-01-27.sql and put the needed SQL commands into it.

[Bakual]

Also, I haven't tested the repo version, but would really like too! I've been working with the files I have in my 3.6.5 Stable-Full install.
How do I install (or create the zip installer) from the repo?

Just download your branch as zipfile (https://github.com/Napoleon-BlownApart/joomla-cms/archive/inheritable.zip)

Do I need to make a 'dummy' change just to trigger Travis/Jenkins?

We can restart it manually if needed.

[Napoleon-BlownApart]

I've been able to do a local build which has created the full and update installers.
I've tested the full install and it now works for me, but not Travis. However, I'm not sure how to test the update installer.

[Bakual]

However, I'm not sure how to test the update installer.

You can try if you apply the PR using the patchtester (joom.la/patchtester) and then use the database fixer. Or you can upload the zip from your branch in a 3.6.5 installation using the Joomla Updater. Both ways should apply the update SQL files

[Bakual]

The error in the unit tests means you either broke something with you PR or the tests need to be adjusted to your change. It looks like fetching the active menu item is broken.

[mbabker]

This pull request is reverting several changes that have been made since 3.6.5, those must be reapplied for this to be considered.

[Bakual]

@Napoleon-BlownApart Did you create your PR based on a 3.6.5 installation? If so that would explain what Michael said. You need to create the PR based on a current staging branch.

[Napoleon-BlownApart]

@Bakual, yes, that was how I originally developed it. However, for the pull request I rewrote and tested using Eclipse PDT and its debugging features. It's not like I copied code from my 3.6.5 install. From the latest commit in my branch (and pushed back to github), the build produced a FULL installer (Joomla_3.7.0-beta1-dev-Full_Package.zip) and it works fine for me. (I don't believe Travis, but what can I do?)

I'm stuck on the update installer because the Joomla 3.4.8 I'm trying to upgrade will not allow me to upload the patch (like you can with extension installs) and wants to access an update server. Even with it set to Custom URL its not working and I haven't been able to figure out the right setup to mimic an update server even though I've handcrafted the xml files to suit.

I've included a link to the Joomla systeminfo from my FULL install.

Thus far I've been working with U14's default php5.5.9. I have now installed php 5.6 and 7.0 on my dev server and will continue with this. Head to head with Travis until it works.

@mbabker: With regards to reverting code, the only place that occurred was in the JMenu constructor (/libraries/cms/menu/menu.php : line 83) because code was removed not fixed, and thus the item's parameters weren't being put in the registry. This was causing errors for me down-stream. I will find a better way of doing this.

I can't upload the text file as an attachment. The error message I get in the window below is:
Something went really wrong, and we can’t process that file.
The text file can be retrieved from https://www.abms.net.au/public/systeminfo-2017-01-29T14_09_18+11_00.txt

[Napoleon-BlownApart]

@Bakual & @mbabker : I updated my previous comment.

And uploaded a System Info text file to https://www.abms.net.au/public/systeminfo-2017-01-29T14_53_04+11_00.txt which is using php 5.6

[mbabker]

The code was changed in a way to lazy load the parameters. See #13073

You need to get your changes in sync with the current development branch, not the 3.6.5 release. From the looks of things you're trying to port code from two different versions which just won't work efficiently, especially when there have been several other changes (query caching in JMenuSite and that PR I linked above being two changes reverted).

[Bakual]

I'm stuck on the update installer because the Joomla 3.4.8

I don't think it works in 3.4.8. The ability to upload a zip directly in the Joomla Updater was added in 3.6.0 or 3.5.0.

[Napoleon-BlownApart]

@Bakual : Yes, I installed 3.6.0 and now am able to use the udate feature. Unfortunately my update failed, and I will debug it after I fix up the issues @mbabker raised.

@mbabker : I will go through my changes again, from a fresh start, but a little advice would help here.

I would like to start this from a fresh 'staging' state. So what I'm thinking is to delete my branch, create it again, and then proceed to make the changes necessary for the Inheritable again. Is this a feasible approach? I don't want to leave this PR (thread) hanging, and I don't want to be developing a poluted brach that is going to give everyone headaches. If the above isn't feasible, what would you recommend?

Also, since a week has gone by, should I be looking to get a more up to date 'staging' branch?

[Napoleon-BlownApart]

I have deleted the Inheritable branch and then recreated it. I will reproduce the changes whilst being mindful of the above comments.

[Napoleon-BlownApart]

I have committed the first stage; Inheritable property managed in the Menu: Edit Item level but it's not showing up here for testing. Travis isn't plaing ball anymore.....

[Napoleon-BlownApart]

Ok, why am I not able to "Reopen and comment"?

[Napoleon-BlownApart]

I have updated the Test Instructions (Setup) to include:

    Set "Guest User Group" to Public in Global Configuration -> Users: Options -> User Options.

[Napoleon-BlownApart]

I have updated the entire Inheritable branch and fixed the issues @mbabker pointed out concerning the caching and callbacks. I don't know how/why but that code was not there when I did my 1st pass. Anyway, @mbabker, your work has now been respected in this version of the pull request.

[Napoleon-BlownApart]

Locally, I am able to build both the full and update packages and have tested both here.
As stated earlier, I have php5.5.9, php5.6 and php7.0 available to my apache2 install and the code works here for all three php versions.

Now I wish Travis would wake up and run my tests.....

[Napoleon-BlownApart]

I created a new PR that correctly implements the changes against staging (@mbabker) with a cleaner commit history. It is PR #13817, and it has passed the tests :)