luci-mod-notification: initial checkin

[feckert]

This pullrequest adds the LuCI module to display/show the notification from the system. The required backend for the Ubus is covered in this pull request. @hnyman before this is accepted, and could be merge you/we have to update/add this module to weblate.

@aparcar @karlp what do you think of this?

[aparcar]

I'd like to have this as a service showing on whatever tab is open rather in some sub menu. Specifically after login a list of things to take care of would be nice.

[feckert]

I'd like to have this as a service showing on whatever tab is open rather in some sub menu. Specifically after login a list of things to take care of would be nice.

@aparcar Unfortunately, I'm not quite sure what you mean by service?

That's how it looks on my system.

[aparcar]

I see, great, i didn't fully compile this in my brain. Wrongly i assumed it'd be its own page in the menu.

The implementation right now looks good!

What do you think about adding the notifications to the side rather than blocking the full view? Just like the "please set a root password" notification which is present but still allows to do other things.

[feckert]

@aparcar I am not sure if this is a good thing, as there is no limit to the notification count so far. This would take up a lot of space if there are a lot of notifications.

I would now try to add the password checks that pushes a notification. @jow where can I hook in, to create a notification that there is now password set on the system

So far we have the check with every call and the check is duplicated on every theme. https://github.com/openwrt/luci/blob/master/themes/luci-theme-openwrt/luasrc/view/themes/openwrt.org/header.htm#L77 https://github.com/openwrt/luci/blob/master/themes/luci-theme-bootstrap/luasrc/view/themes/bootstrap/header.htm#L52 https://github.com/openwrt/luci/blob/master/themes/luci-theme-material/luasrc/view/themes/material/header.htm#L210 https://github.com/openwrt/luci/blob/master/themes/luci-theme-openwrt-2020/luasrc/view/themes/openwrt2020/header.htm#L57

[aparcar]

This would take up a lot of space if there are a lot of notifications.

For that I'd like to put those notifications on the side, take OctoPrint for example

[feckert]

This would take up a lot of space if there are a lot of notifications.

For that I'd like to put those notifications on the side, take OctoPrint for example

@aparcar If we realize it this way, then it is no longer generic. In principle, I think the approach is good, but we must somehow manage that we attach additional information to the notification.

I think to stay generic, we have to add an unique message ID, so that the luci-mod-notification knows which message it is. Additional we could add a data json section to the notification where additional information are include that the luci-mod-notification needs to show the message. For example for an update notification the installed version and new firmware version.

Also, we should translate the notification to i18n. But this can only happen in the luci-mod-notification. The generic notification in the system does not know if a LuCI is installed.

If no message id is set then the message is shown as it is.

[aparcar]

@feckert I'm unsure if the message IDs are a clean implementation. It introduces "corner cases" to a single file rather than keeping it modular. Image you have a Adblock Update notification and the developer wants to add a "Update now" button. Adblock would be an optional package but to support this kind of notification it requires to be glued into notification.js.

How about having a "default" template containing title, description and close button for generic notifications, for other notifications the specific app returns a render-able object. The "Adblock update check daemon" adds a notification with the ID adblock.notification.update_available, which is a function defined luci-app-adblock/.../adblock.js. The function receives title, description and data which is then rendered and shown.

This approach allows i18n usage and even other interfaces, like a Android App or CLI to handle notifications.

I'd really like to get a comment from @jow- since he surely has more knowledge on system-design.

[jow-]

I think first we should define a schema of possible notification properties and ensure that these properties are output-agnostic.

Something along these lines:

{
    "title": "An adblock update is available",
    "description": "A longer description of an event, can be multiple lines and become very long, a potential UI renderer would put it behind a 'Read more...' button or similar.",
    "actions": [
      { "action": "dismiss" },
      { "action": "link", "url": "http://example.org/description/of/something" }   /* would be rendered by ui as "Read more..." button opening a new tab */
      { "action": "upgrade", "package": "adblock" }  /* would be rendered by ui as "Upgrade..." button triggering the corresponding opkg actions */
    ]
}

The same JSON could also be rendered on the command line, but instead of buttons it would render the actions as ASCII descriptions:

An adblock update is available

