feat: Glitch-soc/Firefish emoji reactions API #1921
Conversation
ThatOneCalculator
changed the title
|
Please pass tests. |
| @@ -5,6 +5,7 @@ namespace NotificationType { | |||
| export const Favourite: Entity.NotificationType = 'favourite' | |||
| export const Reblog: Entity.NotificationType = 'reblog' | |||
| export const Mention: Entity.NotificationType = 'mention' | |||
| export const Reaction: Entity.NotificationType = 'reaction' | |||
There was a problem hiding this comment.
Why did you add a new notification type? Is it used?
There was a problem hiding this comment.
There was a problem hiding this comment.
Why did you add Reaction even though EmojiReaction exists? What's the difference?
There was a problem hiding this comment.
Mostly to stay in line with Firefish's reaction notification name. I don't think it's very necessary but helps with differentiating string-only reaction notifications (Pleroma) from Reaction object notifications
There was a problem hiding this comment.
Ah, I got it.
You mean
In Pleroma(current):
When we receive an EmojiReaction notification, it contains emoji field in the object (it does not contain reaction). And it should be NotificationType.EmojiReaction.
In Firefish:
When we receive a Reaction notification, it contains reaction field in the object (it does not contain emoji). And it should be NotificationType.Reaction.
Furthermore, in the future Pleroma, when we receive an EmojiReaction, it contains reaction field in the object instead of emoji field. And at that time, we can remove NotificationType.EmojiReaction and emoji field in the notification object.
Correct?
There was a problem hiding this comment.
Yep, exactly.
|
I'll be able to address the review items later today :) |
Adapted from https://git.joinfirefish.org/firefish/firefish/-/merge_requests/10532/diffs#diff-content-acabf6b2898e3102f138b148d469c390595935d1 from @VyrCossont