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

Hari's solution #36

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions source/app/controllers/urls_controller.rb
Original file line number Diff line number Diff line change
@@ -1,2 +1,39 @@
class UrlsController < ApplicationController
def create
@url = Url.new(urls_params)

if @url.save
redirect_to urls_path
else
render 'new'
end
end

def index
@urls = Url.all
end

def new
@url = Url.new
end

def route_me
url = Url.find_by_short_url params[:short_url]

if url.nil?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer if url (drop .nil?) to phrase things positively, and put the "happy path" code in the top of the if/else.

redirect_to root_path
else
url.update('click_count' => url.click_count + 1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ActiveRecord has a method for this kind of thing.

There's a subtle bug as currently written. Do you see it?

redirect_to url.long_url
end


end

private

def urls_params
params.require(:url).permit(:long_url)
end

end
34 changes: 34 additions & 0 deletions source/app/models/url.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
require 'uri'
require 'net/http'

class Url < ActiveRecord::Base
before_save :shorten_url
validates :long_url, presence: true
validate :long_url_proper_format, :long_url_responds

private

def shorten_url
if short_url.nil?
self.short_url = (0...7).map { [*'0'..'9', *'A'..'Z', *'a'..'z'].sample }.join
end
end

def long_url_proper_format
unless long_url.start_with? 'http://', 'https://'
errors.add(:long_url, 'not in a valid format. (must start with http:// or https://)')
end
end

def long_url_responds
link = URI.parse(long_url)
if link.is_a?(URI::HTTP) || link.is_a?(URI::HTTPS)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This bit seems very similar to long_url.start_with? 'http://', 'https://' in the above method. I think a concept is duplicated here. 🤔

Net::HTTP.get_response(link)
else
errors.add(:long_url, 'must be reachable.')
end
rescue
errors.add(:long_url, 'must be reachable.')
end

end
14 changes: 13 additions & 1 deletion source/app/views/layouts/application.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,22 @@
<%= stylesheet_link_tag 'application', media: 'all', 'data-turbolinks-track' => true %>
<%= javascript_include_tag 'application', 'data-turbolinks-track' => true %>
<%= csrf_meta_tags %>

<link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/4.0.0/css/bootstrap.min.css" integrity="sha384-Gn5384xqQ1aoWXA+058RXPxPg6fy4IWvTNh0E263XmFcJlSAwiGgFAW/dAiS6JXm" crossorigin="anonymous">
<script src="https://code.jquery.com/jquery-3.2.1.slim.min.js" integrity="sha384-KJ3o2DKtIkvYIK3UENzmM7KCkRr/rE9/Qpg6aAZGJwFDMVNA/GpGFF93hXpG5KkN" crossorigin="anonymous"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/popper.js/1.12.9/umd/popper.min.js" integrity="sha384-ApNbgh9B+Y1QKtv3Rn7W3mgPxhU9K/ScQsAP7hUibX39j7fakFPskvXusvfa0b4Q" crossorigin="anonymous"></script>
<script src="https://maxcdn.bootstrapcdn.com/bootstrap/4.0.0/js/bootstrap.min.js" integrity="sha384-JZR6Spejh4U02d8jOt6vLEHfe/JQGiRRSQQxSfFWpi1MquVdAyjUar5+76PVCmYl" crossorigin="anonymous"></script>

</head>
<body>

<%= yield %>
<nav class="navbar navbar-dark bg-dark">
<a href="/"><span class="navbar-text"> <h4>BitlyClone | URL Shortner</h4> </span></a>
</nav> <br>

<div class="container">
<%= yield %>
</div>

</body>
</html>
11 changes: 11 additions & 0 deletions source/app/views/urls/index.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<h1>Welcome to URL Shortner | Home </h1>
<%= link_to 'Shorten a URL', new_url_path, class: "btn btn-primary" %>

<br><br>


<ol>
<% @urls.each do |url| %>
<li> <%= url.long_url %> | <a href="/<%= url.short_url %>"> <%= url.short_url %> </a>| <%= url.click_count %> </li>
<% end %>
</ol>
26 changes: 26 additions & 0 deletions source/app/views/urls/new.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<%= form_for :url, url: urls_path, local: true do |form| %>

<% if @url.errors.any? %>
<div id="error_explanation">
<h2>
<%= pluralize(@url.errors.count, "error") %> couldn't save this url
</h2>
<ul>
<% @url.errors.full_messages.each do |error_msg| %>
<li style="color: red"><%= error_msg %></li>
<% end %>
</ul>
</div>
<% end %>



<p>
<strong><%= form.label 'Enter URL: ' %></strong> <%= form.text_field :long_url, class: "form-control" %>
</p>


<p> <%= form.submit class: "btn btn-primary"%> </p>
<% end %>

<%= link_to 'Back', urls_path %>
3 changes: 3 additions & 0 deletions source/config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,7 @@
# # (app/controllers/admin/products_controller.rb)
# resources :products
# end
resources :urls
get '/:short_url' => 'urls#route_me'
root 'urls#index'
end
13 changes: 13 additions & 0 deletions source/db/migrate/20180125135057_create_urls.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
class CreateUrls < ActiveRecord::Migration
def change
create_table :urls do |t|
t.text :long_url
t.text :short_url
t.integer :visit_count
t.datetime :created_at
t.datetime :updated_at

t.timestamps
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class SetUrlVisitCountDefault < ActiveRecord::Migration
def change
change_column :urls, :visit_count, :integer, default: 0
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class RenameVisitCountToClickCount < ActiveRecord::Migration
def change
rename_column :urls, :visit_count, :click_count
end
end
24 changes: 24 additions & 0 deletions source/db/schema.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# encoding: UTF-8
# This file is auto-generated from the current state of the database. Instead
# of editing this file, please use the migrations feature of Active Record to
# incrementally modify your database, and then regenerate this schema definition.
#
# Note that this schema.rb definition is the authoritative source for your
# database schema. If you need to create the application database on another
# system, you should be using db:schema:load, not running all the migrations
# from scratch. The latter is a flawed and unsustainable approach (the more migrations
# you'll amass, the slower it'll run and the greater likelihood for issues).
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 20180125182544) do

create_table "urls", force: true do |t|
t.text "long_url"
t.text "short_url"
t.integer "click_count", default: 0
t.datetime "created_at"
t.datetime "updated_at"
end

end
5 changes: 5 additions & 0 deletions source/spec/models/url_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
require 'rails_helper'

RSpec.describe Url, :type => :model do
pending "add some examples to (or delete) #{__FILE__}"
end