A longer description of an event, can be multiple lines and become very long, a potential UI
renderer would put it behind a 'Read more...' button or similar.

 - Check http://example.org/description/of/something for more information
 - To upgrade, execute "opkg upgrade adblock"

Ideally the description property content would support some minimal markdown style formatting, at least to allow things like bullet lists, bold, italic, underline and maybe hyperlinks.

Would complicate the UI renderer but would allow outputting the text as-is on the console (or maybe even with ANSII code formatting).

[aparcar]

I think first we should define a schema of possible notification properties and ensure that these properties are output-agnostic.

The schema looks good to me. As a remark which I failed to empathize before, non generic actions should be possible too... or not? It's not about upgrading a regular package but performing a package specific action.

Staying with the example, the (pseudo) "upgrade" action would be adblock-updater fetch http://adblock-update.org/list.txt. This could be represented as { "action": "shell", "command": "adblock-update [...]" } (dangerous?) or calling a LuCI function adblock.update-list which follows ACLs.

For now we could also define a minimal subset and extend "external" functions later on.

Ideally the description property content would support some minimal markdown style formatting, at least to allow things like bullet lists, bold, italic, underline and maybe hyperlinks.

Sounds good, let's please use Markdown and just a minimal subset (e.g. no tables). It looks fine in pure ASCII and should be easy enough to render.

[feckert]

@jow @aparcar Sorry I'm not convinced if that's the right way to go.

1. That is then no longer generic.
2. The problem with the translation is then also still open. Where should we add that.
3. It could also be that there is no LuCI module for the generated notification.
4. If I can create an action array with many action, how does that looks in the LuCI?

I have imagined the following generic notification.

• Interface goes up/down
• Mwan3 interface is online/offline
• Hard disk is full
• .....

In other software areas, IDs are also used. Why not here? The IDs handling can also be realized via plugins not all IDs must be handled in the switch state. If we use IDs this would make the handling clearer.

The advantage from my point of view is that the notification creation remains simple and I don't have to think in advance about what might be needed in the LuCI. Also all LuCI things for the notification are encapsulated in the LuCI module if I set an ID. For this to work, the creators of a notification must of course use a valid ID if the notification should be displayed correctly in the LuCI.

If no ID is set in the notification, then no special handling is done for this message to get displayed in the LuCI.

[dangowrt]

{
    "title": "An adblock update is available",
    "description": "A longer description of an event, can be multiple lines and become very long, a potential UI renderer would put it behind a 'Read more...' button or similar.",
    "actions": [
      { "action": "dismiss" },
      { "action": "link", "url": "http://example.org/description/of/something" }   /* would be rendered by ui as "Read more..." button opening a new tab */
      { "action": "upgrade", "package": "adblock" }  /* would be rendered by ui as "Upgrade..." button triggering the corresponding opkg actions */
    ]
}

I'd like to also see a field priority there so a bunch of relatively unimportant adblock-update-notifications can not take away the attention from something rather critical (think: disk full). Very high priority notifications should always go on top of all other notifications and may also be visually highlighted (bold red frame or something like that).

[jow-]

1. That is then no longer generic.

Why not?

2. The problem with the translation is then also still open. Where should we add that.

Why? It is trivial to extract the translation strings from structured JSON files. In fact we do that already for menu JSON entries in LuCI. What kind of problem do you see here?

3. It could also be that there is no LuCI module for the generated notification.

I proposed a generic definition with a fixed set of potential properties (title, descriptions) and actions (dismiss, link, upgrade). Why would there be notification specific LuCI modules needed?

4. If I can create an action array with many action, how does that looks in the LuCI?

Row of buttons for example, like in a normal modal dialog.

I have imagined the following generic notification.

* Interface goes up/down

* Mwan3 interface is online/offline

* Hard disk is full

* .....

These would be all notifcations with a sole action dismiss:

{
  "title": "Interface \"wan\" is up",
  "actions": [ { "action": "dismiss" } ] 
},
{
  "title": "MWAN3 tracked interface \"wan_b\" has been marked offline",
  "actions": [ { "action": "dismiss" } ] 
},
{
  "title": "Hard disk is full",
  "description": "There's less than 5MB of space available on /. You should remove unused files and directories."
  "actions": [ { "action": "dismiss" } ]
}

