LemmyNet / lemmy

🐀 A link aggregator and forum for the fediverse
https://join-lemmy.org
GNU Affero General Public License v3.0
13.18k stars 871 forks source link

Rework notifications API #2441

Open Nutomic opened 2 years ago

Nutomic commented 2 years ago

The notifications api is currently very complicated and confusing. If you want to handle notifications in a client, you need to follow essentially these steps:

The problem is that theres not only mention, but also private message and comment reply which require the same logic to be duplicated. So you essentially have to write the same code 3 times.

To simplify this, I propose to implement the API in the following way instead:

This probably means that there would be a new db table Notification, which holds the id and other details about each notification.

dessalines commented 2 years ago

These are 3 different tables (mention, reply, and private_message) , with 2 different types (comment_id, and private_message_id). They need to be separate API fetches, so that UIs can continue filtering by the type if they choose. lemmy-UI wouldn't be able to have these filters otherwise:

Screenshot_2022-09-13-17-13-36-293_mark via gp

The tables are person_mention, comment_reply, and private_message. The first two link to comment_id's, and the third is the private message table. These already are notification tables, since they have read columns, and link to the comment_id (except the third which just links to the table directly).

I'm also kind of against having API's return different types of data in lists, and then forcing front ends to use generics and type checking to figure out what they are. IMO its better to have predictable, well-typed list items coming back from the API.

Nutomic commented 2 years ago

I think these filters are only a secondary functionality, compared to the main functionality of showing all notifications. I probably wont implement them at all in lemmyBB.

The data with serde adjacent tagging would look like this. So you just need to have a switch based on the type field, and pass data to the appropriate render function. I think its even easier than the multiple api calls which are necessary now.

{
    "notitfication_id": 123, 
    "type": "mention"
    "data": {
        // fields from CommentView
        ...
    }
dessalines commented 2 years ago

Could you show an example of how this would look in rust? IE a Vec of multiple potential types?

Nutomic commented 2 years ago

Something like this:

struct Notification {
    notificationId: i32,
    otherCommonFields: Xyz,
    data: NotificationData
}

enum NotificationData {
    Mention(CommentView),
    Reply(CommentView)
    PrivateMessage(PrivateMessageView)

Also check the serde docs which i linked above: https://serde.rs/enum-representations.html#adjacently-tagged

dessalines commented 1 year ago

2824