User tests: Successful: Unsuccessful:
This should solve #8876
See there for tests instructions.
Basically, this PR first checks for multiple Home pages in the same menu before other checks.
Please
@andrepereiradasilva @dgt41 @fontanil
Please test.
Status | New | ⇒ | Pending |
Labels |
Added:
?
|
Category | ⇒ | Administration Components Multilanguage |
The message can also be improved "A menu should contain only one Default home."
Something like this?:
"You can't set this menu item as default home page for xxxxx language(s) because there is already a menu item assigned as default home page for xxxxxxx language(s) in this menu.
A menu can only have a menu item assigned as default homepage.
If you want to change the default home page for this menu to xxxxx language(s), unset the current default home page in this menu first."
@infograf768 why "No menu item set to Home"?
There is a menu item set to home (in other language) in that menu... only not the one you tried to set.
The Message is the result of the action, not a statement of the presence or not of a Home in the Menu.
It is independent from this patch as we get exactly the same message before the patch when trying to set a Home for another Content Language than the one which has already a Home in that Menu.
The PR just corrects the issue when trying to set a Home when the menu item is set to ALL languages and the existing Home is a Content Language.
Also, the explanation of the failure is in the Error/Warning.
The strings are and have been there since 1.6.
COM_MENUS_ITEMS_SET_HOME_0="No menu item set to home."
COM_MENUS_ITEMS_SET_HOME_1="1 menu item successfully set to home."
COM_MENUS_ITEMS_SET_HOME_MORE="%d menu items successfully set to home."
COM_MENUS_ITEMS_UNSET_HOME="1 menu item successfully unset to home."
from this code:
https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_menus/controllers/items.php#L141-L157
It is similar to other plural messages in J!.
For example:
COM_USERS_N_USERS_ACTIVATED_0="No user activated."
COM_CONTENT_N_ITEMS_CHECKED_IN_0="No article successfully checked in."
Although there are users activated or articles already checked in. Etc.
If you have another better idea for COM_MENUS_ITEMS_SET_HOME_0
, please propose.
@brianteeman What do you think?
In any case, as the PR is OK, we can separate the discussion about the value of the string from this.
Please set the Test as successful.
I have tested this item successfully on 3cc2166
Confirmed the behaviour, patch works as expected.
What I don't like is that the check is in the store function, might be better in the check function. As developer I would expect when I call the check function that all logical problems are checked and an error in the store function is more something really serious like db connection lost. Might be nitpicking.
I have tested this item successfully on 3cc2166
@infograf768 true your right. This PR is ok. It corrects the problem. Sorry for hijacking it with the message thing.
Just to conclude the messages discussion. I know it's there from a long time, my point was: from usability point, a user receiving one RED and one GREEN message at the same time is like "what?????". The result of the action is ok (GREEN) or not (RED)?
Status | Pending | ⇒ | Ready to Commit |
RTC for 3.5.0. Thanks for testing!
Labels |
Added:
?
|
I have tested this item successfully on 3cc2166
Tested on today's staging: works perfectly. Thanks JM!
@andrepereiradasilva
Concerning the colour of the Message. What could be done —in a separate PR— is this:
if (count($cid) == 0)
{
$this->setMessage(JText::plural($ntext, count($cid)), 'warning');
}
else
{
$this->setMessage(JText::plural($ntext, count($cid)));
}
We would get:
I guess it could be part of a more general PR where all similar instances would be modified.
Or
if (count($cid) == 0)
{
$this->setMessage($model->getError());
}
else
{
$this->setMessage(JText::plural($ntext, count($cid)));
}
In order to only get the error.
+1 for second option! One error message is enough.
Will try to prepare another PR covering all cases in core. Not urgent.
ok thanks @infograf768
Status | Ready to Commit | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-01-16 08:12:33 |
Closed_By | ⇒ | roland-d |
Labels |
Removed:
?
|
Can create a 'warning (simple in html like pop-up mice menu to notice in back admin panel)' when attempting in a multi-languages site to setting 'default' in a new Home page in a new menu 'before save (no close) it' with a specific language, avoiding below error(s) (did not studied code, yet)?
@infograf768 it does not unset the default home now, but the message "No menu item set to home." is still there and it shouldn't.