In other software areas, IDs are also used. Why not here? The IDs handling can also be realized via plugins not all IDs must be handled in the switch state. If we use IDs this would make the handling clearer.

I have no problem with IDs, but I really don't want to carry dozens of "notification type plugins" for notification banners users will likely never see. You'd also need to duplicate these plugins then for each potential output channel (rendering in ui, rendering in terminal, rendering for mail sending, rendering for slack notification, ...).

The advantage from my point of view is that the notification creation remains simple and I don't have to think in advance about what might be needed in the LuCI. Also all LuCI things for the notification are encapsulated in the LuCI module if I set an ID. For this to work, the creators of a notification must of course use a valid ID if the notification should be displayed correctly in the LuCI. If no ID is set in the notification, then no special handling is done for this message to get displayed in the LuCI.

And if LuCI is too old and has no handler for a particular ID (or notification type) then the notification is silently not shown? Or is it then handled by generic fallback code? If we need this generic fallback code then why not draft the system in a way that no type specific code is needed in the first place?

[dangowrt]

I proposed a generic definition with a fixed set of potential properties (title, descriptions) and actions (dismiss, link, upgrade). Why would there be notification specific LuCI modules needed?

Think of the example of adblock lists (ie. list data, not the package itself being updated): We will have to have actions to link to specific luci-apps to let the user perform an action there (think: click on "update lists" in luci-app-adblock) and/or have an action for generic RPC)

[jow-]

We will have to have actions to link to specific luci-apps to let the user perform an action there (think: click on "update lists" in luci-app-adblock) and/or have an action for generic RPC)

No. I was under the impression that a generic notification system is discussed here, not a LuCI notification system. A generic notification system has no notion of "trigger arbitrary action in arbitrary view of random ui".

If you want such a LuCI-only solution for e.g. adblock you could simply ship an autoload class which is instantiated with each page load and which then performs a number of checks and invokes ui.addNotification() with arbitrary markup contents if needed.

The luci-mod-battstatus module is an example for this facility (though it sets indicators instead of notifications, but the approach is the same).

[dangowrt]

Ok, cool :) Never mind the adblock use-case then. Maybe some more ideas for things to be potentially shown as notifications:

• ! running in initramfs (no actions)
• ! rootfs mounted read-only (no actions)
• ! SELinux running in permissive mode (no actions)
• ramoops/pstore crashlog of a previous boot available (maybe with number of records, actions: view pstore logs, dismiss/clear, could be implemented via /usr/libexec/rpcd offering ubus methods as well as luci-app-pstore offering a view using those methods)
• system time unset (actions: read more about RTCs, NTP and GPS -- or redirect to LuCI system/time setting)
• country not set (actions: read more... -- or redirect to LuCI wireless/device/country setting)

So to me it looks like there are system-level notifications (marked by !) without actions to be selected from, so that's easy (and that's the part I'm mostly interested in myself). Now to the more juicy ones: In order to be able to redirect to the system/time settings in case date/time is unset we could have the date/time settings view define that it allows reacting to the system-time-unset notification. Ie. instead of linking from the notification to specific view in LuCI, let a view declare which types of notifications it can handle.

[Ansuel]

@feckert @jow- @aparcar @dangowrt Uber mega spam... Did we found a way to implement this? Also the ubus proposal got stuck. Why? I can see many usercase where having a notification system is useful. Also this can be recycled to force a first init of the system by giving the user multiple notification to correctly setup the device.

[feckert]

It is not only a LuCI notification but from my point of view a system notification. This means that other systems that do not use LuCI can also genrate a notification here. The problem as far as I know was the I18n. Where should this be managed? Who is now taking care of the translation of the ubus or the LuCI?

[Ansuel]

@feckert IMHO we should not care about it... Every command and everything in shell in general is not translated and honestly if someone use console he probably know english as it's the defacto standard for IT configuration...

So we should really care about translation in LuCI. (a solution would be adding a small module for weblate for the package that will handle the description of all these notifications?)

(consider that we have the same exact problem with strings for incompatibility flags across different firmware that are currently not translated, so a weblate solution for this kind of stuff is need also for other thing)

[feckert]

In order to get the whole thing moving forward, we should first integrate the backend.

@aparcar Can you look at the backend again now?