forem / forem

For empowering community 🌱
https://forem.com
GNU Affero General Public License v3.0
21.98k stars 4.04k forks source link

Add outside link functionality to {% link %} liquid tag #274

Closed jessleenyc closed 5 years ago

jessleenyc commented 6 years ago

@benhalpern commented on Thu Jul 12 2018

TASK or Feature

Request or User Story

As a user I'd like to be able to use the {% link interface in order to post outside links.

This would involve fetching the info from the outside link and display it. There are some design decisions here that could use some discussion. It should basically look like the existing type of link liquid tag, but maybe not bother with an image at all. Info can be scraped with Nokogiri (or other gem if you think it would be better)

We should also persist the link in our DB. I think OutsideWebPage could be a good name for a new DB model, but something else might be more elegant. Let's limit the attributes on this model to just what it takes to render this tag, but we could add more functionality later.

Definition of Done

This is done when the {% link tag can be fetched and displayed for outside links. If you wanna push some design decisions til the last minute, that's cool.

joybh98 commented 6 years ago

Can I work on this?

GMartigny commented 6 years ago

It can be pricey for enterprise, but you could take a look at https://embed.ly/ services.

jessleenyc commented 6 years ago

@JoyBhalla Yes! Let's over communicate once you get to some of the design considerations here.

joybh98 commented 6 years ago

Okay!

joybh98 commented 6 years ago

I am having some doubts over how and where i should implement it,and how to read the codebase,can you guide me to a starting point ?

tterb commented 6 years ago

I previously looked into taking this on and I believe the starting point would be adding the additional logic to the existing {% link liquid tag in app/liquid_tags/link_tag.rb to accept and differentiate between dev.to articles and outside links.
Though, I'm admittedly new to Rails so I could be wrong.

joybh98 commented 6 years ago

Thanks @tterb for the help,appreciate it.

joybh98 commented 6 years ago

I am having some problems with setting it up in my computer,so if anyone wants to work on this they please do.

jessleenyc commented 6 years ago

@JoyBhalla are you having issues with general setup or specifically with liquid tags?

joybh98 commented 6 years ago

@jessleenyc first i was stuck on installing the app on my machine,and then the server was not running. I want to work on it, but i don't want the pace to slow down.

joybh98 commented 6 years ago

After running bin/startup this is the output, screenshot from 2018-08-22 11-35-13

And this is the error i'm getting when opening localhost:3035 screenshot from 2018-08-22 11-35-26

How can i resolve this issue ?

rhymes commented 6 years ago

@JoyBhalla the app seems to work fine, you're trying to open the webpacker URL page, you should open the Rails URL: http://localhost:3000

I get the confusion, there's a lot of output in the console and the tutorial doesn't mention the Rails URL :D

@maestromac pinging you again, what do you think about expanding the doc a little bit more? Maybe the README should link http://localhost:3000 after the bin/startup step? thanks!

joybh98 commented 6 years ago

@rhymes thanks,appreciate it !

maestromac commented 6 years ago

@rhymes most definitely. I'll add that.

joybh98 commented 6 years ago

@jessleenyc should i make a new file for the outside link tag or write the code in _'linktag.rb

joybh98 commented 6 years ago

I've been reading up on Nokogiri and what i've observed is that Nokogiri is assuming that we know the HTML Structure of the site we are using,and different sites use different structures for their rendered HTML,can anyone give me any suggestions on what I should do ?

rhymes commented 6 years ago

If you see the code for the https://github.com/thepracticaldev/dev.to/blob/master/app/liquid_tags/link_tag.rb the render method shows a few things:

This is the result:

screenshot_2018-08-26 the dev community

One additional thing you could look for, instead of the profile image, is the OpenGraph's <meta property="og:image"> tag which means you would likely need to generate a cloudinary link for this new image of yours so that it can be properly processed for the size you need. You can check https://github.com/thepracticaldev/dev.to/blob/master/app/labor/profile_image.rb#L11 and https://github.com/thepracticaldev/dev.to/blob/master/app/liquid_tags/link_tag.rb#L15 for an example on how to do it.

Here's an example on how to parse HTML with Nokogiri: http://ruby.bastardsbook.com/chapters/html-parsing/ - You don't have to wonder which structure the page has. If, using xpath or css selectors, you find (for example), a "title tag inside a head tag" you extract its value. We can assume that most pages do have a <head>, otherwise it's malformed HTML, you don't need to go past the <head> anyhow

The only value you need to look for in the HTML for sure is the title of the page. The rest you can probably default to something: no image? use a place holder. no keywords/tags? Don't know what you could do if the title tag is absent, this needs further discussion.

A last tip: since you're going to effectively issue a HTTP GET command everytime this tag is instantiated use a timeout, for example: HTTParty.get('http://www.google.com', timeout: 2) - this way it doesn't hang forever and if in the time frame you deem appropriate you don't get a response let the user see some error like "hey, the website is not available, please try again"

That's all I can think of right now

joybh98 commented 6 years ago

I used rpsec for testing and then i opened the index.html file afterwards and read the test coverage,how will I know that i've passed the tests ?

benhalpern commented 6 years ago

If you run regular bin/rspec spec, you should see the test pass/fail results right there in the terminal.

joybh98 commented 6 years ago

screenshot from 2018-08-27 09-59-42 Getting this offense when commiting,but i need open so it can use the outside link,should i bypass it ?

rhymes commented 6 years ago

@JoyBhalla no, don't use open to open URLs. As I suggested in my previous comment use HTTParty, which is already part of the dev.to codebase:

A last tip: since you're going to effectively issue a HTTP GET command everytime this tag is instantiated use a timeout, for example: HTTParty.get('http://www.google.com', timeout: 2) - this way it doesn't hang forever and if in the time frame you deem appropriate you don't get a response let the user see some error like "hey, the website is not available, please try again"

stale[bot] commented 5 years ago

Thanks for contributing to this issue. As it has been 90 days since the last activity, we are automatically closing the issue in 7 days. This is often because the request was already solved in some way and it just wasn't updated or it's no longer applicable. If this issue still requires attention, please respond with a comment. Happy Coding!