? Success

User tests: Successful: Unsuccessful:

avatar andrepereiradasilva
andrepereiradasilva
18 Apr 2016

Pull Request for New issue.

Summary of Changes

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.

image

Testing Instructions

  1. Use latest staging
  2. Create a menu item of type "Menu Item Alias" and let it generate the alias field. You'll notice it generate a alias with the current date, instead of an alias from the title (as the other menu items)
  3. Apply patch
  4. Repeat step "2." You'll notice it will generate a alias from the title.

Observations

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).

avatar andrepereiradasilva andrepereiradasilva - open - 18 Apr 2016
avatar andrepereiradasilva andrepereiradasilva - change - 18 Apr 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 18 Apr 2016
Labels Added: ?
avatar chivitli
chivitli - comment - 18 Apr 2016

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).

avatar andrepereiradasilva
andrepereiradasilva - comment - 18 Apr 2016

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.

avatar andrepereiradasilva
andrepereiradasilva - comment - 18 Apr 2016

ok how about now?

682bc4c 18 Apr 2016 avatar andrepereiradasilva cs
avatar chivitli
chivitli - comment - 18 Apr 2016

Much better. I will hopefully be able to test tomorrow. Thanks!

3314231 18 Apr 2016 avatar andrepereiradasilva cs
avatar infograf768
infograf768 - comment - 19 Apr 2016

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.

avatar chivitli
chivitli - comment - 19 Apr 2016

@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)

avatar brianteeman brianteeman - change - 19 Apr 2016
Category Administration Router / SEF
avatar brianteeman
brianteeman - comment - 22 Apr 2016

If the reason for having that message displayed is no longer needed then we shouldnt display the message any more


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9980.

avatar andrepereiradasilva
andrepereiradasilva - comment - 22 Apr 2016

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.

avatar infograf768
infograf768 - comment - 23 Apr 2016

@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.

avatar JoshuaLewis
JoshuaLewis - comment - 23 Apr 2016

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.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9980.

avatar andrepereiradasilva
andrepereiradasilva - comment - 23 Apr 2016

@infograf768

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

avatar andrepereiradasilva
andrepereiradasilva - comment - 23 Apr 2016

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

avatar grhcj grhcj - test_item - 25 Apr 2016 - Tested successfully
avatar grhcj
grhcj - comment - 25 Apr 2016

I have tested this item :white_check_mark: successfully on 3314231


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9980.

avatar freshweb freshweb - test_item - 21 May 2016 - Tested successfully
avatar freshweb
freshweb - comment - 21 May 2016

I have tested this item successfully on 3314231

1) Before patch alias created as a date

2) After patch alias created using menu title


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9980.

avatar freshweb
freshweb - comment - 21 May 2016

screen shot 2016-05-21 at 09 53 51


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9980.

avatar brianteeman brianteeman - change - 21 May 2016
Status Pending Ready to Commit
avatar brianteeman
brianteeman - comment - 21 May 2016

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9980.

avatar joomla-cms-bot joomla-cms-bot - change - 21 May 2016
Labels Added: ?
avatar brianteeman brianteeman - change - 21 May 2016
Milestone Added:
avatar roland-d
roland-d - comment - 21 May 2016

@andrepereiradasilva Should the warning be removed or not as suggested in #9980 (comment) ?

avatar roland-d roland-d - change - 22 May 2016
Status Ready to Commit Information Required
Labels
avatar andrepereiradasilva
andrepereiradasilva - comment - 25 May 2016

@roland-d just saw your commnet now, sorry

will check

avatar joomla-cms-bot joomla-cms-bot - change - 25 May 2016
Labels Removed: ?
avatar andrepereiradasilva
andrepereiradasilva - comment - 25 May 2016

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):

  • apply this patch
  • 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"

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?

avatar infograf768
infograf768 - comment - 27 May 2016

@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).

avatar infograf768
infograf768 - comment - 27 May 2016

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.

avatar andrepereiradasilva
andrepereiradasilva - comment - 27 May 2016

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).

avatar joomla-cms-bot
joomla-cms-bot - comment - 27 May 2016

This PR has received new commits.

CC: @freshweb, @grhcj


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9980.

avatar andrepereiradasilva
andrepereiradasilva - comment - 27 May 2016

ok, the warning has removed . see ce20802

avatar andrepereiradasilva
andrepereiradasilva - comment - 27 May 2016

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.

avatar joomla-cms-bot
joomla-cms-bot - comment - 27 May 2016

This PR has received new commits.

CC: @freshweb, @grhcj


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9980.

avatar andrepereiradasilva
andrepereiradasilva - comment - 27 May 2016

@infograf768 added several restrictions, please test and also check the code.

avatar infograf768
infograf768 - comment - 28 May 2016

@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:

screen shot 2016-05-28 at 08 19 07

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

screen shot 2016-05-28 at 08 35 57

avatar infograf768
infograf768 - comment - 28 May 2016

I guess the store() method has to be patched too.

avatar infograf768
infograf768 - comment - 28 May 2016

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.

avatar brianteeman
brianteeman - comment - 7 Jun 2016

@andrepereiradasilva @infograf768 where are we at with this.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9980.

avatar andrepereiradasilva
andrepereiradasilva - comment - 7 Jun 2016

@brianteeman will close this. i will make a new PR (today or tomorrow)

avatar andrepereiradasilva andrepereiradasilva - change - 7 Jun 2016
Status Information Required Closed
Closed_Date 0000-00-00 00:00:00 2016-06-07 09:59:50
Closed_By andrepereiradasilva
avatar andrepereiradasilva andrepereiradasilva - close - 7 Jun 2016

Add a Comment

Login with GitHub to post a comment