@alikon It needs schema updates 3.7.3-2017-06-11.sql (or similar).

yes but,
just want to know if can be considered expected/correct behaviour
cause we are adding

Well it will run after the schema updates of my PR #16577 , which will delete any present menutype main, so it should work if you insert it again with the correct client ID. So from my point of view it would be OK. But I don't know if it is desired to have that menutype because it is filtered out anyway in the menu items list.

Maybe some experienced maintaner who is dealing with the admin menu and the new possibility to add user-defined admin menus should have a look on it, too.

I think in 4.0 or whenever possible we should start to use real foreign keys in the database anyway, if that is possible.

using ENGINE=InnoDB we can use foreign keys even before 4.0

well i have no idea why that hes not been done in past.

IIRC part of the problem is JTable doesn't cope with key relations well (if
at all). And we don't have proper routines in place in PHP to ensure the
keys retain integrity.

On Sun, Jun 11, 2017 at 12:19 PM Richard Fath notifications@github.com
wrote:

well i have no idea why that hes not been done in past.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#16625 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfoT5kfYjeyh3gBGWXN6a0VU82_sbnks5sDCGYgaJpZM4N2cdg
.

--

• Michael Please pardon any errors, this message was sent from my iPhone.

@mbabker

And we don't have proper routines in place in PHP to ensure the keys retain integrity.

The database does that for us if we do not bypass that. E.g. deleting a menu type will cause an exception if there are menus referring to it when having an FK. We just have to catch the exception if necessary and display the error message in the right way, depending on where we use the SQL, e.g. when deleting a menu type. No need for any checks in PHP. It would simplify our PHP code much.

In the same way we should handle primary and unique key violations, e.g. duplicate alias stuff. All to be done in PHP is to delete stuff in the right order, e.g. if someone really wants to delete a menu type and all menu items referring to it, catch the foreign key violation on a first delete and then ask for the option to delete also the menu items, and if yes, delete them, too. We would have much less SQL being issued in our PHP if we would let the database do the work for us and we just handle the exceptions in the right way.

I have a dream ;-)

The only thing I am not sure with is ajax stuff, e.g. if database actions are triggered from any js.

I have a dream ;-)

make joomla great again

let the database do the databases works for us
hopefully elections are over ?

Yes, we will also build a wall against WP .. and they will pay for it ;-)

Either way we've got problems to fix and I'd love for us to fix them one
day. The database API, the interactions with it, and the underlying data
model being something that sorely needs someone to bring up to spec.

On Sun, Jun 11, 2017 at 12:30 PM Richard Fath notifications@github.com
wrote:

@mbabker https://github.com/mbabker

And we don't have proper routines in place in PHP to ensure the keys
retain integrity.

The database does that for us if we do not bypass that. E.g. deleting a
menu type will cause an exception if there are menus referring to it when
having an FK. We just have to catch the exceptionif necessary and display
the error message in the right way, depending on where we use the SQL, e.g.
when deleting a menu type. No need for any checks in PHP. It would simplify
our PHP code much.

In the same way we should handle primary and unique key violations, e.g.
duplicate alias stuff. All to be done in PHP is to delete stuff in the
right order, e.g. if someone really wants to delete a menu type and all
menu items referring to it, catch the foreign key violation on a first
delete and then ask for the option to delete also the menu items, and if
yes, delete them, too. We would have much less SQL being issued in our PHP
if we would let the database do the work for us and we just handle the
exceptions in the right way.

I have a dream ;-)

The only thing I am not sure with is ajax stuff, e.g. if database actions
are triggered from any js.

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#16625 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfoXc3rFVvUV19L9MUgE9rlNDFauLnks5sDCQkgaJpZM4N2cdg
.

--

• Michael Please pardon any errors, this message was sent from my iPhone.

@alikon I just wanted to make a PR to your branch for this PR, adding the missing schema update scripts so at least when people update the reglar way, the menu type would be added, too.
Then I noticed that you add the menu type also with the scripts for sample data, but in opposite to the menu type "mainmenu" for the site where ID is always 1, the "menu" menutype for the admin is added with different ID in the sample data sxripts than it is in joomla.sql.
Have you tested if it can cause trouble, i.e. made a new install with your patch and check if sample data installs without errors and then checked in db that all as expected?
I think you would be on the safer side if using always the 2nd record with ID 2 for the new "main" menutype, i.e. in the sample data sql scripts move it to the 2nd record and change IDs of the other menu types coming after that.

@alikon I added a PR to your branch with fixes I suggested in my comment above, see alikon#32. Please check.

One reason against this PR is that in the menu manager we hide menu items with menutype "main" so nobody can accidently or by purpose delete them.

But if we show the menu type in the backend, someobe could delete it, and I am really not sure if not all the menu items belonging it would be deleted too by the PHP code doing that.

So we should hide also the menu type in the backend, or we should protect it and its menu itesm somhow, similar as we do with core extensions in the extension manager.

@richard67
protected the reserved menu "main"

@alikon Hmm, 2 things I noticed when applying the schema update and the patch:

1. The menu manager shows the menu "Admin Main Menu" (menutype: main) now in the "Menus" view, but the menu items for this menu in the "Menu Items" view are not shown, because they are excluded from the query.

I think for consistency the menu should be hidden in the "Menus" view like the menu items are in the "Menu Items" view, or vice versa, both should be shown. I prefer to hide it.

2. The schema update maybe should either delete the new record before inserting it, or it should only insert if it does not exist yet. I know this is paranoid, since this is fixed with a previous schema update for 3.7.0 (which I recently fixed), and J 3.7.x would not allow to create a menu of type "main", but if for some unknown reason there exists such a record, then the insert statement will fail with unique key violation. I am not 100% sure and so I leave it up to you how important it would be to catch that case. This came into my mind after I made the schema updates for you, sorry for that.

@alikon Is this PR still valid?

lack of interest as every db related stuff