Ok, what is Drone and what do I have to do to make my PR pass it?

@Napoleon-BlownApart

Ok, what is Drone and what do I have to do to make my PR pass it?

It handle our codestyle checks now ?
http://213.160.72.75/joomla/joomla-cms/204

FILE: ...mla/joomla-cms/administrator/components/com_menus/controllers/items.php
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 157 | ERROR | Please start your comment with a capital letter; found "// set
     |       | the items Inheritable."
--------------------------------------------------------------------------------
UPGRADE TO PHP_CODESNIFFER 2.0 TO FIX ERRORS AUTOMATICALLY
--------------------------------------------------------------------------------
FILE: /drone/src/github.com/joomla/joomla-cms/libraries/cms/menu/site.php
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 173 | ERROR | Please start your comment with a capital letter; found "//
     |       | record the View Access Levels required for this Menu Item."
--------------------------------------------------------------------------------
UPGRADE TO PHP_CODESNIFFER 2.0 TO FIX ERRORS AUTOMATICALLY
--------------------------------------------------------------------------------

You can check the error on the Details button.

Just one more suggestion. For the core we use for the @since tag __DEPLOY_VERSION__ this got replaced by bumping the version before the release so you don't need to take care in which version you change gets merged.

@zero-24 : Thanks. I had to refresh my page before I could see that, and I've already fixed the problems.

@zero-24 : regarding the @since tag, do you mean in the doxy function header? So instead of putting the date, as I did, I would use __DEPLOY_VERSION__?

Testing Instructions

Setup

Install latest staging (that includes my Inheritable branch)
Enable users to register (or manually create the users; one public, one registered).
Set "Guest User Group" to Public in Global Configuration -> Users: Options -> User Options.
Create a menu with 3 items, Home, Register, Edit Profile.
Set the Home menu item to Public Access with _Inheritable_ YES (default).
Set the Register menu item to Public Access but with _Inheritable_ NO (either on the Menu:Item Edit or Menu:Items screens)
Set the Edit Profile menu item to Registered Access with Inheritable YES (default).

Test 1: Visibility of the Menu Item to users.

Test 1.1

Open a browser and visit the site (make sure you are not logged in).
In the menu, you will see menu items "Home" and "Register".

Test 1.2

Register on the site (or login with a registered user)
In the menu, you will only see menu items "Home" and "Edit Profile" ("Register" will not be shown).
Log out.
In the menu, you will see menu items "Home" and "Register".

Test 2: Managing the Inheritable as an Administrator

Test 2.1

On the Menu:Item Edit screen, change the Inheritable property then "Save & Close"
You will see in the Menu:Items list that the menu item reports the state of Inheritable according to your setting.

Test 2.2

On the Menu:Items screen, attempt to change the inheritable property of the DEFAULT page.
You should be presented with an error stating you cannot do this for the default page.

Test 2.3

Choose a non-Default menu item, set  it to un-published AND set it to un-inheritable.
Now choose that same item, on the Menu:Items screen, and make it the Default page.
You should see that this action makes the item both Published as well as Inheritable.

Note I have not implemented this check at the Menu:Item Edit screen level. This allows the default menu item to be un-inheritable if required. If necessary, I can change this behaviour.

Test 3: Language settings

Change the text of the following language dependent variables according to your requirements. You will need to add these for languages other than en-GB.
Revisit the administrator pages to see your settings.

In administrator/language/en-GB/en-GB.com_menus.ini

COM_MENUS_HTML_SETINHERITABLE_ITEM
COM_MENUS_HTML_UNSETINHERITABLE_ITEM
COM_MENUS_ITEM_FIELD_INHERITABLE_DESC
COM_MENUS_ITEM_FIELD_INHERITABLE_LABEL
COM_MENUS_ITEMS_SET_INHERITABLE_0
COM_MENUS_ITEMS_SET_INHERITABLE_1
COM_MENUS_ITEMS_SET_INHERITABLE_MORE
COM_MENUS_ITEMS_UNSET_INHERITABLE

In _ administrator/language/en-GB/en-GB.ini_

JINHERITABLE
JUNIHERITABLE
JERROR_LOADING_MENUACCESSLEVEL
JGRID_HEADING_INHERITABLE_ASC
JGRID_HEADING_INHERITABLE_DESC
JOPTION_SELECT_INHERITABLE

