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

#5397 proper links for mentions and tags in outgoing tumblr posts #6407

Conversation

@Realtin
Copy link
Contributor

Realtin commented Sep 17, 2015

We reformatted the tags and mentions to be links to the tag path and person's profile both formatted as markdown

add link to tag and mention in tumblr export so user is redirected to diaspora
tag path and person's profile both formatted as markdown

add some specs to test the new formatting
(#5397)
# the following regular expressions matches the tag with its surrounding html
# the two capture groups capture the tag with and without #-sign to build the
# markdown link
html.gsub!(/<a\s.+?>(#([\w|\d|_-|<3]+\b))<\/a>/, '[\1]' + "(#{AppConfig.pod_uri}tags/" + '\2)')

This comment has been minimized.

Copy link
@diaspora-code-review

diaspora-code-review Sep 17, 2015

Use %r around regular expression.

This comment has been minimized.

Copy link
@jhass

jhass Sep 17, 2015

Member

We really shouldn't duplicate the tag regexp here, rather add options to MessageRenderer and subsequently Taggable to output the links with the full URL.

This comment has been minimized.

Copy link
@jhass

jhass Sep 17, 2015

Member

Looking around there probably is a nice way we can reuse https://github.com/diaspora/diaspora/blob/develop/lib/diaspora/taggable.rb#L62 (through MessageRenderer)

@@ -39,8 +39,23 @@ def tumblr_template(post, url)
post.photos.each do |photo|
html << "![photo](#{photo.url(:scaled_full)})\n\n"
end
# save all the mentions within a post
@mentions = []

This comment has been minimized.

Copy link
@jhass

jhass Sep 17, 2015

Member

I don't think this needs to be an instance variable.

@@ -39,8 +39,23 @@ def tumblr_template(post, url)
post.photos.each do |photo|
html << "![photo](#{photo.url(:scaled_full)})\n\n"
end
# save all the mentions within a post
@mentions = []
post.message.instance_values["options"][:mentioned_people].each do |o|

This comment has been minimized.

Copy link
@jhass

jhass Sep 17, 2015

Member

We probably should add proper API for accessing it.

@mentions = []
post.message.instance_values["options"][:mentioned_people].each do |o|
@person_url = o.url + "people/" + o.guid
@mentions << {name: o.name, url: @person_url}

This comment has been minimized.

Copy link
@jhass

jhass Sep 17, 2015

Member

o is a person instance, right? Name it person and use person.profile_url

post.message.instance_values["options"][:mentioned_people].each do |o|
@person_url = o.url + "people/" + o.guid
@mentions << {name: o.name, url: @person_url}
end

This comment has been minimized.

Copy link
@jhass

jhass Sep 17, 2015

Member

This is a map

end

it "properly links tags to the pod's tag site" do
expect(tumblr_export).to include("[#ilike](#{AppConfig.pod_uri}tags/ilike)")

This comment has been minimized.

Copy link
@jhass

jhass Sep 17, 2015

Member

AppConfig.url_to("/tags/ilike")

@zaziemo

This comment has been minimized.

Copy link
Contributor

zaziemo commented Sep 17, 2015

@jhass thanks for all the feedback. We will implement the changes!

html << post.message.html(mentioned_people: [])
html << "\n\n[original post](#{url})"
@mentions.each { |mention|
html.gsub!("#{mention[:name]}", "[#{mention[:name]}](#{mention[:url]})")

This comment has been minimized.

Copy link
@jhass

jhass Sep 17, 2015

Member

Mmmh, this will also replace occurrences of the name anywhere that aren't meant as mentions. Say I mention a user named "commit" and post a link like https://github.com/foo/bar/commit/132345abdef ...

I'm afraid this likewise needs to add support to Mentionable for generating full URL links and expose that as a MessageRenderer option, perhaps a common full_urls option.

@denschub

This comment has been minimized.

Copy link
Member

denschub commented Mar 22, 2016

Closing due to inactivity.

@denschub denschub closed this Mar 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.