-
Notifications
You must be signed in to change notification settings - Fork 58
WIP: optional: translate twitter to mastodon mentions #650
Conversation
note that only twitter mentions of accounts registered on this crossposter instance are translated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't been working with Rails in the last 8 years and I am a bit overwhelmed to understand how I can setup my environment and use test-driven development. We should definitely cover this feature with tests.
if Rails.configuration.x.translate_twitter_accounts | ||
mapping = User.twitter_mastodon_mapping | ||
mapping.default_proc = proc {|hash, key| key } | ||
text.gsub!(twitter_mention_regex, mapping) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not work currently. twitter_mention_regex
also matches leading spaces. However, the mapping hash does not have this leading space and thus does not carry out a substitution.
@renatolond What is the best approach here? Add a different twitter regexp?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't remember the mention results exactly, but I think you have the leading spaces in \1
, so you could something like:
gsub!(..., "\1#{mapping}")
since you're using a proc there, I'm not too sure on how you could do that, though 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi! mapping
is a hash of the form {"@twitter_handle" => "@mastodon_handle@server.com", ...}
. gsub can pick here the replacement text if the capture is found amongst the hash keys. The default_proc ensures that twitter mentions not found in the mapping are just replaced by themselves, i.e. not modified.
The expression "\1#{mapping}"
would be converted into a string and gsub would then replace all captures by the same text.
If we can rewrite the twitter_mention_regex
to not match a leading space/char, then we could use the regexp. I assume though that there was a specific reason to match a leading space…
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, and wouldn't it work if you do a transform_keys in your mapping
hash to have \1@twitter_handle
keys instead?
@@ -184,6 +184,9 @@ def self.get_authorization(provider, uid) | |||
BLOCKED_UIDS = ["example@bad.bad.server"] | |||
|
|||
def self.from_omniauth(auth, current_user) | |||
# reset twitter mastodon mapping cache upon human change of the user database | |||
Rails.cache.delete('/twitter_mastodon_mapping') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the cache should be devalidated (deleted), whenever a user is deleted or created. The position in the code is not obvious to me. I think it could fit here.
# https://developer.twitter.com/en/docs/twitter-api/users/lookup/api-reference/get-users-by-username-username | ||
# rate limit: 900 requests per 15-min window shared among all users of your app | ||
User.all.each do |user| | ||
[user.twitter_handle, "@#{user.mastodon_id}"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked the Twitter API and it seams the translation from Twitter IDs to twitter_handle
does not allow for a batch request. With many accounts registered on a crossposer instance, this call to #twitter_handle
in a loop can potentially be heavy.
I don't expect more than 200 accounts for the next months. So it's acceptable for the time being. However, I wonder if the twitter_handle should not be put in the local database alongside the twitter ID.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do think it should go in the database, it would make more sense. Then you don't have to deal with the cache, it should go in the Authorization
table, so that it gets cleared automatically if the user changes the twitter account they linked :)
You also need to commit the changes to the schema file, that's why the tests are failing here :) |
I guess the schema file is updated upon |
Closes #930, #913, #911, #906, #904, #897, #896, #895, #890, #886, #880 Closes #877, #872, #871, #868, #866, #863, #862, #859, #820, #810, #809 Closes #724, #721, #711, #688, #679, #606, #603, #593, #581, #577, #551 Closes #533, #517, #506, #505, #493, #430, #422, #311, #297, #241, #238 Closes #233, #197, #165, #164, #161, #160, #159, #151, #150, #148, #146 Closes #140, #139, #138, #135, #134, #127, #124, #123, #120, #119, #118 Closes #116, #112, #109, #106, #105, #103, #96, #90, #88, #86, #85, #74 Closes #65, #55, #47, #36, #33, #28, #19, #18 Closes #957, #955, #953, #950, #949, #936, #927, #925, #894, #873, #867 Closes #864, #650
note that only twitter mentions of accounts registered on this
crossposter instance are translated