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

Add user token to article links #158

Merged
merged 5 commits into from Aug 23, 2013
Merged
Diff settings

Always

Just for now

@@ -1,5 +1,6 @@
class ApplicationController < ActionController::Base
include CacheCooker::Oven
include ApplicationHelper

protect_from_forgery

@@ -1,7 +1,8 @@
class ArticlesController < ApplicationController
before_filter :find_article, :only => [:show, :edit, :update, :share]
before_filter :redirect_to_slug, :only => [:show]
before_filter :create_visit, :only => [:show]
before_filter :update_url, :only => [:show]


skip_before_filter :authenticate, :only => [:shared, :samples]
skip_before_filter :authenticate_user, :only => [:shared, :samples]
@@ -86,6 +87,15 @@ def find_article
render_http_error(404) unless @article
end

# NOTE: This method uses our custom override of article_path in ApplicationHelper

def update_url
slug_needs_updating = @article.slug.present? && params[:id] != @article.slug
missing_token = params[:u].blank?

redirect_to(article_path(@article)) if slug_needs_updating || missing_token
end

def decorate_article
@article = ArticleDecorator.decorate(@article)
end
@@ -104,10 +114,4 @@ def create_visit
article_visit.create
end
end

def redirect_to_slug
return unless @article.slug.present?

redirect_to article_path(@article.slug) unless params[:id] == @article.slug
end
end
@@ -1,4 +1,16 @@
module ApplicationHelper
def article_url(article, params={})
return super unless current_user

ArticleLink.new(article, params).url(current_user.share_token)
end

def article_path(article, params={})
return super unless current_user

ArticleLink.new(article, params).path(current_user.share_token)
end

def home_path
if current_user
library_path
Copy path View file
@@ -0,0 +1,24 @@
class ArticleLink
include Rails.application.routes.url_helpers

def initialize(article, params)
self.article = article
self.params = params
end

def path(token)
article_path(article, params_with_token(token))
end

def url(token)
article_url(article, params_with_token(token))
end

private

attr_accessor :params, :article

def params_with_token(token)
{:u => token}.merge(params)
end
end
Copy path View file
@@ -23,13 +23,16 @@ class User < ActiveRecord::Base

attr_protected :admin, :status


before_save do
if changed.include?("contact_email")
write_attribute(:contact_email, contact_email.strip.downcase)
end
end

before_create do
write_attribute(:share_token, SecureRandom.hex(5))
end

def self.to_notify
where(notifications_enabled: true, :status => ACTIVE_STATUSES)
end
@@ -1,9 +1,9 @@
You can see what has been said and share your own thoughts via the link below:

<%= article_url(@article, :anchor => "comments") %>
<%= ArticleLink.new(@article, :anchor => "comments").url("placeholder_token") %>

----

You can customize your notification settings via the link below:

<%= profile_settings_url %>
<%= profile_settings_url %>
@@ -1,9 +1,9 @@
You can see what was said and share your own thoughts via the link below:

<%= article_url(@article, :anchor => "comments") %>
<%= ArticleLink.new(@article, :anchor => "comments").url("placeholder_token") %>

----

You can customize your notification settings via the link below:

<%= profile_settings_url %>
<%= profile_settings_url %>
@@ -1,9 +1,9 @@
You can see what people are saying and share your own thoughts via the link below:

<%= article_url(@article, :anchor => "comments") %>
<%= ArticleLink.new(@article, :anchor => "comments").url("placeholder_token") %>

----

You can customize your notification settings via the link below:

<%= profile_settings_url %>
<%= profile_settings_url %>
Copy path View file
@@ -11,7 +11,7 @@ class Application < Rails::Application
if Rails.env == "production"
config.middleware.use("Rack::GoogleAnalytics", :web_property_id => "UA-33127211-2")
end


# Enable the asset pipeline
config.assets.enabled = true
@@ -61,7 +61,12 @@ class Application < Rails::Application

# Action Mailer Defaults
config.action_mailer.delivery_method = :mailhopper
config.action_mailer.default_url_options = { :host => "practicingruby.com" }

url_options = { :host => "practicingruby.com" }

config.action_mailer.default_url_options = url_options
Rails.application.routes.default_url_options = url_options

ActionMailer::Base.default :from => "Practicing Ruby <gregory@practicingruby.com>"

config.generators do |g|
@@ -0,0 +1,7 @@
class AddShareTokenToUser < ActiveRecord::Migration
def change
change_table :users do |t|
t.string :share_token
end
end
end
Copy path View file
@@ -11,7 +11,8 @@
#
# It's strongly recommended to check this file into your version control system.

ActiveRecord::Schema.define(:version => 20130816174433) do
ActiveRecord::Schema.define(:version => 20130820143254) do

create_table "announcements", :force => true do |t|
t.text "title"
t.text "body"
@@ -174,6 +175,7 @@
t.boolean "notify_updates", :default => true, :null => false
t.text "payment_provider"
t.text "payment_provider_id"
t.string "share_token"
end

create_table "volumes", :force => true do |t|
@@ -184,4 +186,5 @@
t.datetime "created_at"
t.datetime "updated_at"
end

end
@@ -4,12 +4,18 @@ class ArticleRoutingTest < ActionDispatch::IntegrationTest
setup do
@article = FactoryGirl.create(:article, :slug => "awesome-article")
simulated_user { register Support::SimulatedUser.default }

# FIXME: Another simulated user oddity
@user = User.first
end

test "by slug" do
visit "/articles/awesome-article"

assert_content @article.subject

assert_current_path "/articles/awesome-article"
assert_url_has_param "u", @user.share_token
end

test "by id" do
@@ -19,6 +25,7 @@ class ArticleRoutingTest < ActionDispatch::IntegrationTest
assert_content @article.subject

assert_current_path "/articles/awesome-article"
assert_url_has_param "u", @user.share_token
end

test "with invalid slug" do
Copy path View file
@@ -70,6 +70,13 @@ def sign_out
click_link 'Log out'
end

# FIXME: THIS IS ALMOST CERTAINLY A HACK.
def assert_url_has_param(key, value)
params = Rack::Utils.parse_query(URI.parse(current_url).query)

assert_equal params[key], value
end

def within(scope, prefix=nil)
scope = '#' << ActionController::RecordIdentifier.dom_id(scope, prefix) if scope.is_a?(ActiveRecord::Base)
super(scope)
@@ -1,4 +1,4 @@
require 'test_helper'
require_relative '../../test_helper'

class ConversationMailerTest < ActionMailer::TestCase
context "conversation started" do
@@ -21,10 +21,8 @@ class ConversationMailerTest < ActionMailer::TestCase

email_bodies = ActionMailer::Base.deliveries.map {|e| e.body.to_s }

article_url = Rails.application.routes.url_helpers.
article_url( first_comment.commentable,
:anchor => "comments",
:only_path => true )
article_url = ArticleLink.new(first_comment.commentable, :anchor => "comments")
.url("placeholder_token")

email_bodies.each do |body|
assert body[article_url]
@@ -69,7 +67,7 @@ class ConversationMailerTest < ActionMailer::TestCase

messages = ActionMailer::Base.deliveries

assert_equal 0, messages.count { |msg|
assert_equal 0, messages.count { |msg|
msg.bcc.include?(dont_notify_user.contact_email) }

assert_equal 1, messages.count { |msg|
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.