In _ administrator/language/en-GB/en-GB.lib_joomla.ini_

JLIB_DATABASE_ERROR_MENU_UNINHETITABLE_DEFAULT_HOME

In _ language/en-GB/en-GB..ini_

JERROR_LOADING_MENUACCESSLEVEL

In _ language/en-GB/en-GB.lib_joomla.ini_

JLIB_DATABASE_ERROR_MENU_UNINHETITABLE_DEFAULT_HOME

I've prepared two implementations of the same scenario. Before I continue, I need to qualify that I am not an expert in Joomla ACL, let alone Joomla in general. So I may not be the best person to argue the benefits of this feature, but here goes....

Feedback concerning my scenario set up are most welcome.

Justification

1 The current technique to provide non-inheritable User Group specific content (I will use the group extension -exclusive for this) requires the creation of additional child User Groups that are leaf nodes in the tree. Thus, up to twice as many User Groups need to be set up to facilitate this. In addition, up to twice the number of View Access Level needs to be set up to allow exclusive access.

2 Content assigned to the -exclusive is outside the normal hierarchical structure of the User Group inheritance, and thus needs to be configured separately. This is particularly relevant to Category access and permission granting. Category Lists do not show content that is separated out for exclusive access.

3 Since ACL is so important, the Inheritable attribute makes the task of organising and checking access and permissions much easier. The small and relatively simple scenario I set up was very complicated to achieve (and I would like feedback from those more familiar with Joomla ACL configuration) because of the dual User Group/Access Level problem. Using the Inheritable property, the task was greatly simplified, which ensured the quality of the outcome.

4 With fewer User Groups and Access Levels, there is a significant saving in the size of the database. You will notice that the Inheritable version of the Akeeba backup is more than 10MB smaller than the standard version. And this is for a very simple scenario.

5 The feature is transparent and has no effect on the Joomla configuration when the default setting of YES is used. Thus you can export/import the sql from one site to another and it will work after you add the Inheritable column to #__menu table. I used this property to simplify the populate of the Inheritable scenario, inside a basic Joomla installation.

6 I believe that Wordpress has won the website publishing (blog) race. I have been a supporter, advocate, and user of Joomla since around 2006. I have never used Wordpress but I see it in use all over the place when I google. I have studied the architecture of Drupal but never implemented a site with it. I think Joomla is a terrific CMS, but I think it's time Joomla moves beyond the author/editor/publisher paradigm. I believe Joomla filling the gap between Wordpress and Drupal. Joomla has a nice framework and is a lot easier to work with than Drupal, but much more sophisticated than Wordpress, as demonstrated by the large number of off-the-shelf applications that can be installed into Joomla (eg Virtumart). An object molded into shape is stronger than an object beaten into shape.

Scenario Details

The scenario is of a company with the following user types: public, registered customers, general employees, a sales department, and a warehousing department. Each of these groups has access to appropriate content (which sometimes overlaps). Images follow below.

I have prepared two Akeeba backups (using MySQL), one that implements the scenario using standard Joomla, and the other with the Inheritable modification. I have used Joomla 3.6.5 for this as I had some issues with the staging version during (Take 2).

A detailed scenario description (text file) can be downloaded from https://www.abms.net.au/public/inheritable_comparison_scenario.txt
The scenario implemented in a vanilla 3.6.5 install using the standard Joomla approach can be downloaded from https://www.abms.net.au/public/site-jmfull.int-20170208-070014.jpa
The scenario implemented in a modified 3.6.5 install (to enable Inheritance) can be downloaded from https://www.abms.net.au/public/site-jminherit.int-20170208-070257.jpa

Sample images comparing the scenarios:
LHS/Above is standard, RHS/Below is with Inheritable.

• Screen showing Employee Coordinator logged in and viewing the Employees link:
  
• Screen showing Menus: Items (Main Menu):
  
• Screen showing Users: Groups:
  
• Screen showing User: View Access Levels:
  
• Screen showing Users: (All passwords are test.)
  

@zero-24 : regarding the @SInCE tag, do you mean in the doxy function header? So instead of putting the date, as I did, I would use __DEPLOY_VERSION__?

Correct ?

Could someone help me understand what Appveyor is doing?

Tests the code on a windows machine (basically the MSSQL)

Next question.

From the output I see in the details link, for all three cases, it seems the errors are beyond my control. I don't know what I can do to fix the problem.

