Skip to content

#372 Add Slug to Protip URLs for SEO #222

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

Merged
merged 1 commit into from
Nov 3, 2014
Merged
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
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ gem 'acts_as_follower', '0.1.1'
gem 'color'
gem 'createsend'
gem 'fog'
gem 'friendly_id', '4.0.10.1'
gem 'geocoder'
gem 'hashie'
gem 'linkedin'
Expand Down
3 changes: 3 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,8 @@ GEM
dotenv (~> 0.11.1)
thor (~> 0.19.1)
formatador (0.2.5)
friendly_id (4.0.10.1)
activerecord (>= 3.0, < 4.0)
fssm (0.2.10)
fukuzatsu (0.9.16)
ephemeral
Expand Down Expand Up @@ -739,6 +741,7 @@ DEPENDENCIES
flog
fog
foreman
friendly_id (= 4.0.10.1)
fukuzatsu
fuubar (= 2.0.0.rc1)
geocoder
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,11 @@ window.ProtipRouter = Backbone.Router.extend(
'*path': 'closeProtip'

fetchProtip: (id)->
if(id.match(/^[\dA-Z\-_]{6}$/i))
$.ajax '/p/' + id,
type: 'GET'
data:
mode: 'popup'
dataType: 'script'
else
@.closeProtip()

closeProtip: ->
$('#x-active-preview-pane').remove()
Expand Down
2 changes: 2 additions & 0 deletions app/controllers/protips_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,8 @@ def show
end

return redirect_to protip_missing_destination, notice: "The pro tip you were looking for no longer exists" if @protip.nil?
return redirect_to protip_path(@protip.public_id<<'/'<<@protip.friendly_id, :p => params[:p], :q => params[:q]) if params[:slug]!=@protip.friendly_id

@comments = @protip.comments
@reply_to = show_params[:reply_to]
@next_protip = Protip.search_next(show_params[:q], show_params[:t], show_params[:i], show_params[:p]) if is_admin?
Expand Down
11 changes: 9 additions & 2 deletions app/models/protip.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
require 'search'

class Protip < ActiveRecord::Base
extend FriendlyId
friendly_id :slug_format, :use => :slugged

include Featurable
# TODO: Break out the various responsibilities on the Protip into modules/concerns.

Expand Down Expand Up @@ -951,7 +954,11 @@ def matching_jobs
def to_html
CFM::Markdown.render self.body
end


def slug_format
"#{title}"
end

protected
def check_links
errors[:body] << "one or more of the links are invalid or not publicly reachable/require login" unless valid_links?
Expand All @@ -969,7 +976,7 @@ def adjust_like_value(user, like_value)
def analyze_spam
AnalyzeSpamJob.perform_async({ id: id, klass: self.class.name })
end

end

# == Schema Information
Expand Down
3 changes: 2 additions & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,9 @@
get '/jobs(/:location(/:skill))' => 'opportunities#index', as: :jobs
get '/jobs-map' => 'opportunities#map', as: :jobs_map

resources :protips, :path => '/p', :constraints => {id: /[\dA-Z\-_]{6}/i} do
resources :protips, :path => '/p' do
collection do
get ':id/:slug' => 'protips#show', as: :slug, :constraints => {:slug => /(?!.*?edit).*/}
get 'random'
get 'search' => 'protips#search', as: :search
post 'search' => 'protips#search'
Expand Down
6 changes: 6 additions & 0 deletions db/migrate/20141015182230_add_slug_to_protips.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class AddSlugToProtips < ActiveRecord::Migration
def change
add_column :protips, :slug, :string
add_index :protips, :slug
end
end
4 changes: 3 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
#
# It's strongly recommended to check this file into your version control system.

ActiveRecord::Schema.define(:version => 20140807214719) do
ActiveRecord::Schema.define(:version => 20141015182230) do

add_extension "citext"

Expand Down Expand Up @@ -254,9 +254,11 @@
t.float "boost_factor", :default => 1.0
t.integer "inappropriate", :default => 0
t.integer "likes_count", :default => 0
t.string "slug"
end

add_index "protips", ["public_id"], :name => "index_protips_on_public_id"
add_index "protips", ["slug"], :name => "index_protips_on_slug"
add_index "protips", ["user_id"], :name => "index_protips_on_user_id"

create_table "purchased_bundles", :force => true do |t|
Expand Down
10 changes: 10 additions & 0 deletions lib/tasks/generate_protip_slugs.rake
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
desc 'Generate slugs for existing protips'
task :generate_protip_slugs => :environment do
begin
Protip.all.each do |pt|
pt.save
end
rescue => e
puts "Rake task protip slugs failed: #{e}"
end
end
14 changes: 12 additions & 2 deletions spec/controllers/protips_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,20 @@ def valid_session
# end
# end

describe "GET show" do
it "assigns the requested protip as @protip" do
describe "GET show using public_id" do
it "redirects to GET show using slug" do
protip = Protip.create! valid_attributes
protip.save
get :show, {id: protip.to_param}, valid_session
expect(response).to redirect_to slug_protips_path(protip,protip.friendly_id)
end
end

describe "GET show using slug" do
it "assigns the requested protip as @protip" do
protip = Protip.create! valid_attributes
protip.save
get :show, {id: protip.public_id, slug: protip.friendly_id}, valid_session
expect(assigns(:protip)).to eq(protip)
end
end
Expand Down