Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Notification management #26

Open
julien-nc opened this issue Aug 2, 2023 · 2 comments
Open

Notification management #26

julien-nc opened this issue Aug 2, 2023 · 2 comments
Labels
enhancement New feature or request

Comments

@julien-nc
Copy link
Member

Goals of the notification system in the app ecosystem v2:

  • not break the compatibility with the SDKs when we extend it
  • be generic enough so that we don't have a limited list of supported use cases
  • allow the apps to set
    • the notification target (INotification::setLink)
    • an icon (by path? like actions/download)
    • a title/subject
    • a message
    • the rich parameters for the title and message
    • a list of actions

We would have our own data format for the API calls, no need to be close to what the core notification system does, the app ecosystem parses the params sent by the ext apps and produces notifications.

For links (notification target and action link), we could support a list of target elements (file, user, talk room...) so the SDK could avoid generating URLs and leave it to the app ecosystem. For example, the SDK would pass

{
    "type": "file",
    "id": "123",
}

and it would result in calling

$notification->setLink($this->url->linkToRoute('files.view.index', ['fileid' => 123]));

For the rich parameters, we could just directly use the ones sent by the ext apps so we would support everything supported by INotification.

Any other aspect of the notifications we should cover?

@bigcat88
Copy link
Member

bigcat88 commented Aug 2, 2023

  • an icon (by path? like actions/download)

Suggest to postpone this feature into the future -> (v1.1+)
This is not hard to do, but we do not need from start.
Adding this will not break compatibility, as currently we does not have such ability at all.

For links (notification target and action link), we could support a list of target elements (file, user, talk room...) so the SDK could avoid generating URLs and leave it to the app ecosystem. For example, the SDK would pass

Can we postpone this to version v1.2-v1.3 at least?
This will require a large amount of tests with stuff that is currently not implemented(talk room stuff will require me to implement support of talk in nc_py_api as we test most stuff with python)...

  • a list of actions

Suggest to postpone this feature into the future -> (v2.0+)
This will be a little big harder, will not break compatibility, but currently it will be fine without it, imho.
If there will be many requests for this we always can implement it earlier.

@julien-nc
Copy link
Member Author

Agreed, agreed and agreed.

Adding this will not break compatibility

That's an important point indeed, whatever you do, let's try to not break the SDK implementations when we improve/extend this.

Let's keep track of what's happening regarding notifications here.

@bigcat88 bigcat88 added the enhancement New feature or request label Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants