Skip to content

Commit

Permalink
add pretty user urls
Browse files Browse the repository at this point in the history
* rerouted Project#tips and Project#deposits to Tips#index and Deposits#index
* added TipsController#load_project method
* added pretty user paths for #tips and #show
* added pretty user paths tests
* added	features/step_definitions/users_steps.rb
* added features/view_tips.feature

NOTE:
TipsController#load_project method is nearly identical to ProjectsController#load_project - they could probably be refactored into ApplicationController method
  • Loading branch information
bill-auger committed Nov 2, 2014
1 parent 2106e7b commit ca6bac2
Show file tree
Hide file tree
Showing 10 changed files with 109 additions and 49 deletions.
17 changes: 2 additions & 15 deletions app/controllers/projects_controller.rb
Expand Up @@ -51,8 +51,8 @@ def show
@project.update_attribute :bitcoin_address, bitcoin_address
end
end
@project_tips = @project.tips
@recent_tips = @project_tips.includes(:user).order(created_at: :desc).first(5)
@project_tips = @project.tips.with_address
@recent_tips = @project_tips.with_address.order(created_at: :desc).first(5)
end

def edit
Expand Down Expand Up @@ -94,19 +94,6 @@ def decide_tip_amounts
end
end

def tips
@tips = @project.tips.includes(:user).order(created_at: :desc).
page(params[:page]).
per(params[:per_page] || 30)
render :template => 'tips/index'
end

def deposits
@deposits = @project.deposits.order(created_at: :desc).
page(params[:page]).
per(params[:per_page] || 30)
render :template => 'deposits/index'
end

private

Expand Down
19 changes: 15 additions & 4 deletions app/controllers/tips_controller.rb
Expand Up @@ -3,12 +3,17 @@ class TipsController < ApplicationController
before_action :load_project

def index
if params[:project_id]
@tips = @project.tips.includes(:user)
if @project
@tips = @project.tips.includes(:user).with_address
elsif params[:user_id] && @user = User.find(params[:user_id])
if @user.nil? || @user.bitcoin_address.blank?
flash[:error] = I18n.t('errors.user_not_found')
redirect_to users_path and return
end

@tips = @user.tips.includes(:project)
else
@tips = Tip.includes(:user, :project)
@tips = Tip.with_address.includes(:project)
end
@tips = @tips.order(created_at: :desc).
page(params[:page]).
Expand All @@ -19,9 +24,15 @@ def index
end
end


private

def load_project
super(params[:project_id]) if params[:project_id].present?
if params[:project_id].present?
super params[:project_id]
elsif params[:service].present? && params[:repo].present?
super Project.where(host: params[:service]).
where('lower(`full_name`) = ?' , params[:repo].downcase).first
end
end
end
2 changes: 1 addition & 1 deletion app/controllers/users_controller.rb
Expand Up @@ -8,7 +8,7 @@ def show
end

def index
@users = User.order(withdrawn_amount: :desc, commits_count: :desc).where('commits_count > 0').page(params[:page]).per(30)
@users = User.order(withdrawn_amount: :desc, commits_count: :desc).where('commits_count > 0 AND withdrawn_amount > 0').page(params[:page]).per(30)
end

def update
Expand Down
2 changes: 0 additions & 2 deletions app/views/tips/index.html.haml
Expand Up @@ -37,8 +37,6 @@
= t('.refunded')
- elsif tip.undecided?
= t('.undecided')
- elsif tip.user.bitcoin_address.blank?
= t('.no_bitcoin_address')
- elsif tip.user.balance < CONFIG["min_payout"]
= t('.below_threshold')
- else
Expand Down
9 changes: 7 additions & 2 deletions config/routes.rb
Expand Up @@ -4,10 +4,15 @@

get '/blockchain_info_callback' => "home#blockchain_info_callback", :as => "blockchain_info_callback"

# reserved routes (rejected pertty url usernames)
RESERVED_USER_ROUTES_REGEX = /(^(?:(?!^\d+$|\b(sign_in|cancel|sign_up|edit|confirmation|login)\b).)*$)/
get '/users/:name/tips' => 'tips#index', :constraints => {:name => /\RESERVED_USER_ROUTES_REGEX/}
get '/users/:name' => 'users#show', :constraints => {:name => /\RESERVED_USER_ROUTES_REGEX/}

get '/:service/:repo/edit' => 'projects#edit', :constraints => {:service => /github/, :repo => /.+/}
get '/:service/:repo/decide_tip_amounts' => 'projects#decide_tip_amounts', :constraints => {:service => /github/, :repo => /.+/}
get '/:service/:repo/tips' => 'projects#tips', :constraints => {:service => /github/, :repo => /.+/}
get '/:service/:repo/deposits' => 'projects#deposits', :constraints => {:service => /github/, :repo => /.+/}
get '/:service/:repo/tips' => 'tips#index', :constraints => {:service => /github/, :repo => /.+/}
get '/:service/:repo/deposits' => 'deposits#index', :constraints => {:service => /github/, :repo => /.+/}
get '/:service/:repo' => 'projects#show', :constraints => {:service => /github/, :repo => /.+/}

devise_for :users,
Expand Down
15 changes: 15 additions & 0 deletions features/step_definitions/users_steps.rb
@@ -0,0 +1,15 @@

def create_user nickname , has_bitcoiin_address
User.create do |user|
user.name = nickname
user.email = "#{nickname}@example.com"
user.bitcoin_address = '1AFgARu7e5d8Lox6P2DSFX3MW8BtsVXEn5' if has_bitcoiin_address
user.nickname = nickname
user.password = Devise.friendly_token.first(Devise.password_length.min)
user.skip_confirmation!
end
end