In (1) and (3) Appveyor is unable to download the php it needs because the URL is incorrect:

(1) http://windows.php.net/downloads/releases/archives/php-5.6.30-nts-Win32-VC11-x86.zip
(3) http://windows.php.net/downloads/releases/archives/php-7.1.2-nts-Win32-VC14-x64.zip

And in (2), Appveyor is unable to access https://github.com in order to clone the repo.

:) Appveyor worked.

@Napoleon-BlownApart i will test as soon as i understand PR.

@franz-wohlkoenig : Thanks. If you have any questions or concerns, please email me at napoleon_blownapart [@] abms.net.au. My Skype address is the same as my nick here except replace the hyphen with a period.

applied PR shows 404 - Component not found. on Homepage, no Menu is shown.

@franz-wohlkoenig : Let me see and I'll fix what needs fixing. Might have forgotten to replicate some code from the previous PR.

@franz-wohlkoenig : I just pulled from my origin/inheritable branch and did a build here. Everything worked fine.

git remote -v

user@host:[21:33]:~/Downloads/joomla-cms$ git remote -v
origin  git@github.com:Napoleon-BlownApart/joomla-cms.git (fetch)
origin  git@github.com:Napoleon-BlownApart/joomla-cms.git (push)
upstream        git@github.com:joomla/joomla-cms.git (fetch)
upstream        git@github.com:joomla/joomla-cms.git (push)

@franz-wohlkoenig : If there is anything I can do, please let me know.

i should test https://github.com/Napoleon-BlownApart/joomla-cms/archive/staging.zip?

@franz-wohlkoenig : That's not how I've done it. When I build, I test the build from joomla-cms/build/tmp/packages_full3.7.0-beta3, and have tried the Joomla_3.7.0-beta3-dev-Full_Package.zip and Joomla_3.7.0-beta3-dev-Update_Package.zip files. I hope that is the correct way to do it?

No idea what generates the link you referenced.

No idea what generates the link you referenced.

if PR are not in latest staging its a way to test.

I tested on rc1-nightly-build and got the 404-Error (comment)

@franz-wohlkoenig : let me know if there is something I should do. At the moment, I have no idea. I needed to do a merge since I proposed this version, but as you can see the CI results, it all is supposed to be working.

@franz-wohlkoenig
Use
https://github.com/Napoleon-BlownApart/joomla-cms/archive/inheritable.zip

Some comments:
The new ini strings are not alpha ordered and some of the English has to be reviewed
COM_MENUS_ITEMS_UNSET_INHERITABLE="1 menu item successfully had inheritable unset."
is a JPlural. You need 3 strings in core, with a better English

typo in constant and code
JLIB_DATABASE_ERROR_MENU_UNINHETITABLE_DEFAULT_HOME

Problem merging tests/unit/schema/sqlsrv.sql

Test 2.2. fails on a multilanguage site where we have multiple home pages. It works only for the Home set to ALL languages.

Test 2.3 fails for the same reason

Code should be
if ($table->load($pk) && $table->home) (language is not concerned in your case).

@infograf768 : What do you mean by JPlural, and '3 strings in core`?

You did it for set but not for unset

+COM_MENUS_ITEMS_SET_INHERITABLE_0="No menu items set to inheritable."
+COM_MENUS_ITEMS_SET_INHERITABLE_1="1 menu item successfully set to inheritable."
+COM_MENUS_ITEMS_SET_INHERITABLE_MORE="%d menu items successfully set to inheritable."
+COM_MENUS_ITEMS_UNSET_INHERITABLE="1 menu item successfully had inheritable unset."

You should also have
COM_MENUS_ITEMS_UNSET_INHERITABLE_0
COM_MENUS_ITEMS_UNSET_INHERITABLE_1
COM_MENUS_ITEMS_UNSET_INHERITABLE_MORE

and no
COM_MENUS_ITEMS_UNSET_INHERITABLE

I guess also the basic text should be
successfully set to **not inheritable** as it looks like that un-inheritable as well as uninheritable do not exist in en-GB although wiktionary has a page for it.

@infograf768 : I have:

