Add Mastodon connector to settings / profile #1173
Conversation
jmcharter
force-pushed
the
feat/mastodon-profile-field
branch
from
5ac6c3b
to
a525ea9
Compare
jmcharter
force-pushed
the
feat/mastodon-profile-field
branch
from
a525ea9
to
e71127d
Compare
|
Thanks for taking on such a big feature! The Twitter endpoint is getting less reliable, so this is pretty timely. It looks like you’ve made a solid start. I can see you’re still working on this, so please |
| </p> | ||
|
|
||
| <div class="boxline"> | ||
| <%= f.label :mastodon_instance_name, "Mastodon Instance:", :class => "required" %> |
There was a problem hiding this comment.
We have a few placeholder attributes in the code, I think this would be a good place for one. With the way you wrote sanitized_instance_name, you don’t even need to include the instruction above about not including https.
There was a problem hiding this comment.
Ah yes, good point! I added sanitized_instance_name later. I'll await further review before making any changes, but I've noted this.
|
@pushcx no problem. I'll be very happy to see Mastodon added as social connection option. Please review whenever you have time. |
There was a problem hiding this comment.
This is a solid start, I see how you got it working with a minimum of fuss. I left some small notes, but this also needs some high-level rearranging.
The User model should has_one :mastodon_instance rather than repeating the instance name. The functions in Mastodon that take instance_name as an argument should move into MastodonInstance (perhaps register_application should be a factory?).
extras/mastodon.rb
Outdated
| :scope => "read" | ||
| ).body | ||
| ps = JSON.parse(res) | ||
| puts "ps:", ps |
extras/mastodon.rb
Outdated
| :post, | ||
| :client_id => client_id, | ||
| :client_secret => client_secret, | ||
| :redirect_uri => "http://localhost:3000/settings/mastodon_callback?instance=#{instance}", |
There was a problem hiding this comment.
Replace local host with Rails.application.domain
| <% end %> | ||
|
|
||
| <div class="box wide"> | ||
| <%= form_with url: "/settings/mastodon_auth", method: :get do |f| %> |
There was a problem hiding this comment.
Use the Rails url generation here and above. Run rails routes in your console to see the name for this endpoint, then add _path to generate a relative URL.
| return mastodon_disconnect | ||
| end | ||
|
|
||
| return redirect_to "/settings" |
|
|
||
| <div class="boxline"> | ||
| <%= f.label :mastodon_instance_name, "Mastodon Instance:", :class => "required" %> | ||
| <%= f.text_field :mastodon_instance_name, :autofocus => true %> |
| return redirect_to "/settings" | ||
| end | ||
|
|
||
| tok, username = Mastodon.token_and_user_from_code(params[:instance], params[:code]) |
There was a problem hiding this comment.
I think this creates duplicate MastodonInstances when two users use the same Instance. You probably want MastodonInstance.find_or_create_by
|
@pushcx thanks for taking the time to review this PR. My inexperience with Ruby / Rails is shining through, clearly. |
pushcx
force-pushed
the
master
branch
2 times, most recently
from
1944bdd
to
1c73b2a
Compare
jmcharter
force-pushed
the
feat/mastodon-profile-field
branch
from
2045cb4
to
169c7ea
Compare
|
@jmcharter Thanks for taking this on. I had some free dev time so I decided to get this over the line. |
|
@pushcx thanks for doing so - I've always meant to come back to this, but you know how it is. Cheers! |
Related to #1141
Adds a profile connector for Mastodon to settings and public profile.
This is similar to how GitHub and Twitter are currently handled, but with a key difference: There's no central app registration. Each instance requires OAuth app registration. I've handled this by registering with an instance the first time a user connects and storing the client_id and client_secret for future connections.