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

Custom and remote emojis #405

Merged
merged 21 commits into from
Nov 11, 2023
Merged

Custom and remote emojis #405

merged 21 commits into from
Nov 11, 2023

Conversation

zeerooth
Copy link
Collaborator

@zeerooth zeerooth commented Oct 29, 2023

It's not ready to be merged or even reviewed, but I'd like to already open this draft PR to keep track of the progress and address potential feedback.

How it works:

  • Server saves all the emojis that it gets through through federated posts (to confirm that they are genuine they get resolved as an AP type, just like accounts are)
  • Users then are able to use :[email protected]: syntax to insert the emojis into posts (unfortunately it doesn't work for emojis that the server doesn't know about yet as there's no webfinger like for the accounts)
  • These emojis get translated into :emoji__example_com: (neither mastodon nor misskey allow special characters like @ or . which sucks) and inserted into the tag list, with id and image url pointing to the original resource.
  • Voila, it works

Things that are already implemented and work:

  • Parser for emojis in posts
  • DB tables
  • Barebones emoji service

Things that probably work but need to be tested:

  • Emoji federation - mastodon actually exposes emojis as a resolvable AP type. I implemented a basic resolver but haven't tested it yet. Also who knows how it behaves with misskey or pleroma for example. it works!

To be added:

  • GET /api/v1/custom_emojis
  • GET /api/v1/statuses/{id} <- needs the emoji list
  • Update emojis during post updates
  • More tests
  • Expose the emojis as a resolvable AP type

Out of scope for now (I'll make an issue for it later)

  • Resolve emojis in account properties, such as description, display name, fields
  • Make it easy to add local emojis through the CLI and/or admin panel (if we ever end up making one)

Issue ref: #105

@aumetra
Copy link
Member

aumetra commented Nov 3, 2023

Screenshot_20231103-083818.png

It indeed does work, heh

@zeerooth zeerooth changed the title [Draft/Experimental] Custom and remote emojis Custom and remote emojis Nov 7, 2023
@zeerooth zeerooth requested a review from aumetra November 7, 2023 00:20
@zeerooth
Copy link
Collaborator Author

zeerooth commented Nov 7, 2023

hmmm I have no idea why the tests keep timing out

@zeerooth zeerooth marked this pull request as ready for review November 8, 2023 19:33
@zeerooth
Copy link
Collaborator Author

zeerooth commented Nov 8, 2023

@aumetra could you take a look at this PR when you've got some time pls?

@aumetra
Copy link
Member

aumetra commented Nov 9, 2023

could you take a look at this PR when you've got some time pls?

Will do on my commute home

Copy link
Member

@aumetra aumetra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some initial comments. I'll look at it closer later

crates/kitsune-core/src/activitypub/mod.rs Outdated Show resolved Hide resolved
crates/kitsune-core/src/mapping/activitypub/object.rs Outdated Show resolved Hide resolved
crates/kitsune-core/src/mapping/activitypub/object.rs Outdated Show resolved Hide resolved
crates/kitsune-core/src/service/custom_emoji.rs Outdated Show resolved Hide resolved
lib/post-process/src/lib.rs Outdated Show resolved Hide resolved
@zeerooth
Copy link
Collaborator Author

@aumetra thanks so much for these comments, I addressed everything now :)

BTW there is one more aspect of this PR that I was thinking about - for the custom emoji table I decided to not save the domain for local emojis (it's NULL), but I saw that in other tables, for example for user accounts, the domain part is not nullable and saved even for local accounts. Should I change it accordingly for custom emojis?

@aumetra
Copy link
Member

aumetra commented Nov 10, 2023

Should I change it accordingly for custom emojis?

I don't think that's necessary here. We store the domains on accounts and users because of potential multi-tendant support (i.e. one instance with multiple domains).

But not everyone will be allowed to add custom emoji anyway, just the moderators and admins of the overall system, so might as well not store the domain here.

@zeerooth
Copy link
Collaborator Author

Right, then I'll leave it as it is

@aumetra aumetra mentioned this pull request Nov 11, 2023
@aumetra
Copy link
Member

aumetra commented Nov 11, 2023

@zeerooth would you mind rebasing this? I don't really have any more comments to this

@aumetra
Copy link
Member

aumetra commented Nov 11, 2023

Merged main into it. Do you have any last things you wanna add before I hit the merge button?

@zeerooth
Copy link
Collaborator Author

Merged main into it. Do you have any last things you wanna add before I hit the merge button?

Not really, just that you're too quick, I merged it myself but then I try to push and see that you already pushed to this branch lol

Thanks though, you can merge it now :)

@aumetra aumetra added this pull request to the merge queue Nov 11, 2023
Merged via the queue into kitsune-soc:main with commit b633701 Nov 11, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants