-
Notifications
You must be signed in to change notification settings - Fork 40
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
WIP #18
base: master
Are you sure you want to change the base?
WIP #18
Changes from 14 commits
77587f9
50e64f4
cb97f4a
b7fb95e
8121eaa
dbf14e3
b8781f1
757d275
0988a69
7f3c8d2
9b60634
54d5414
93fe6c1
3e84172
fc84dbd
31a254c
c73e277
fcb3964
bedc27c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,3 +14,6 @@ | |
# Ignore all logfiles and tempfiles. | ||
/log/*.log | ||
/tmp | ||
|
||
# Ignore Vim swapfiles | ||
.*.swp |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
--color | ||
--require spec_helper |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
body { | ||
background-color: #fff; | ||
color: #333; | ||
font-family: verdana, arial, helvetica, sans-serif; | ||
font-size: 13px; | ||
line-height: 18px; | ||
} | ||
|
||
p, ol, ul, td { | ||
font-family: verdana, arial, helvetica, sans-serif; | ||
font-size: 13px; | ||
line-height: 18px; | ||
} | ||
|
||
pre { | ||
background-color: #eee; | ||
padding: 10px; | ||
font-size: 11px; | ||
} | ||
|
||
a { | ||
color: #000; | ||
&:visited { | ||
color: #666; | ||
} | ||
&:hover { | ||
color: #fff; | ||
background-color: #000; | ||
} | ||
} | ||
|
||
div { | ||
&.field, &.actions { | ||
margin-bottom: 10px; | ||
} | ||
} | ||
|
||
#notice { | ||
color: green; | ||
} | ||
|
||
.field_with_errors { | ||
padding: 2px; | ||
background-color: red; | ||
display: table; | ||
} | ||
|
||
#error_explanation { | ||
width: 450px; | ||
border: 2px solid red; | ||
padding: 7px; | ||
padding-bottom: 0; | ||
margin-bottom: 20px; | ||
background-color: #f0f0f0; | ||
h2 { | ||
text-align: left; | ||
font-weight: bold; | ||
padding: 5px 5px 5px 15px; | ||
font-size: 12px; | ||
margin: -7px; | ||
margin-bottom: 0px; | ||
background-color: #c00; | ||
color: #fff; | ||
} | ||
ul li { | ||
font-size: 12px; | ||
list-style: square; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,20 @@ | ||
class SessionsController < ApplicationController | ||
def new | ||
end | ||
|
||
def create | ||
user = User.find_by(email: params[:session][:email].downcase) | ||
if user && user.authenticate(params[:session][:password]) | ||
log_in user | ||
redirect_to root_url | ||
else | ||
flash[:danger] = 'Invalid email/password combination' | ||
render 'new' | ||
end | ||
end | ||
|
||
def destroy | ||
log_out | ||
redirect_to login_path | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,47 @@ | ||
class UrlsController < ApplicationController | ||
# GET /urls | ||
# GET /urls.json | ||
def index | ||
redirect_to login_path and return if !logged_in? | ||
@urls = Url.all | ||
end | ||
|
||
# GET /urls/1 | ||
# GET /urls/1.json | ||
def show | ||
url = Url.find_by(short_url: params[:id]) | ||
url.click_count += 1 | ||
url.save | ||
redirect_to url.real_url | ||
end | ||
|
||
# GET /urls/new | ||
def new | ||
redirect_to login_path and return if !logged_in? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line is repeated a few times. Check out There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good tip. Read about this over the weekend but had not applied it until now. Thanks! |
||
@url = Url.new | ||
end | ||
|
||
# POST /urls | ||
# POST /urls.json | ||
def create | ||
redirect_to login_path and return if !logged_in? | ||
@url = Url.new(url_params) | ||
|
||
respond_to do |format| | ||
if @url.save | ||
@urls = Url.all | ||
format.html { render :index, notice: 'Url was successfully created.' } | ||
format.json { render :show, status: :created, location: @url } | ||
else | ||
format.html { render :new } | ||
format.json { render json: @url.errors, status: :unprocessable_entity } | ||
end | ||
end | ||
end | ||
|
||
private | ||
# Never trust parameters from the scary internet, only allow the white list through. | ||
def url_params | ||
params.require(:url).permit(:short_url, :real_url) | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,80 @@ | ||
class UsersController < ApplicationController | ||
before_action :set_user, only: [:show, :edit, :update, :destroy] | ||
|
||
# GET /users | ||
# GET /users.json | ||
def index | ||
redirect_to login_path and return if !logged_in? | ||
@users = User.all | ||
end | ||
|
||
# GET /users/1 | ||
# GET /users/1.json | ||
def show | ||
redirect_to login_path and return if !logged_in? | ||
end | ||
|
||
# GET /users/new | ||
def new | ||
@user = User.new | ||
end | ||
|
||
# GET /users/1/edit | ||
def edit | ||
redirect_to login_path and return if !logged_in? | ||
end | ||
|
||
# POST /users | ||
# POST /users.json | ||
def create | ||
@user = User.new(user_params) | ||
|
||
respond_to do |format| | ||
if @user.save | ||
log_in @user | ||
format.html { redirect_to @user, notice: 'User was successfully created.' } | ||
format.json { render :show, status: :created, location: @user } | ||
else | ||
format.html { render :new } | ||
format.json { render json: @user.errors, status: :unprocessable_entity } | ||
end | ||
end | ||
end | ||
|
||
# PATCH/PUT /users/1 | ||
# PATCH/PUT /users/1.json | ||
def update | ||
redirect_to login_path and return if !logged_in? | ||
respond_to do |format| | ||
if @user.update(user_params) | ||
format.html { redirect_to @user, notice: 'User was successfully updated.' } | ||
format.json { render :show, status: :ok, location: @user } | ||
else | ||
format.html { render :edit } | ||
format.json { render json: @user.errors, status: :unprocessable_entity } | ||
end | ||
end | ||
end | ||
|
||
# DELETE /users/1 | ||
# DELETE /users/1.json | ||
def destroy | ||
redirect_to login_path and return if !logged_in? | ||
@user.destroy | ||
respond_to do |format| | ||
format.html { redirect_to users_url, notice: 'User was successfully destroyed.' } | ||
format.json { head :no_content } | ||
end | ||
end | ||
|
||
private | ||
# Use callbacks to share common setup or constraints between actions. | ||
def set_user | ||
@user = User.find(params[:id]) | ||
end | ||
|
||
# Never trust parameters from the scary internet, only allow the white list through. | ||
def user_params | ||
params.require(:user).permit(:name, :email, :password, :password_confirmation) | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,18 @@ | ||
module SessionsHelper | ||
def log_in(user) | ||
session[:user_id] = user.id | ||
end | ||
|
||
def log_out | ||
session.delete(:user_id) | ||
@current_user = nil | ||
end | ||
|
||
def current_user | ||
@current_user ||= User.find_by(id: session[:user_id]) | ||
end | ||
|
||
def logged_in? | ||
!current_user.nil? | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
require 'uri' | ||
require 'open-uri' | ||
|
||
class Url < ActiveRecord::Base | ||
before_save :make_short_url | ||
validates :real_url, presence: true | ||
validate :test_url | ||
|
||
private | ||
def make_short_url | ||
self.short_url = SecureRandom.hex(4) if self.short_url.nil? || self.short_url.empty? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ActiveSupport adds a |
||
end | ||
|
||
def test_url | ||
# validate url is http or https | ||
uri = URI(self.real_url) | ||
errors.add(:real_url, 'is not http or https') unless uri.scheme == 'http' || uri.scheme == 'https' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer using the accepted answer to this question: http://stackoverflow.com/questions/1805761/check-if-url-is-valid-ruby (under "Edit 2"). |
||
|
||
# validate url is accessible | ||
begin | ||
result = open(uri.to_s) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
rescue | ||
errors.add(:real_url, 'is not accessible') | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
class User < ActiveRecord::Base | ||
before_save { self.email = email.downcase } | ||
validates :name, presence: true, length: { maximum: 50 } | ||
VALID_EMAIL_REGEX = /\A[\w+\-.]+@[a-z\d\-.]+\.[a-z]+\z/i | ||
validates :email, presence: true, length: { maximum: 255 }, | ||
format: { with: VALID_EMAIL_REGEX }, | ||
uniqueness: { case_sensitive: false } | ||
has_secure_password | ||
validates :password, presence: true, length: { minimum: 6 } | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
<% provide(:title, "Log in") %> | ||
<h1>Log in</h1> | ||
|
||
<div class="row"> | ||
<div class="col-md-6 col-md-offset-3"> | ||
<%= form_for(:session, url: login_path) do |f| %> | ||
|
||
<%= f.label :email %> | ||
<%= f.email_field :email, class: 'form-control' %> | ||
|
||
<%= f.label :password %> | ||
<%= f.password_field :password, class: 'form-control' %> | ||
|
||
<%= f.submit "Log in", class: "btn btn-primary" %> | ||
<% end %> | ||
|
||
<p>New user? <%= link_to "Sign up now!", signup_path %></p> | ||
</div> | ||
</div> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
<%= form_for(@url) do |f| %> | ||
<% if @url.errors.any? %> | ||
<div id="error_explanation"> | ||
<h2><%= pluralize(@url.errors.count, "error") %> prohibited this url from being saved:</h2> | ||
|
||
<ul> | ||
<% @url.errors.full_messages.each do |message| %> | ||
<li><%= message %></li> | ||
<% end %> | ||
</ul> | ||
</div> | ||
<% end %> | ||
|
||
<div class="field"> | ||
<%= f.label :real_url %><br> | ||
<%= f.text_field :real_url %> | ||
</div> | ||
<div class="actions"> | ||
<%= f.submit %> | ||
</div> | ||
<% end %> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
<h1>Editing url</h1> | ||
|
||
<%= render 'form' %> | ||
|
||
<%= link_to 'Show', @url %> | | ||
<%= link_to 'Back', urls_path %> |
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.
find_by
returnsnil
when a record isn't found. Since the lines below this depend onurl
being non-nil, switch this tofind_by!
.find_by!
raisesActiveRecord::RecordNotFound
instead. Rails automatically responds with a 404 whenActiveRecord::RecordNotFound
is raised.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'm having trouble getting this to work. On my working branch it's throwing ActiveRecord::RecordNotFound, but no 404 seems to be happening - or at least my test is failing due to the exception. Some env setting I'm missing?
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.
Sorry I wasn't clear. Rails rescues
ActiveRecord::RecordNotFound
and responds with 404 in production, not development or test.