Authorizator have no effect on whether the item is displayed or not

[patrickkusebauch]

I have just implemented authorizator, where the public function isMenuItemAllowed(IMenuItem $item): bool returns false in all cases, but it has no effect what so ever on whether the item is displayed or not. The only effect it has is on whether the item can be active or not. Did I misunderstand something or is it a bug?

[foxycode]

@patrickkusebauch This is mostly solved in menu template. Have you custom template? If yes, can you post it?

[patrickkusebauch]

{varType Carrooi\Menu\IMenuItemsContainer $menu}
<nav class="mt-2">
    <ul class="nav nav-pills nav-sidebar flex-column" data-widget="treeview" role="menu" data-accordion="false">
        {foreach $menu->getVisibleItemsOnMenu() as $itemsParent}
            {varType Carrooi\Menu\Menu|Carrooi\Menu\MenuItem $itemsParent}
            {continueIf $itemsParent->isAllowed() === false}
            <hr n:if="$itemsParent->getData('section', false)">
            {if $itemsParent->hasVisibleItemsOnMenu() === false}
                <li class="nav-item">
                    <a n:class="$itemsParent->isActive() ? 'nav-link active' : 'nav-link'"
                            href="{$itemsParent->getRealLink()}">
                        <i class="nav-icon {$itemsParent->getData('icon') ?? 'fas fa-th'}"></i>
                        <p>{$itemsParent->getRealTitle()}</p>
                    </a>
                </li>
            {else}
                <li n:class="$itemsParent->isActive() ? 'nav-item has-treeview menu-open' : 'nav-item has-treeview'">
                    <a n:class="$itemsParent->isActive() ? 'nav-link active' : 'nav-link'" href="#">
                        <i class="nav-icon {$itemsParent->getData('icon') ?? 'fas fa-th'}"></i>
                        <p>{$itemsParent->getRealTitle()}
                            <i class="right fas fa-angle-left"></i>
                        </p>
                    </a>
                    <ul class="nav nav-treeview">
                        {foreach $itemsParent->getVisibleItemsOnMenu() as $item}
                            {varType Carrooi\Menu\MenuItem $item}
                            <li class="nav-item">
                                <a n:class="$item->isActive() ? 'nav-link active' : 'nav-link'"
                                        href="{$item->getRealLink()}">
                                    <i class="nav-icon {$item->getData('icon') ?? 'fas fa-th'}"></i>
                                    <p>{$item->getRealTitle()}</p>
                                </a>
                            </li>
                        {/foreach}
                    </ul>
                </li>
            {/if}
        {/foreach}
    </ul>
</nav>

You can notice the continueIf macro I had to add, but I don't think should be neccessary.

[patrickkusebauch]

The menu.latte serving as a template for user implementation has no mention of needing to filter in the template. It does not even make sense from the domain logic perspective. You generally should have no knowledge about things you are not allowed to access.

[foxycode]

You are right, but I only took over this repository, didn't design it. I will need to look into it deeper. Would be nice if you could send PR with failing test.

[foxycode]

@patrickkusebauch I see test is already present. All tests are passing.

In template, you need to call isAllowed() and that will call isMenuItemAllowed() from you authorizator.

Didn't saying this is good implementation, but for now, it will function.

[darkWolf-PR]

Hi, how to connect the MenuAuthorizator to Nette Authorizator that I have already set-up? I can ask isAllowed or isInRole on User in Presenters, but how can I get that to menu-control?

Ok, so I found how to get user there to use isAllowed($resource) , but not sure how to hide top category if all sub-cats are not not allowed to user?

[foxycode]

@darkWolf-PR You can inject nette User or better IUserStorage to menu authorizator using contructor.

You can use this condition I believe:

if (count($menuItem->getVisibleItemsOnMenu())) {
    // show menu
}

[patrickkusebauch]

@darkWolf-PR the answer about hiding the top category has 2 parts.

The first is kind of about authorization - are you authorized to top category if you are not authorized to any subcategories? In the authorizator, you have the instance of IMenuItem that can get its children with getItems(). You can loop them and test whether you are authorized to at least one of them.

So maybe you find one sub-category you are allowed to access, so you are allowed to access to the top category. But here comes the second part - maybe you don/t display your subcategory on the menu, but only in the breadcrumb.

Therefor in the menu template, you have to do pretty much the same thing as in the Authorizator.
Loop over all visible children tests whether there is at least one allowed and if not, do not display the top category.

Pseudocode follows (I do not want to bother with latte styling)

displayTopCategory = false
foreach TopCategory->GetVisibleItemsOnMenu() as subCategory
    if subcategory->isAllowed(): displayTopCategory = true
/foreach
<li n:if="displayTopCategory && TopCategory->isAllowed()">TopCategory->getRealName()</li>

[patrickkusebauch]

@foxycode Yeah I know it functions like that. That is why I have {continueIf $itemsParent->isAllowed() === false} in the template I posted earlier. My problem is that it is not intuitive, it is not mentioned in the documentation and it is not used in the default template that serves at e guide to how to do your own template. So nowhere you will find any mention about this unexpected behaviour and it will bite you in the ass.

[darkWolf-PR]

I have to say I´m yet to understand whole this connection between menu and breadcrumb...I´ve only used this control for menu in admin, where all links are known and I only need to hide those not accessible for logged role. Also there is never a top category link with subcats, either it´s top category link, or it´s a blank header for opening subcats.

