Conversation
| private | ||
|
|
||
| def set_url | ||
| @url = Url.find_by(short_url: params[:short_url]) |
There was a problem hiding this comment.
Since this method is only invoked for one controller action, I'd suggest inlining it there.
| @url.click_count += 1 | ||
| @url.save | ||
|
|
||
| redirect_to "#{@url.long_url}" |
There was a problem hiding this comment.
The string interpolation isn't needed here. redirect_to @url.long_url should do the trick.
| end | ||
|
|
||
| def url_params | ||
| params.require(:url).permit(:long_url, :short_url) |
There was a problem hiding this comment.
We permit setting the short_url here, but there isn't an input in the form for it, and it will be overwritten in the model. I think short_url should be removed here.
| def create_short_url | ||
| begin | ||
| short_url = SecureRandom.hex(4) | ||
| end while Url.where(short_url: short_url).exists? |
There was a problem hiding this comment.
I'm pretty sure Url.exists?(short_url: short_url) works here too.
|
|
||
| def compliant? | ||
| new_uri = URI.parse(self.long_url) | ||
| new_uri.is_a?(URI::HTTP) && !new_uri.host.nil? |
There was a problem hiding this comment.
I thought you had a bug here. Instead, I just learned that URI::HTTPS is a subclass of URI::HTTP. 👍
|
I don't see an implementation for this URL validation yet:
|
2 participants
@jaybobo
just letting you know. the secret page is a user's profile page.