[#13932] - Check for component container item in admin menu recovery mode
- Closed
- 25 Feb 2017
- Medium
- Build: staging
- # 13932
- Diff
- SniperSister:37-patch-componentcontainer-recmode
User tests: Successful: Unsuccessful:
Summary of Changes
First, this PR reverts #13838 because it's inconsistent from a user perspective: the new admin menu has a built-in recovery mode that checks the presence of important menu items and offers a link that temporarily restores the old menu structure. With #13838 an additional check for the component container item was added - but not to the recovery mode, but to the general code of the module causing the item to show up without any explanation.
This PR moves the check for the container to the general recovery-mode logic and adjusts the messages accordingly, giving a consistent workflow.
Testing Instructions
- Create a custom admin menu
- Set it as your active menu in menu module settings.
- A message stating that the recommended menu items for Modules, Menus and Components are missing should be shown including a link to enable the recovery mode.
- Create a new menu item of type "System Links > Components Menu Container" in your custom menu.
- The message regarding the missing component item should be gone.
Documentation Changes Required
None
| Status | New | ⇒ | Pending |
| Category | ⇒ | Administration Language & Strings Modules |
| Labels |
Added:
?
?
|
||
Two things that I don't like here:
- The repeating "The active administrator menu does not contain a..." in all messages when we have more than one of these important items missing.
- We are not checking any authorisation for missing "Components" menu item. Chances are their that the user has no access to any of those components and the default menu item too will hide the components menu item anyway.
Rest is fine by me. No objection to this approach.
@izharaazmi: I don't get the issue you are mentioning in 2. I haven't touched the checks logic at all, just moved it to a different place?
The menu permission is ok. What I am talking about can be reproduced as below:
- Create a new administrator user.
- Revoke all permission to all components under "components" menu for this new user.
- Login as this user after setting a custom menu for the backend, which does not have components folder.
- He still sees the warning, which he should not as it doesn't matter for him anyway.
Am I clear?
Ok, now I get what you mean - but If I read the code correctly, this won't happen anyway? The check for the container only checks if a menuitem with the type "container" exists - if so, now message is shown. Once the message processing is done, the loadItems() method is called where an empty container is hidden anyway, so this happens AFTER the check and doesn't interfere with it?
@izharaazmi
I think it is a good idea to force component container in the same way we do for modules and menu items.
Can you folks complete this patch in collaboration?
@SniperSister is this PR to test or are you working in collaboratin like @infograf768 suggested?
I think we are waiting for @izharaazmi to provide feedback on my question
Alright. I'll do it. :)
Thanks for Info @SniperSister @izharaazmi
@izharaazmi
Better be fast as next release is a RC and we may freeze languages. :)
@infograf768 I am going to show the warning against "container" only to
those users who has access to menu manager. Coz, IMO others can't do
anything about it anyway. Is that okay?
i guess any custom admin menu item, including these in component container should anyway display only when the user has permissions for the components concerned.
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-02-25 09:56:05 |
| Closed_By | ⇒ | infograf768 |
Set to "closed" on behalf of @infograf768 by The JTracker Application at issues.joomla.org/joomla-cms/13932
Closing in favour of #14235
Thanks!
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13932.
@izharaazmi
what do you think?