akeeba / fof

Rapid Application Development framework for Joomla!™ 3 and 4
0 stars 0 forks source link

Display menuitems in sidebar based on "group title" or "user id" #621

Closed marcodings closed 8 years ago

marcodings commented 8 years ago

2nd time is a charm.. Display menuitems in sidebar based on "group title" or "user id". Extend the definition of meta metadata.xml, if no viewaccess is defined, item is shown. If viewmetadata is specified, current users settings are tested against it and only displayed if the user matches

<?xml version="1.0" encoding="utf-8"
<viewmetadata>
    <foflib>
        <ordering>99</ordering>
        <viewaccess>
            <!-- can have muliple entries for userid or groupname -->
            <userid>42</userid>
            <groupname>ViryaDevelopment</groupname>
        </viewaccess>
    </foflib>
</viewmetadata>
nikosdion commented 8 years ago

OK, this is promising but I need to shape it a bit better. I don't like uncached static methods because they ultimately make testing a pain in the rear. Next week I'm on vacation, but I'll try to get it merged before mid-August :)

marcodings commented 8 years ago

enjoy your vacation..

nikosdion commented 8 years ago

Hm, the more I'm looking at it the more issues with it I find.

Hard-coding user IDs and group names (or even view access levels) is very site-specific and has no place in the metadata.xml file. It makes more sense for such limits to be part of your component.

You can modify the sidebar menu on the fly, e.g. as we're doing in Akeeba Subs (if you ignore the tree structure we're basically telling FOF which views to render in the submenu). In your component's Toolbar you can simply override renderSubmenu, call getMyViews and then filter out the views you don't want. Or do a custom presentation of select views like we do in Akeeba Subs. Or even limit views by user ID, group, view access level etc. This is why Toolbar is overridable.

Furthermore, a few comments regarding coding issues and coding style I spotted:

        if (!is_string($groupNames) && !is_array($groupNames))
        {
            throw new InvalidArgumentException('$groupNames should be passed as array or string');
        }

        $groupNamesArr = (array) $groupNames;

        if (is_string($groupNames))
        {
            // Convert a lone string into an array
            $groupNamesArr = empty($groupNames) ? array() : array($groupNames);
        }

Overall I am not going to merge this PR since it's a feature that doesn't belong to the framework.