Breadcrumb have to show sublinks like /users/new, /users/edit, I would have to define those in menu config too to have the breadcrumb working?

[patrickkusebauch]

@darkWolf-PR well the issue in your case is that you have one specific use case. That use case (admin navigation) usually uses the tree structure to group some links together, that thematically go together. So you usually do not have a link for the top category. Because it is really not a top category. It is a group identifier with no meaning on its own.

But you can also imagine another use case like an e-shop catalog. In that case, having a link for the top category makes sense. For example:

• Your top category is Furniture
• Your subcategories might be Kitchen, Living room, etc.

In this use case, a link for the top category Furniture would lead you to a page with all the available furniture for all possible rooms.

As for the second question - yes, that would be the case. Again in your case, it is tedious and does not make much sense.

But if you come back to the e-shop example, the last level for the menu/breadcrumb would be the actual item detail page. You want to display the item detail page in the breadcrumb, but for sure you do not want to display them in the menu.

I think this library is trying to be very general and for many possible use cases, which is good, but the issue I see is that each use case has some caveats that would benefit from proper documentation. Documentation is generally a sore sport form Contributte components IMHO.

For example in the e-shop case, you probably do not really care about Authorization at all.

[foxycode]

@darkWolf-PR I'm adding complete example of what is working for me. Hope it will help you. It could definitelly work better and docs should be updated too. I will look into it when I have more time. Hope others will help too (@patrickkusebauch ?)

menu.neon

extensions:
    menu: Contributte\MenuControl\DI\MenuExtension

services:
    - Billing\Components\Menu\DefaultAuthorizator

menu:
    default:
        templates:
            menu: %appDir%/Components/Menu/menu.latte
            breadcrumb: %appDir%/Components/Menu/breadcrumbs.latte
        authorizator: @Billing\Components\Menu\DefaultAuthorizator
        items:
            overview:
                title: Přehled
                action: Overview:default
            invoicing:
                title: Fakturace
                link: '#'
                items:
                    actuarialList:
                        title: Doklady
                        action: Actuarial:ActuarialList:default
                        include:
                            - ^Actuarial\:ActuarialDetail\:.+$
                            - ^Actuarial\:Phone\:.+$
                    actuarialItems:
                        title: Položky dokladů
                        action: Actuarial:ActuarialItems:default
                    payList:
                        title: Platby
                        action: Actuarial:PayList:default
                        include: ^Actuarial\:PayList\:.+$

DefaultAuthorizator.php

final class DefaultAuthorizator implements IAuthorizator
{
    /**
     * @var User
     */
    private $user;

    public function __construct(User $user)
    {
        $this->user = $user;
    }

    public function isMenuItemAllowed(IMenuItem $item): bool
    {
        return $this->user->isLoggedIn();
    }
}

menu.latte

<ul class="nav navbar-nav" n:if="$menu->hasVisibleItemsOnMenu()">
    {foreach $menu->getVisibleItemsOnMenu() as $item}
        {if $item->hasVisibleItemsOnMenu()}
            <li n:if="$item->isAllowed()" n:class="dropdown, $item->isActive() ? active">
                <a href="#" class="dropdown-toggle" data-toggle="dropdown">{$item->getRealTitle()} <span class="caret"></span></a>
                <ul class="dropdown-menu">
                    <li n:foreach="$item->getVisibleItemsOnMenu() as $subitem" n:class="$subitem->isActive() ? active">
                        <a href="{$subitem->getRealLink()}">{$subitem->getRealTitle()}</a>
                    </li>
                </ul>
            </li>
        {else}
            <li n:if="$item->isAllowed()" n:class="$item->isActive() ? active">
                <a href="{$item->getRealLink()}">{$item->getRealTitle()}</a>
            </li>
        {/if}
    {/foreach}
</ul>

breadcrumbs.latte

<ol class="breadcrumb">
    <li n:foreach="$menu->getPath() as $item">
        <a href="{$item->getRealLink()}">{$item->getRealTitle()}</a>
    </li>
</ol>

BasePresenter.php

abstract class BasePresenter extends Presenter
{
    /**
     * @var IMenuComponentFactory
     */
    private $menuFactory;

    /**
     * @var MenuContainer
     */
    private $menuContainer;

    public function injectBasePresenter(
        IMenuComponentFactory $menuFactory,
        MenuContainer $menuContainer
    ): void {
        $this->menuFactory = $menuFactory;
        $this->menuContainer = $menuContainer;
    }

    protected function createComponentMenu(): MenuComponent
    {
        return $this->menuFactory->create('default');
    }

    protected function getMenu(): IMenu
    {
        return $this->menuContainer->getMenu('default');
    }
}

ActuarialDetailPresenter.php

final class ActuarialDetailPresenter extends BasePresenter
{
    /**
     * @var Actuarial
     */
    private $actuarial;

    /**
     * @throws EntityNotFoundException
     */
    public function actionDefault(int $id): void
    {
        $this->actuarial = $this->actuarialFacade->get($id);
    }

    public function actionNew(): void
    {
        $this->setView('default');
    }

    public function renderDefault(int $id = NULL): void
    {
        $this->template->actuarial = $this->actuarial;

        $this->getMenu()
            ->getItem('invoicing')
            ->getItem('actuarialList')
            ->addItem('detail', "Doklad $id", function (IMenuItem $item) use ($id): void {
                $item->setAction('ActuarialDetail:', ['id' => $id]);
                $item->setMenuVisibility(FALSE);
            });
    }
}