? ? Pending

User tests: Successful: Unsuccessful:

avatar infograf768
infograf768
12 Jul 2018

Summary of Changes

Preventing saving the same title for a Menu

Testing Instructions

Create a new Menu administrator/index.php?option=com_menus&view=menu&layout=edit
Use the same title as an existing Menu, for example Main Menu

Before patch

The Menu is saved OK.
screen shot 2018-07-12 at 12 52 39

Its title displays in all Select Menu fields
screen shot 2018-07-12 at 12 53 02

==> Great confusion as we have 2 Menu with the same title

After patch

It now does the same as for the Menu Type, i.e. it prevents saving.
screen shot 2018-07-12 at 12 51 39

Documentation Changes Required

Don't know if we have a doc for the Menu Type. If yes, may need changes to add this aspect.

avatar infograf768 infograf768 - open - 12 Jul 2018
avatar infograf768 infograf768 - change - 12 Jul 2018
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 12 Jul 2018
Category Administration Language & Strings Libraries
avatar infograf768 infograf768 - change - 12 Jul 2018
Title
[FIX} Forcing Menu to have a unique title
[FIX] Forcing Menu to have a unique title
avatar infograf768 infograf768 - edited - 12 Jul 2018
avatar Sandra97
Sandra97 - comment - 12 Jul 2018
avatar brianteeman
brianteeman - comment - 12 Jul 2018

i thought the entire purpose of the menutype was for it to be used as a unique reference

avatar infograf768
infograf768 - comment - 12 Jul 2018

We have one for the screen: https://docs.joomla.org/Help38:Menus_Menu_Manager_Edit

Looks like it is a bit wrong as it uses the term Unique name. I guess it then should be corrected indeed after this is merged as both Menu Type and Menu Title should be unique.

i thought the entire purpose of the menutype was for it to be used as a unique reference

It is internally unique indeed —look at the _menu table where it is used for the menu items—, but it is the Title which is displayed in the Select Menu field... Having the same Title there for 2 different menus is evidently an issue.

Please just test the PR. Thanks.

avatar infograf768
infograf768 - comment - 12 Jul 2018

Relaunched drone which was failing on js which is not touched by this PR. Let's cross fingers...

avatar alikon
alikon - comment - 12 Jul 2018

you can do this without code just add a new unique index on that field
something like https://github.com/joomla/joomla-cms/blob/staging/installation/sql/mysql/joomla.sql#L1419

avatar brianteeman
brianteeman - comment - 12 Jul 2018

So i am going to say that this is wrong. It is up to a user to ensure that they have unique titles IF it is an issue for them - we don't enforce unique titles anywhere else. The UI issue you noticed will be present anywhere that we have a list not just for menus

avatar Bakual
Bakual - comment - 12 Jul 2018

Imho the title is allowed to be the same, similar like we can have multiple articles with the same title.
That's up to the site administrator to decide how he wants that.
The menutype afaik is what has to be unique, and in fact it will be named "Unique Name" in J4.

avatar infograf768
infograf768 - comment - 12 Jul 2018

Imho the title is allowed to be the same, similar like we can have multiple articles with the same title.
That's up to the site administrator to decide how he wants that.

In the case of Menus, it is so simple to force unique titles to prevent possible issues for the site administrator that I do not see why we should not implement it.

avatar brianteeman
brianteeman - comment - 12 Jul 2018

you can force anything to be unique. doesnt mean it is desirable to do so AND if you insist on doing it here then you must do it everywhere for consistency or it will make no logic to the user why it is enforced here

avatar infograf768
infograf768 - comment - 12 Jul 2018

you can force anything to be unique. doesnt mean it is desirable to do so AND if you insist on doing it here then you must do it everywhere for consistency or it will make no logic to the user why it is enforced here

Did I say everything has to be unique? No. What has consistency to do with it?
Obviously if someone decides that the title of an article, category, etc. should have the same name, that is their problem (and may be good in multilingual sites).
In this specific case it just makes no sense at all.

In any case, I am pretty sure that the way it goes, this will not be merged. Therefore I leave it as is as a remembrance that we can make things simpler but some do not want to.
I will not respond to any more comments.

avatar brianteeman
brianteeman - comment - 12 Jul 2018

Closed for the reasons stated above

avatar brianteeman brianteeman - change - 12 Jul 2018
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2018-07-12 12:17:30
Closed_By brianteeman
Labels Added: ? ?
avatar brianteeman brianteeman - close - 12 Jul 2018
avatar infograf768 infograf768 - change - 12 Jul 2018
Status Closed New
Closed_Date 2018-07-12 12:17:30
Closed_By brianteeman
avatar infograf768 infograf768 - change - 12 Jul 2018
Status New Pending
avatar infograf768 infograf768 - reopen - 12 Jul 2018
avatar infograf768
infograf768 - comment - 12 Jul 2018

reopened as you are not the only one to be able to comment on this.
Please do not close any more. Let the release leader do it if he wishes.

avatar mbabker
mbabker - comment - 12 Jul 2018

We should only have unique constraints where they are absolutely required. This display title IMO doesn't require a unique constraint. So I don't think we need this personally.

avatar infograf768
infograf768 - comment - 12 Jul 2018

@mbabker
If you refuse to consider merging this and as you are the Release leader, you may indeed close this.

(To make it clear: does not mean I am happy with it, but I accept your decision as a release leader and not from anyone who has the power to close and has decided such unilaterally.)

avatar brianteeman
brianteeman - comment - 12 Jul 2018

Closed - again now that three maintainers have said no

avatar brianteeman brianteeman - change - 12 Jul 2018
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2018-07-12 12:34:35
Closed_By brianteeman
avatar brianteeman brianteeman - close - 12 Jul 2018
avatar infograf768
infograf768 - comment - 12 Jul 2018

Who now has a rattle and a pram? GL.

avatar mbabker
mbabker - comment - 12 Jul 2018

I accept your decision as a release leader and not from anyone who has the power to close and has decided such unilaterally

Just for reference I don't see the workflow in Joomla being that whomever is running releases is the only one who can make a decision on merge or reject. The entire reason we're supposed to have a maintenance team is to help with maintenance, which includes merging or rejecting pull requests. Obviously if something gets into a debated state where there's minimal or no consensus, that release lead should be able to make a "final" decision, but that's really the extent of it in my eyes. So personally, I don't have an issue with other people merging RTC PRs or declining PRs that are better off not being merged.

avatar infograf768
infograf768 - comment - 13 Jul 2018

...

Add a Comment

Login with GitHub to post a comment