Skip to content

Commit ca6bac2

Browse files
committed
add pretty user urls
* 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
1 parent 2106e7b commit ca6bac2

File tree

10 files changed

+109
-49
lines changed

10 files changed

+109
-49
lines changed

app/controllers/projects_controller.rb

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,8 @@ def show
5151
@project.update_attribute :bitcoin_address, bitcoin_address
5252
end
5353
end
54-
@project_tips = @project.tips
55-
@recent_tips = @project_tips.includes(:user).order(created_at: :desc).first(5)
54+
@project_tips = @project.tips.with_address
55+
@recent_tips = @project_tips.with_address.order(created_at: :desc).first(5)
5656
end
5757

5858
def edit
@@ -94,19 +94,6 @@ def decide_tip_amounts
9494
end
9595
end
9696

97-
def tips
98-
@tips = @project.tips.includes(:user).order(created_at: :desc).
99-
page(params[:page]).
100-
per(params[:per_page] || 30)
101-
render :template => 'tips/index'
102-
end
103-
104-
def deposits
105-
@deposits = @project.deposits.order(created_at: :desc).
106-
page(params[:page]).
107-
per(params[:per_page] || 30)
108-
render :template => 'deposits/index'
109-
end
11097

11198
private
11299

app/controllers/tips_controller.rb

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,17 @@ class TipsController < ApplicationController
33
before_action :load_project
44

55
def index
6-
if params[:project_id]
7-
@tips = @project.tips.includes(:user)
6+
if @project
7+
@tips = @project.tips.includes(:user).with_address
88
elsif params[:user_id] && @user = User.find(params[:user_id])
9+
if @user.nil? || @user.bitcoin_address.blank?
10+
flash[:error] = I18n.t('errors.user_not_found')
11+
redirect_to users_path and return
12+
end
13+
914
@tips = @user.tips.includes(:project)
1015
else
11-
@tips = Tip.includes(:user, :project)
16+
@tips = Tip.with_address.includes(:project)
1217
end
1318
@tips = @tips.order(created_at: :desc).
1419
page(params[:page]).
@@ -19,9 +24,15 @@ def index
1924
end
2025
end
2126

27+
2228
private
2329

2430
def load_project
25-
super(params[:project_id]) if params[:project_id].present?
31+
if params[:project_id].present?
32+
super params[:project_id]
33+
elsif params[:service].present? && params[:repo].present?
34+
super Project.where(host: params[:service]).
35+
where('lower(`full_name`) = ?' , params[:repo].downcase).first
36+
end
2637
end
2738
end

app/controllers/users_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ def show
88
end
99

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

1414
def update

app/views/tips/index.html.haml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,6 @@
3737
= t('.refunded')
3838
- elsif tip.undecided?
3939
= t('.undecided')
40-
- elsif tip.user.bitcoin_address.blank?
41-
= t('.no_bitcoin_address')
4240
- elsif tip.user.balance < CONFIG["min_payout"]
4341
= t('.below_threshold')
4442
- else

config/routes.rb

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,15 @@
44

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

7+
# reserved routes (rejected pertty url usernames)
8+
RESERVED_USER_ROUTES_REGEX = /(^(?:(?!^\d+$|\b(sign_in|cancel|sign_up|edit|confirmation|login)\b).)*$)/
9+
get '/users/:name/tips' => 'tips#index', :constraints => {:name => /\RESERVED_USER_ROUTES_REGEX/}
10+
get '/users/:name' => 'users#show', :constraints => {:name => /\RESERVED_USER_ROUTES_REGEX/}
11+
712
get '/:service/:repo/edit' => 'projects#edit', :constraints => {:service => /github/, :repo => /.+/}
813
get '/:service/:repo/decide_tip_amounts' => 'projects#decide_tip_amounts', :constraints => {:service => /github/, :repo => /.+/}
9-
get '/:service/:repo/tips' => 'projects#tips', :constraints => {:service => /github/, :repo => /.+/}
10-
get '/:service/:repo/deposits' => 'projects#deposits', :constraints => {:service => /github/, :repo => /.+/}
14+
get '/:service/:repo/tips' => 'tips#index', :constraints => {:service => /github/, :repo => /.+/}
15+
get '/:service/:repo/deposits' => 'deposits#index', :constraints => {:service => /github/, :repo => /.+/}
1116
get '/:service/:repo' => 'projects#show', :constraints => {:service => /github/, :repo => /.+/}
1217

1318
devise_for :users,
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
2+
def create_user nickname , has_bitcoiin_address
3+
User.create do |user|
4+
user.name = nickname
5+
user.email = "#{nickname}@example.com"
6+
user.bitcoin_address = '1AFgARu7e5d8Lox6P2DSFX3MW8BtsVXEn5' if has_bitcoiin_address
7+
user.nickname = nickname
8+
user.password = Devise.friendly_token.first(Devise.password_length.min)
9+
user.skip_confirmation!
10+
end
11+
end
12+
13+
Given /^a user named "(.*?)" exists (with|without?) a bitcoin address$/ do |nickname , with|
14+
(@users ||= {})[nickname] = (create_user nickname , (with.eql? 'with'))
15+
end

features/step_definitions/web.rb

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,21 +29,24 @@ def parse_path_from_page_string page_string
2929
path = nil
3030

