I think it would be more efficient to change the bookmarks property to return a dict keyed on the relationship type (star, membership, subscription) with the hub lists as values. Otherwise the filtering is making additional calls to the other properties which end up in SQL queries.

The userEntries name does not seem very appropriate anymore.

Please add the key="All groups" value so React can identify the item in the list (there's an error message in the JS console if you don't).

It's not actually required (see the "All groups" entry), please remove the .isRequired.

Are you sure you want to use the "home" icon here?

Thanks for the review, @abompard !

• removed the bookmarks_starred, and other properties, and made the bookmarks property return a dict keyed to the type. Doing this broke the allgroups page (which i am planning to hack on next), so i just updated the menu there so it works the same way -- futher formatting on that page will come in a future PR
• renamed userEntries to leftMenuEntries
• added the key="All groups"
• removed the is_required on the hubs_type bool
• changed the home icon for all users back to the 'users' one, we are using this for the groups down below, but this is easily changed in the future.

Hmm this is still calling self.starred_hubs multiple times. Actually I think it would be better to run through the self.associations list and distribute items in the resulting dict's lists. This way you'd iterate over them only once. Something like:

        bookmarks = {
            "starred": [],
            "memberships": [],
            "subscriptions": [],
        }
        for assoc in self.associations:
            if assoc.hub.name == self.username:
                continue
            if assoc.role == "stargazer":
                bookmarks["starred"].append(assoc.hub)
            elif assoc.role == "member" or assoc.role == "owner":
                bookmarks["memberships"].append(assoc.hub)
            elif assoc.role == "subscriber":
                bookmarks["subscriptions"].append(assoc.hub)
        bookmarks = dict(
            (key, sorted(list(set(values)), key=operator.attrgetter('name')))
            for key, values in bookmarks.items()
        )
        return bookmarks

(I haven't tested it)

Also, please add unit tests for that, to make sure starred hubs, member hubs and subscribed hubs don't appear multiple times for example.

That's a loooot of spaces. Are those tabulations?

This should be designteam

This should be designteam too.

Gah, sorry -- really shouldnt update commits at midnight :)

I got the tests running, then ran lint and one of those lines with designteam in it was too long, so i changed it to design, but forgot to change it up further. Just changed it all to commops now, sorry designteam ;)

also fixed the weird indenting.

You don't have to set the defaultProps here because the text property was defined as isRequired above. So there's no need for a default value, it will have to be explicitely given.