Make the user menu extensible

Development
#1

Hi all,

Currently adding an entry to the menu on the left-hand side of the user profile requires the modification of the code of the platform (cf XWikiUserSheet.xml).

This is not really nice because it requires to be intrusive of the platform code in order to add new user menu entries.

Following this discussion https://markmail.org/message/ojgmy7pljzwjq7vp, we consider two solutions to migrate to a modular user menu mechanism: UIX or XClass.

While this discussion is broad than the extensibility of our user menu, I will discuss the pro and cons of both choices in the context of the extension of the user menu.

Use case

First, it is important to understand some details of the behavior of the menu.
The menu is composed of multiple entries that are composed of a title, an icon, and a Sheet.

Entries can be displayed according to some criterion, for instance:

  • is the profile being accessed the one of the user currently logged in
  • is the current user an admin

The visibility of an entry is based on the evaluation of a combination of criteria using boolean conditions.

UIX

Pro

The UIX mechanism is dedicated to such user interface extension.

Cons

The parameters are untyped, but this should not be an issue for our use case, which is quite similar to the one presented in the UIX tutorial.

XClass

Pro

The attributes are typed, which make it safer.

Cons

Following the example of ConfigurableClass, a non-trivial macro needs to be defined to select the documents that contain objects of type ConfigurableClass.

Conclusion

While using the UIX mechanism is easier and more suited to the task, the need to filter the displayed entries based on intricate and possibly unanticipated criteria makes it easier to realize with XClass.

So unless we find a way to cleanly and safely define the display criterion using UIX, I propose to use XClass to make the user menu extensible.

WDYT?

PS: I will be writing a PoC using XClass. I will share it on this discussion as soon as it is done.

#2

+1

#3

IMO this is not the right criteria for choosing UIXP vs XClass. You can achieve the exact same logic with both and UIXP are also extensible since they provide parameters too.

I’d prefer that the discussion based about defining when to use UIXP and when to use XClass as I’ve discussed with Simon over video conf this morning.

For example:

  • When the need is to provide extensibility in the skin/UI of XWiki: use UIXP
  • When the need is to provide extensibility in the page content: use XClass
  • Some of our content in wiki pages are actually skin /UI of XWiki and the following features could well be moved to the navigation/UI areas and not as page content:
    • Panel (such as Applications Panel)
    • Admin UI vertical menu
    • Profile page vertical menu

Thus for these 3 examples, I think it makes sense to consider them as UI/skin and use UIXP. This is already the case for the Applications Panel. For the Admin UI, I think the ConfigurableClass feature could be moved to a UIXP over time.

WDYT?

PS: I haven’t read again the discussion we had back then in https://markmail.org/message/ojgmy7pljzwjq7vp and I should but just wanted to reply quickly for now.

#4

I understand the distinction you want to make here and quite frankly I was more in favor of using an UIXP for the Profile Menu at first because of that distinction: i.e. for me the menu is part of the UI and should be extended with UIXP.

Now, we discussed with Manuel about the problem of deciding when to display some elements of the menu, and I don’t think UIXP provides a good way for computing that decision.
The parameters can express some values and you can express their value with velocity, but IMO in our case they would be expressed with some nasty velocity expression and I’m a bit afraid that it really wouldn’t ease maintaining it in the future.

So it looks easier for that reason to introduce a XClass with a specific parameter dedicated to write a piece of script which would be evaluated to ensure the condition is true to be displayed in the menu.

Now maybe that idea too is wrong and there’s something better to do, but for now it looks like the xclass is a better match for that specific problem.

#5

Out of curiosity, I tried to define an extension point, more as a though experiment than anything else.

So on one side I defined a UIX with the following parameters:

display=#if(($services.model.resolveDocument($xcontext.user) == $doc.documentReference) || $hasAdmin)true#{else}false#end
id=test
sheet=Dashboard.XWikiUserDashboardSheet

And in the other side, I introduced an extension point in XWiki.XWikiUserSheet:

#foreach ($uix in $services.uix.getExtensions('test.usermenu.uix'))
#if($uix.parameters.get('display') == 'true')
#set($discard = $categories.add({'id': $uix.parameters.get('id'), 'sheet': $uix.parameters.get('sheet'), 'glyphicon': 'th'}))
#end
#end

The end result is a new entry displayed only if the user is consulting it own profile, or if the user is an admin.

Pro

It is quite straightforward and based on the current mechanism dedicated to user interfaces extensibility.

Cons

It feels hacky, and the value of the param key need to be inlined very carefully in order to avoid the introduction of undesired spaces around true/false.
It probably does not scale very well to more complex display conditions, but afaik it more an hypothetical limitation than a real one in regard to the existing implementation.

#6

Just replying to this part (haven’t read yet the answer from Simon :)).

Note that there are ways to make it less hacky. For example by creating a velocity define and use it in the display parameter, as in:

#define ($display)
  #if (($services.model.resolveDocument($xcontext.user) == $doc.documentReference) || $hasAdmin)
    true
  #else
    false
  #end
#end
display=$display
#7

Sadly the currently implementation of is not parsing the content as a whole, par reads each line individually, splits on the first = and interprets the right-hand side of the line as a velocity expression.
The line without an = char are ignored (cf WikiUIExtensionParameters).

May the $display definition be defined somewhere else?

#8

There is a huge miss in the pro and cons listed here: UIX are components. Among other things it means a JAR extension can provide an extension point in the user profile.

It’s possible UIX is the perfect fit (we could introduce a new component if you feel it’s too generic which is a valid point) but frankly introducing this kind of extension point without using components is a -1 for me right now.

#9

@tmortagne I’m not sure I follow you. You can also create XObject in java.If you’re saying that we shouldn’t use the XClass/XObject concept anymore because we have UIXP/UIX then I’m not sure I agree with you. Both are serving a very similar need and we need to either:

  • Deprecate one and move to the other
  • Clearly define the scope of application of one vs another.

Also, what you mention as a PRO (store the UIX as Java code), I see it as a possible CON: the user won’t be able to easily modify the UIX if he wants to customize it/disable it/etc. Whereas a JAR that creates an XObject in Java (and thus it’s materialized in a wiki page) allows for modifications/tuning.

EDIT: I’m playing the devil’s advocate here because I also think that we should use UIXP for the user profile since I consider the profile menu to be a Skin feature (it just happens to be currently located in the page but that’s an implementation detail).

#10

Let’s start:

  • XClass/XObject:
    • Typed
    • Can be queried using HQL/XWQL
    • Currently supposed to be used to hold business data
  • UIXP/UIX
    • A generic and simplified XClass/XObject to be used for UI customizations
    • Untyped but String parameters are supported
    • Can be stored in Java Components
    • Cannot really be queried (at least no complex query since they are not typed: all the parameters are stored in a single field)
    • Currently supposed to be used for Skins, i.e. to add some customizations to the skin without having the write a new skin or have to overwrite a full vm file.

WDYT? Anything more? Is this correct?

#11

BTW about this, what I did in the past for “is the current user an admin”, is simply to set a permission on the UIX page so that only admins can view it. We had this need for example for Applications Panel entries.

EDIT: Now I’m not saying this is the best because it does prevent a user from viewing the page actually which is potentially less good :slight_smile:

#12

+1 for using UIX in this case. Note that besides the fact that they are implemented as components, you can also set a visibility scope (global, wiki, user).

Using a sheet could be considered an implementation detail. Maybe I want to use a template. For me the need is generic: have a way to specify the content to inject. UIXs have a dedicated field for specifying the content to inject: the content field. Using a parameter whose value is a page name is useful if you want to check access rights on the specified page, but otherwise the content field is more powerful. Moreover, the decision to display or not the UIX can be written in the content field (i.e. inject the content only if some condition is met). Now I know that you probably don’t want to render all the profile sections in order to find out which menu entries to show, but there might be ways to overcome this (maybe using 2 UIX).

Thanks,
Marius

#13

I think we are converging toward an agreement that UIX is the relevant mechanism for the extension of the user profile menu, and in general for cases where elements of the UI can be extended by external elements.

Action plan:

  1. Introducing a UIXP in the user menu
  2. Migrating the existing user menu entries to UIX
  3. Using the UIXP in the AP extension.

Two additional points to consider.

Entries ordering
Do we want to introduce a mechanism to enforce some order for the menu entries?
I think the sortByCustomOrder optional parameter can be used to enforce a specific order for extensions provided by default. Other extensions are not specifically ordered.

Entries visibility
UIX can be injected into a UIXP according to the context (e.g, user right, parameters).
Each UIX can express its own criteria (e.g, each menu entry has its own displays conditions).
Does it exist a generic mechanism to realize that? If not, it could be interesting to provide a generic mechanism to realize that. In this way, we avoid the typing issue as this specific feature is always expected to return a boolean value.

#14

At the very least we want the user profile entries to appear in a stable order and not completely random depending on the order in which the JARs were loaded.

You mean having something like a UIExtension#isEnabledd() and make the resolution of this value dynamic in the wiki based UI extensions ? I guess this is doable trough parameters which are already dynamic (for example you could have a enabled property and have each UI extension use Velocity if they need to dynamically decide if they should be listed or not and the default would be true if that property is not set) but it’s indeed something which would make a lot of sense as a dedicated field instead of a custom property.

#15

I made a full PoC in this direction.

The user menu is fully defined by loading the UIX through the org.xwiki.plaftorm.user.profile.menu.content UIXP (see XWikiUserSheet.xml l51). The entries order is made deterministic by using the sortByCustomOrder parameter.

The UIExtension#isEnabledd() is emulated by defining a display parameter in the UIX (e.g., here).

WDYT?