3131
# explicit cases
32+
# e.g. "a-user/a-project github project edit"
33+
# e.g. "a-user user edit"
3234
tokens = page_string.split ' '
3335
name = tokens[0]
3436
model = tokens[1]
3537
action = tokens[2] || ''
3638
is_user = model.eql? 'user'
3739
is_project = ['github-project' , 'bitbucket-project'].include? model
38-
if is_project # e.g. "me/my-project github project edit"
40+
if is_project
3941
projects_paths = ['' , 'edit' , 'decide_tip_amounts' , 'tips' , 'deposits'] # '' => 'show'
4042
is_valid_path = projects_paths.include? action
4143
service = model.split('-').first
4244
path = "/#{service}/#{name}/#{action}" if is_valid_path
4345
elsif is_user
44-
projects_paths = ['show' , 'edit']
45-
is_valid_path = projects_paths.include? action
46-
path = "/users/#{name}/#{action}" if is_valid_path
46+
user_paths = ['' , 'edit' , 'tips']
47+
is_valid_path = user_paths.include? action
48+
# path = "/users/#{name}/#{action}" if is_valid_path # TODO: nyi
49+
path = "/users/#{@users[name].id}/#{action}" if is_valid_path
4750

4851
# implicit cases
4952
else case page_string

features/view_tips.feature

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
Feature: Visitors should be able to see claimed tips
2+
Background:
3+
Given a "github" project named "seldon/seldons-project" exists
4+
And a user named "yugo" exists with a bitcoin address
5+
And a user named "gaal" exists without a bitcoin address
6+
And our fee is "0"
7+
And a deposit of "500" is made
8+
And the most recent commit is "AAA"
9+
And a new commit "BBB" with parent "AAA"
10+
And a new commit "CCC" with parent "BBB"
11+
And the author of commit "BBB" is "yugo"
12+
And the author of commit "CCC" is "gaal"
13+
When the project syncs with the remote repo
14+
Then there should be a tip of "5" for commit "BBB"
15+
And there should be a tip of "4.95" for commit "CCC"
16+
Given I'm not logged in
17+
18+
Scenario: Visitors should see all claimed tips but not unclaimed tips
19+
When I visit the "tips" page
20+
Then I should be on the "tips" page
21+
And I should see "yugo"
22+
But I should not see "gaal"
23+
24+
Scenario: Visitors should see all claimed tips per project but not unclaimed tips
25+
When I visit the "seldon/seldons-project github-project" page
26+
Then I should be on the "seldon/seldons-project github-project" page
27+
And I should see "yugo"
28+
But I should not see "gaal"
29+
When I visit the "seldon/seldons-project github-project tips" page
30+
Then I should be on the "seldon/seldons-project github-project tips" page
31+
And I should see "yugo"
32+
But I should not see "gaal"
33+
34+
Scenario: Visitors should see all claimed tips per user but not unclaimed tips
35+
When I visit the "yugo user tips" page
36+
Then I should be on the "yugo user tips" page
37+
And I should see "yugo"
38+
When I visit the "gaal user tips" page
39+
Then I should be on the "users" page
40+
And I should see "yugo"
41+
And I should not see "gaal"
42+
But I should see "User not found"

spec/controllers/projects_controller_spec.rb

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -82,20 +82,6 @@
8282
end
8383
end
8484

85-
describe 'GET #tips' do
86-
it 'returns 302 status code' do
87-
get :tips , :service => 'github' , :repo => 'test/test'
88-
response.should be_redirect
89-
end
90-
end
91-
92-
describe 'GET #deposits' do
93-
it 'returns 302 status code' do
94-
get :deposits , :service => 'github' , :repo => 'test/test'
95-
response.should be_redirect
96-
end
97-
end
98-
9985
describe "routing" do
10086
it "routes GET /projects to Project#index" do
10187
{ :get => "/projects" }.should route_to(
@@ -188,16 +174,16 @@
188174

189175
it "routes GET /:provider/:repo/tips to Project#tips" do
190176
{ :get => "/github/test/test/tips" }.should route_to(
191-
:controller => "projects" ,
192-
:action => "tips" ,
177+
:controller => "tips" ,
178+
:action => "index" ,
193179
:service => "github" ,
194180
:repo => "test/test")
195181
end
196182

197183
it "routes GET /:provider/:repo/deposits to Project#deposits" do
198184
{ :get => "/github/test/test/deposits" }.should route_to(
199-
:controller => "projects" ,
200-
:action => "deposits" ,
185+
:controller => "deposits" ,
186+
:action => "index" ,
201187
:service => "github" ,
202188
:repo => "test/test")
203189
end

spec/controllers/users_controller_spec.rb

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,20 +92,20 @@
9292
end
9393

9494
describe "routing" do
95-
it "routes GET /users to Project#index" do
95+
it "routes GET /users to User#index" do
9696
{ :get => "/users" }.should route_to(
9797
:controller => "users" ,
9898
:action => "index" )
9999
end
100100

101-
it "routes GET /users to Project#show" do
101+
it "routes GET /users to User#show" do
102102
{ :get => "/users/1" }.should route_to(
103103
:controller => "users" ,
104104
:action => "show" ,
105105
:id => "1" )
106106
end
107107

108-
it "routes GET /login to Project#login" do
108+
it "routes GET /login to User#login" do
109109
{ :get => "/users/login" }.should route_to(
110110
:controller => "users" ,
111111
:action => "login" )
@@ -118,4 +118,17 @@
118118
:user_id => "1" )
119119
end
120120
end
121+
122+
describe "pretty user url routing" do
123+
it "routes regex rejects reserved user paths" do
124+
# accepted pertty url usernames
125+
should_accept = %w{logi ogin s4c2 42x}
126+
# reserved routes (rejected pertty url usernames)
127+
should_reject = %w{sign_in cancel sign_up edit confirmation login}
128+
129+
accepted = should_accept.select {|ea| ea =~ RESERVED_USER_ROUTES_REGEX}
130+
rejected = should_reject.select {|ea| (ea =~ RESERVED_USER_ROUTES_REGEX).nil? }
131+
accepted.size.should eq should_accept.size and rejected.size.should eq should_reject.size
132+
end
133+
end
121134
end

0 commit comments

Comments
 (0)