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

Federate custom emojis with accounts #6124

Merged
merged 2 commits into from
Apr 1, 2018

Conversation

davefp
Copy link
Contributor

@davefp davefp commented Dec 28, 2017

Fixes #5666

This PR Adds an option to the simplified_format helper to render custom emoji.

It also adds this option to calls made from the REST account serializer and the account show partial.

Finally, there are unit tests for the new option.

I'm not familiar with the ins and outs of the various views and serializers, so if this PR puts code in the wrong place please let me know. Thanks!

Copy link
Contributor

@akihikodaki akihikodaki left a comment

Choose a reason for hiding this comment

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

The idea sounds great, but this implementation does not federate, does it?

@davefp
Copy link
Contributor Author

davefp commented Jan 4, 2018

I don't know. I'm not familiar enough with the codebase to recognize which bits deal with federation vs. which don't. Can you expand on which bits are problematic?

@akihikodaki
Copy link
Contributor

akihikodaki commented Jan 5, 2018

To federate:

  • the sender should attach emojis
  • the receiver should receive attached emojis

For instance, the sender implementation for statuses attaches emojis

  • to note as tags for ActivityPub (see ActivityPub::NoteSerializer)
  • to post object as links for OStatus (see OStatus::AtomSerializer)

The receiver implementation for statuses resolves emojis

  • in Create object for ActivityPub (see ActivityPub::Activity::Create)
  • in post object for OStatus (see OStatus::Activity::Creation)

Similar changes are required by:

  • ActivityPub::ActorSerializer
  • OStatus::AtomSerializer
  • ActivityPub::ProcessAccountService
  • UpdateRemoteProfileService

@davefp
Copy link
Contributor Author

davefp commented Jan 5, 2018

Thanks! I'll take a look at this in more detail over the weekend.

@akihikodaki akihikodaki dismissed their stale review January 9, 2018 03:28

My concern seems addressed but I have not reviewed yet.

@davefp
Copy link
Contributor Author

davefp commented Jan 9, 2018

I also want to add tests and do a little refactoring, please do not merge yet :)

@nightpool nightpool changed the title Enable custom emoji on account pages and in the sidebar [WIP] Enable custom emoji on account pages and in the sidebar Jan 9, 2018
@Gargron
Copy link
Member

Gargron commented Apr 1, 2018

This looks alright, I think, is it still WIP?

@davefp
Copy link
Contributor Author

davefp commented Apr 1, 2018 via email

@Gargron Gargron changed the title [WIP] Enable custom emoji on account pages and in the sidebar Federate custom emojis with accounts Apr 1, 2018
@Gargron Gargron merged commit 123a343 into mastodon:master Apr 1, 2018
@Gargron
Copy link
Member

Gargron commented Apr 1, 2018

For reference, I think this is still missing the parts for rendering custom emojis within the web UI.

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.

None yet

3 participants