User tests: Successful: Unsuccessful:
Pull Request for New issue.
The goal of this PR is to solve a long standing bug of the Menu item alias.
When you create a menu item of type alias, the alias field is auto-generated with the system date and not with the menu title.
This is very annoying since we have to manually edit it after or we get the menu item children URI with that date.
I'm not yet very comfortable with the JTable, so if a maintainer can review this would be appreciated (is just one line of code that changes).
Status | New | ⇒ | Pending |
Labels |
Added:
?
|
hum. i never used the alias like that. I use it mostly to create a parent menu that redirects to it's first child. But yes, i can see that usage too. I will check if we can have both worlds.
ok how about now?
Much better. I will hopefully be able to test tomorrow. Thanks!
We have a warning there and there must have been a reason for it:
Leave the alias field empty if the menu item alias and the menu item linked to by the alias have the same parent.
@infograf768 That's exactly because of the use case I mentioned, however with the new commit it should be taken care of (I still didn't test)
Category | ⇒ | Administration Router / SEF |
If the reason for having that message displayed is no longer needed then we shouldnt display the message any more
If the reason for having that message displayed is no longer needed then we shouldnt display the message any more
Sure i can rmeove it in this PR. but was waiting for @chivitli and @infograf768 tests and comments.
@andrepereiradasilva
When using multilanguage, we can have 2 menu items with the same alias as long as their language tag is different.
As far as I see, this PR does not take this into account.
If we are worried about excessive duplicate warnings, why not simply have the alias increment just like articles? Currently when you create a duplicate article with the name of "Awesome Joomla", the duplicate article alias would be "awesome-joomla-1" which is much better than a date.
When using multilanguage, we can have 2 menu items with the same alias as long as their language tag is different.
As far as I see, this PR does not take this into account.
Yes it does. See https://github.com/andrepereiradasilva/joomla-cms/blob/patch-3/libraries/legacy/table/menu.php#L120
$itemSearch = array('alias' => $testAlias, 'parent_id' => $this->parent_id, 'client_id' => (int) $this->client_id, 'language' => $this->language);
Pleae test
If we are worried about excessive duplicate warnings, why not simply have the alias increment just like articles? Currently when you create a duplicate article with the name of "Awesome Joomla", the duplicate article alias would be "awesome-joomla-1" which is much better than a date.
The date alias fallback to all menu items types is not changed in this PR.
PR are welcomed to change it.here i think
I have tested this item successfully on 3314231
I have tested this item
1) Before patch alias created as a date
2) After patch alias created using menu title
Status | Pending | ⇒ | Ready to Commit |
Labels |
Added:
?
|
Milestone |
Added: |
@andrepereiradasilva Should the warning be removed or not as suggested in #9980 (comment) ?
Status | Ready to Commit | ⇒ | Information Required |
Labels |
Labels |
Removed:
?
|
We have a warning there and there must have been a reason for it:
Leave the alias field empty if the menu item alias and the menu item linked to by the alias have the same parent.
Tested the use case the warning message describes.
What i did (always let it auto generate the alias):
And saw no problem.
So i see no problem in removing this message after this PR, since it doesn't seem to apply anymore
@infograf768 Can i ask for your help in also confirming that?
@test
created 1 parent menu item "The parent" and 1 child menu item "The child 1"
created a new child menu item, menu alias type "The child 2", and linked it to "The child 1"
This works fine.
But, if I use the same Title for the menu alias type as the target menu item, I get the date again instead of the usual warning:
Save failed with the following error: Another menu item with the same parent has this alias (remember it may be a trashed item).
Another remark:
On a monolanguage site, if the menu alias type and the target have different languages (for example: All and en-GB) then we get the same alias. (no issue though in this specific case).
But I would check if it could be an issue, i.e. look for $language ONLY when multilang is enabled.
ok for your comments the warning can be removed. ^Will remove it.
Reggarding
But, if I use the same Title for the menu alias type as the target menu item, I get the date again instead of the usual warning:
I didn't change that.
Try without this patch and you will see this is waht is happening right now (without the patch).
This PR has received new commits.
ok it seems there is bug in this PR.
According to @infograf768 in a monolanguage site we can't have 2 menu items that are set to different languages (all and en-gb for example) with the same alias.
This PR allows to have the same alias in that case, so needs to be reviewed.
This PR has received new commits.
@infograf768 added several restrictions, please test and also check the code.
@test negative here.
Monolingual.
May have some Content Languages created, we have at least en-GB by default.
Expected result:
We never should get 2 menu items with same alias when same Parent (including ROOT), whatever the assigned language, including ALL.
(Exception: A Menu Item Alias can have the same alias as its target menu item)
After patching, here 3 items with different languages get the same alias:
Multilingual:
To test,
A menu item can have the same alias (when same Parent including ROOT) when their language is different, EXCEPT for language ALL.
The reason is that loading a menu item tagged to language ALL can give the same URL as an item tagged to a specific language.
After patch:
It accepts the same alias when item tagged to ALL
I guess the store() method has to be patched too.
Yep, looks like it is a different issue (unrelaed to Menu Item Alias) which has to be solved in the store method.
Looking into it now.
@andrepereiradasilva @infograf768 where are we at with this.
@brianteeman will close this. i will make a new PR (today or tomorrow)
Status | Information Required | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-06-07 09:59:50 |
Closed_By | ⇒ | andrepereiradasilva |
I am not sure this is a bug, but rather intended. I think if this is to be implemented, people will suddenly start receiving a bunch of 'duplicate alias' warnings when they try to create a menu item of this type. In my experience at least, any time I resorted to creating a menu alias, I used the same title as of the menu to which it was aliased - with this pr accepted now I'd get a warning if they are both on the same level (which is usually the case for me).