Given /^a user named "(.*?)" exists (with|without?) a bitcoin address$/ do |nickname , with|
(@users ||= {})[nickname] = (create_user nickname , (with.eql? 'with'))
end
11 changes: 7 additions & 4 deletions features/step_definitions/web.rb
Expand Up @@ -29,21 +29,24 @@ def parse_path_from_page_string page_string
path = nil

# explicit cases
# e.g. "a-user/a-project github project edit"
# e.g. "a-user user edit"
tokens = page_string.split ' '
name = tokens[0]
model = tokens[1]
action = tokens[2] || ''
is_user = model.eql? 'user'
is_project = ['github-project' , 'bitbucket-project'].include? model
if is_project # e.g. "me/my-project github project edit"
if is_project
projects_paths = ['' , 'edit' , 'decide_tip_amounts' , 'tips' , 'deposits'] # '' => 'show'
is_valid_path = projects_paths.include? action
service = model.split('-').first
path = "/#{service}/#{name}/#{action}" if is_valid_path
elsif is_user
projects_paths = ['show' , 'edit']
is_valid_path = projects_paths.include? action
path = "/users/#{name}/#{action}" if is_valid_path
user_paths = ['' , 'edit' , 'tips']
is_valid_path = user_paths.include? action
# path = "/users/#{name}/#{action}" if is_valid_path # TODO: nyi
path = "/users/#{@users[name].id}/#{action}" if is_valid_path

# implicit cases
else case page_string
Expand Down
42 changes: 42 additions & 0 deletions features/view_tips.feature
@@ -0,0 +1,42 @@
Feature: Visitors should be able to see claimed tips
Background:
Given a "github" project named "seldon/seldons-project" exists
And a user named "yugo" exists with a bitcoin address
And a user named "gaal" exists without a bitcoin address
And our fee is "0"
And a deposit of "500" is made
And the most recent commit is "AAA"
And a new commit "BBB" with parent "AAA"
And a new commit "CCC" with parent "BBB"
And the author of commit "BBB" is "yugo"
And the author of commit "CCC" is "gaal"
When the project syncs with the remote repo
Then there should be a tip of "5" for commit "BBB"
And there should be a tip of "4.95" for commit "CCC"
Given I'm not logged in

Scenario: Visitors should see all claimed tips but not unclaimed tips
When I visit the "tips" page
Then I should be on the "tips" page
And I should see "yugo"
But I should not see "gaal"

Scenario: Visitors should see all claimed tips per project but not unclaimed tips
When I visit the "seldon/seldons-project github-project" page
Then I should be on the "seldon/seldons-project github-project" page
And I should see "yugo"
But I should not see "gaal"
When I visit the "seldon/seldons-project github-project tips" page
Then I should be on the "seldon/seldons-project github-project tips" page
And I should see "yugo"
But I should not see "gaal"

Scenario: Visitors should see all claimed tips per user but not unclaimed tips
When I visit the "yugo user tips" page
Then I should be on the "yugo user tips" page
And I should see "yugo"
When I visit the "gaal user tips" page
Then I should be on the "users" page
And I should see "yugo"
And I should not see "gaal"
But I should see "User not found"
22 changes: 4 additions & 18 deletions spec/controllers/projects_controller_spec.rb
Expand Up @@ -82,20 +82,6 @@
end
end

describe 'GET #tips' do
it 'returns 302 status code' do
get :tips , :service => 'github' , :repo => 'test/test'
response.should be_redirect
end
end

describe 'GET #deposits' do
it 'returns 302 status code' do
get :deposits , :service => 'github' , :repo => 'test/test'
response.should be_redirect
end
end

describe "routing" do
it "routes GET /projects to Project#index" do
{ :get => "/projects" }.should route_to(
Expand Down Expand Up @@ -188,16 +174,16 @@

it "routes GET /:provider/:repo/tips to Project#tips" do
{ :get => "/github/test/test/tips" }.should route_to(
:controller => "projects" ,
:action => "tips" ,
:controller => "tips" ,
:action => "index" ,
:service => "github" ,
:repo => "test/test")
end

it "routes GET /:provider/:repo/deposits to Project#deposits" do
{ :get => "/github/test/test/deposits" }.should route_to(
:controller => "projects" ,
:action => "deposits" ,
:controller => "deposits" ,
:action => "index" ,
:service => "github" ,
:repo => "test/test")
end
Expand Down
19 changes: 16 additions & 3 deletions spec/controllers/users_controller_spec.rb
Expand Up @@ -92,20 +92,20 @@
end

describe "routing" do
it "routes GET /users to Project#index" do
it "routes GET /users to User#index" do
{ :get => "/users" }.should route_to(
:controller => "users" ,
:action => "index" )
end

it "routes GET /users to Project#show" do
it "routes GET /users to User#show" do
{ :get => "/users/1" }.should route_to(
:controller => "users" ,
:action => "show" ,
:id => "1" )
end

it "routes GET /login to Project#login" do
it "routes GET /login to User#login" do
{ :get => "/users/login" }.should route_to(
:controller => "users" ,
:action => "login" )
Expand All @@ -118,4 +118,17 @@
:user_id => "1" )
end
end

describe "pretty user url routing" do
it "routes regex rejects reserved user paths" do
# accepted pertty url usernames
should_accept = %w{logi ogin s4c2 42x}
# reserved routes (rejected pertty url usernames)
should_reject = %w{sign_in cancel sign_up edit confirmation login}

accepted = should_accept.select {|ea| ea =~ RESERVED_USER_ROUTES_REGEX}
rejected = should_reject.select {|ea| (ea =~ RESERVED_USER_ROUTES_REGEX).nil? }
accepted.size.should eq should_accept.size and rejected.size.should eq should_reject.size
end
end
end

0 comments on commit ca6bac2

Please sign in to comment.