1. Corrected the code in item.php that caused the tests to fail (erroneous ALL language setting requirement),
2. Used not inheritable as you suggested,
3. Corrected the typo in the DB ERROR token,
4. Added COM_MENUS_ITEMS_UNSET_INHERITABLE_0, 1, MORE and removed COM_MENUS_ITEMS_UNSET_INHERITABLE, and
5. Re-ordered the ini strings I added (as well as a few others that were near mine and out of order).

See the next commit.

Much better. :)

remains tests/unit/schema/sqlsrv.sql which is not modified to include inheritable and in any case does not merge here with eclipse.

@infograf768 : I have already updated tests/unit/schema/sqlsrv.sql (line 977). What else should I have updated? Like indexes? Is this script generated by some other process?

@csthomas : installation/sql/sqlazure/joomla.sql: looks like its been copy/pasted (or generated ) from an SQL Export/Dump into this file, probably from phpMyAdmin. The file has changed entirely since I made my changes. The field delimiter has changed from square brackets to MySQL style double quotes throughout the entire file.

All I need to do to fix this merge conflict is add "inheritable" tinyint NOT NULL DEFAULT 1, after "access" int NOT NULL DEFAULT 0, in CREATE TABLE "#__menu" (.

However, there are some changes I am not in a position to decide on. They are:

1. The staging version has SET IDENTITY_INSERT "#__languages" ON; just before the INSERT INTO "#__languages" statement, where my version had SET IDENTITY_INSERT [#__languages] OFF;. Should it be ON or OFF?
2. The staging version is missing, when compared to my version, a SET QUOTED_IDENTIFIER ON; statement just before the creation of the #__menu table. Is it necessary or not?

@csthomas : Can you please advise on what is the correct merge so I can push back to the PR.

@csthomas : Not sure if you got notification of the above comments as I added your name after my original posting, once I found out that you were the last to push the installation/sql/sqlazure/joomla.sql file.

I have changed mssql sql files to be more similar to postgresql file.

Means [, ] to ", simpler INSERT and maybe a few more.

I generated it from postgresql file, it is not "dump from db".
Sqlsrv is a copy of postgresql with additional changes for sqlsrv.

The staging version has SET IDENTITY_INSERT "#__languages" ON; just before the INSERT INTO "#__languages" statement, where my version had SET IDENTITY_INSERT [#__languages] OFF;. Should it be ON or OFF?

If you use primary key auto increment column in insert like "INSERT INTO content (id, title, ...) VALUES (1, 'Hello'), ..."
then you have to use SET IDENTITY_INSERT "table" ON; because you don't use AUTO_INCREMENT functionality. After you complete please turn it OFF.

The staging version is missing, when compared to my version, a SET QUOTED_IDENTIFIER ON; statement just before the creation of the #__menu table. Is it necessary or not?

This is not needed any more because it was added to driver file on creating connection:
https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/database/driver/sqlsrv.php#L153

Instead [ or ] I suggest to use " as name quote because QUOTED_IDENTIFIER is always ON.
Do not quote column type like nvarchar Instead [nvarchar] use simpler nvarchar.

@csthomas : Cheers, I'll do the merge next.

@csthomas : done. If there are problems, I might ask your advice.

@infograf768 your comment helps – now i can test.

I have tested this item ? unsuccessfully on 86a169f

Using default Settings of User Groups, Viewing Access Levels; Parent Item of Menu Registered is Menu Item Root of Main Menu:

Menu

• Registration Form set to Access: Public
• Inheritable: No

Frontend

Menu Register isn't shown to Public or any Group.

Menu

• Registration Form set to Access: Public
• Inheritable: Yes

Frontend

Menu Register is shown to Public and logged-in Users.

Tested installed https://github.com/Napoleon-BlownApart/joomla-cms/archive/inheritable.zip downloaded Today with- and without PR applied by Patchtester.

@franz-wohlkoenig : Please set the Default User Group to Public, not Guest. Guest inherits from Public, but the menu item is set to not inheritable. So the test actually produced the correct result. The second test also produced the correct, default Joomla, behaviour.

I have tested this item ✅ successfully on 86a169f

The "Register"-Menu-Example works – but i hang on understanding why. Maybe some Real-Life-Examples more would help.

@franz-wohlkoenig : Points 1-3 in my justification above, #14116 (comment) explain the benefits. In the Senario Details section (same comment), I give a more robust example, with screenshots, and have provided Akeeba backups that demonstrate the benefit.

@Napoleon-BlownApart i have missed "Scenario Details", will search for them. There was a lot of technical Discussions which i skip.

I'm curious as to what the status of this proposal is?

@Napoleon-BlownApart it needs a second successfully Test and Maintainers decide to merge PR.

Cheers @franz-wohlkoenig. @Bakual referred this to @sanderpotjer on my (Take2) Pull Requeust.

there are 2 different PR for same Issue?

No, only this one (Take3). (Take1) and (Take2) are both closed.
This was the first fork/pull request I've done, and I had some troubles with Travis etc. (Take3) is my 3rd attempt and all the commits have been cleaned up to make it easier to follow the development.

@Bakual can you referre @sanderpotjer on this Issue?

You already did by mentioning him ;)

@Napoleon-BlownApart many thanks for your Pull Request(s), the detailed information and the Akeeba files to test! I have been looking into this and I do understand what you are trying to achieve.

The example of the Registration menu-item is correct but could confuse people to be honest. I think your initial example in the forum post (https://forum.joomla.org/viewtopic.php?f=708&t=945868&p=3457261#p3457261) is a good one:

A given menu item can have it's access level determined. However, the assigned access level is in fact the root of a branch that includes higher authorities, and therefore access to the resource is not limited to the assigned group alone.
This is causing me problems in that (using the default access levels) I want to have a menu item displayed for an author but not for their editor/publisher. And I cannot invoke the permission system to help me here since the permission system overlooks the fact that viewing is in itself an action.
Anyone have any suggestions on how can I have a menu item that only shows for an author and the editor does not see it (since they don't need it and I want to maintain a clean UI for my users)?

However, my main concern is that this approach works for menu-items, but not for any other content set to an access level (articles, modules, etc..). Let me explain:

So when you log in as an employee manager (user: eman) you can't see and access the menu item "Become a Employee Coordinator" as this is set to Employee-Exclusive access. URL:
http://domain.com/employees/become-a-employee-coordinator

In your setup including the patch the same user can't see the menu item when "Inheritable" is unset (like before), but the can access the url directly: http://domain.com/employees/become-a-employee-coordinator

This is also visible in your screenshot where you can see more articles in the inherit setup:

So your PR works for menu-items, but not for other areas, which is confusing I think.

Right now the only solution I can think of is that all "Access" settings do get the toggle like you created for the menu-item:

Which possible need to be renamed to "Access Inheritable" as it is not clear now that it is related to the access. The description is also not correct as it is "Access Levels" instead of "Higher Groups". So maybe: Determines if this menu item can be viewed by child Access Levels.

Another thing I doubt about in the current setup is the toggle in the list view. Right now you can't change the Access in the view directly in Joomla, only by opening the item or via the batch button. So wouldn't it be more logical if this applies for the Inheritable column too? And maybe change the access to something like:

As you can see, thinking about what labels would be clear for end-users... Not sure if "Inheritable" is clear enough.

To summarise: technically your PR is correct, but I am not sure about the concept. I fully understand the issue, but it might need a different solution to cover all access settings in Joomla, and not just for the menu-items.

@sanderpotjer thanks for having a look. I'll have a think about your comments and the amount of effort needed to expand the scope. I'm away next week, so I'll get back to this afterwards.

@Napoleon-BlownApart any Update?

@franz-wohlkoenig , @sanderpotjer : Thanks for the reminder. Clearly changing the description/text fields for the functions is simple and I'm happy to go with the suggestions being made. That ensures a consistency across Joomla as a whole. But, as I am not a Joomla power user, if you would be so kind, or sanderpotjer , to enumerate the other screens/areas where this patch would need to be applied, it would help me understand exactly what needs to be done.

Just reminding @sanderpotjer

@Napoleon-BlownApart thx for your time you invested into this PR and Joomla!
This PR will not be added to J3.x because of our no new feature policy.

To get this integrated into J4 it have to be rebased and maybe rethought a bit. Maybe it would be great if you add a ned RFC issue to talk about this feature.

In the meantime I'm closing this PR.

Thx for trying to make Joomla! a better CMS.

@HLeithner : Sorry but I've been really busy with other things I'm working on. I think your idea of an RFC is good as it will enable me to understand the larger scope you've mentioned. In the end, I think I will be able to implement whatever is agreed to. How/where can I open the RFC?

@Napoleon-BlownApart Type in Title "[RFC]" like #23778