Permalink
Browse files

Huge refactoring, moving to ownership model for gem permissions

  • Loading branch information...
1 parent 744846c commit 42ebc5223621417dc12736a4339be8720f9c0ac7 @qrush qrush committed Jun 7, 2009
@@ -14,7 +14,7 @@ def search
end
def mine
- @gems = current_user.rubygems.by_name(:asc)
+ @gems = current_user.rubygems
end
def index
@@ -58,14 +58,14 @@ def create
rubygem = Rubygem.find_or_initialize_by_name(spec.name)
- if !rubygem.new_record? && rubygem.user != current_user
+ if !rubygem.new_record? && !rubygem.owned_by?(current_user)
render :text => "You do not have permission to push to this gem.", :status => 403
return
end
rubygem.spec = spec
rubygem.path = temp.path
- rubygem.user = current_user
+ rubygem.ownerships.build(:user => current_user, :approved => true)
rubygem.save
render :text => "Successfully registered new gem: #{rubygem}"
end
@@ -83,7 +83,7 @@ def authenticate
def load_gem
@gem = Rubygem.find(params[:id])
- if @gem.user != current_user
+ if !@gem.owned_by?(current_user)
flash[:warning] = "You do not have permission to edit this gem."
redirect_to root_url
end
View
@@ -0,0 +1,4 @@
+class Ownership < ActiveRecord::Base
+ belongs_to :rubygem
+ belongs_to :user
+end
View
@@ -3,6 +3,8 @@ class Rubygem < ActiveRecord::Base
sluggable_finder :name
belongs_to :user
+ has_many :owners, :through => :ownerships, :source => :user
+ has_many :ownerships
has_many :versions, :dependent => :destroy
has_one :linkset, :dependent => :destroy
@@ -26,6 +28,10 @@ def self.pull_spec(path)
end
end
+ def owned_by?(user)
+ ownerships.find_by_user_id(user.id).try(:approved) if user
+ end
+
def to_s
"#{name} (#{current_version})"
end
View
@@ -1,7 +1,8 @@
class User < ActiveRecord::Base
include Clearance::User
- has_many :rubygems
+ has_many :rubygems, :through => :ownerships
+ has_many :ownerships
before_create :generate_api_key
protected
@@ -17,7 +17,7 @@
%span.big= version.number
%span.small= version.created_at.to_date.to_formatted_s(:long)
.grid_4
- -if current_user == @gem.user
+ -if @gem.owned_by?(current_user)
.block.red
%h3 Administration
%p
@@ -31,10 +31,7 @@
:lib => 'shoulda',
:source => 'http://gems.github.com',
:version => '>= 2.10.1'
-config.gem 'thoughtbot-factory_girl',
- :lib => 'factory_girl',
- :source => 'http://gems.github.com',
- :version => '>= 1.2.1'
+config.gem 'factory_girl'
config.gem 'webrat',
:version => '>= 0.4.4'
config.gem 'cucumber',
@@ -0,0 +1,20 @@
+class CreateOwnerships < ActiveRecord::Migration
+ def self.up
+ create_table :ownerships do |table|
+ table.belongs_to :rubygem
+ table.belongs_to :user
+ table.string :token
+ table.boolean :approved, :default => false
+ table.timestamps
+ end
+
+ add_index :ownerships, :rubygem_id
+ add_index :ownerships, :user_id
+ end
+
+ def self.down
+ remove_index :ownerships, :rubygem_id
+ remove_index :ownerships, :user_id
+ drop_table :ownerships
+ end
+end
@@ -0,0 +1,9 @@
+class RemoveUserIdFromRubygems < ActiveRecord::Migration
+ def self.up
+ remove_column :rubygems, :user_id
+ end
+
+ def self.down
+ add_column :rubygems, :user_id, :integer
+ end
+end
View
@@ -9,7 +9,7 @@
#
# It's strongly recommended to check this file into your version control system.
-ActiveRecord::Schema.define(:version => 20090603212455) do
+ActiveRecord::Schema.define(:version => 20090607011852) do
create_table "dependencies", :force => true do |t|
t.string "name"
@@ -31,10 +31,21 @@
t.datetime "updated_at"
end
+ create_table "ownerships", :force => true do |t|
+ t.integer "rubygem_id"
+ t.integer "user_id"
+ t.string "token"
+ t.boolean "approved", :default => false
+ t.datetime "created_at"
+ t.datetime "updated_at"
+ end
+
+ add_index "ownerships", ["rubygem_id"], :name => "index_ownerships_on_rubygem_id"
+ add_index "ownerships", ["user_id"], :name => "index_ownerships_on_user_id"
+
create_table "rubygems", :force => true do |t|
t.string "name"
t.string "token"
- t.integer "user_id"
t.datetime "created_at"
t.datetime "updated_at"
t.integer "downloads", :default => 0
@@ -0,0 +1,4 @@
+Factory.define :ownership do |ownership|
+ ownership.association(:rubygem)
+ ownership.association(:user)
+end
@@ -3,7 +3,6 @@
Factory.define :rubygem do |rubygem|
rubygem.name { Factory.next(:name) }
rubygem.token { 'asdf' }
- rubygem.association :user
rubygem.spec { gem_spec }
rubygem.path { gem_file.path }
end
@@ -2,6 +2,11 @@
class RubygemsControllerTest < ActionController::TestCase
+ def create_gem(owner)
+ @gem = Factory(:rubygem)
+ @gem.ownerships << Factory(:ownership, :user => owner, :rubygem => @gem, :approved => true)
+ end
+
context "When logged in" do
setup do
@user = Factory(:email_confirmed_user)
@@ -23,7 +28,11 @@ class RubygemsControllerTest < ActionController::TestCase
context "On GET to mine" do
setup do
3.times { Factory(:rubygem) }
- @gems = (1..3).map { Factory(:rubygem, :user => @user) }
+ @gems = (1..3).map do
+ rubygem = Factory(:rubygem)
+ rubygem.ownerships << Factory(:ownership, :user => @user, :rubygem => rubygem)
+ rubygem
+ end
get :mine
end
@@ -55,7 +64,7 @@ class RubygemsControllerTest < ActionController::TestCase
context "On GET to show for this user's gem" do
setup do
- @gem = Factory(:rubygem, :user => @user)
+ create_gem(@user)
get :show, :id => @gem.to_param
end
@@ -70,7 +79,7 @@ class RubygemsControllerTest < ActionController::TestCase
context "On GET to edit for this user's gem" do
setup do
- @gem = Factory(:rubygem, :user => @user)
+ create_gem(@user)
get :edit, :id => @gem.to_param
end
@@ -91,7 +100,7 @@ class RubygemsControllerTest < ActionController::TestCase
context "On GET to edit for another user's gem" do
setup do
@other_user = Factory(:email_confirmed_user)
- @gem = Factory(:rubygem, :user => @other_user)
+ create_gem(@other_user)
get :edit, :id => @gem.to_param
end
should_respond_with :redirect
@@ -102,8 +111,8 @@ class RubygemsControllerTest < ActionController::TestCase
context "On PUT to update for this user's gem that is successful" do
setup do
- @gem = Factory(:rubygem, :user => @user)
@url = "http://github.com/qrush/gemcutter"
+ create_gem(@user)
put :update, :id => @gem.to_param, :linkset => {:code => @url}
end
should_respond_with :redirect
@@ -117,7 +126,7 @@ class RubygemsControllerTest < ActionController::TestCase
context "On PUT to update for this user's gem that fails" do
setup do
- @gem = Factory(:rubygem, :user => @user)
+ create_gem(@user)
@url = "totally not a url"
put :update, :id => @gem.to_param, :linkset => {:code => @url}
end
@@ -271,21 +280,22 @@ class RubygemsControllerTest < ActionController::TestCase
should_assign_to(:_current_user) { @user }
should_change "Rubygem.count", :by => 1
should "register new gem" do
- assert_equal @user, Rubygem.last.user
+ assert_equal @user, Rubygem.last.ownerships.first.user
assert_equal "Successfully registered new gem: test (0.0.0)", @response.body
end
end
context "On POST to create for existing gem" do
setup do
- @rubygem = Factory(:rubygem, :user => @user, :name => "test")
+ rubygem = Factory(:rubygem, :name => "test")
+ rubygem.ownerships << Factory(:ownership, :user => @user, :rubygem => rubygem, :approved => true)
@request.env["RAW_POST_DATA"] = gem_file("test-1.0.0.gem").read
post :create
end
should_respond_with :success
should_assign_to(:_current_user) { @user }
should "register new version" do
- assert_equal @user, Rubygem.last.user
+ assert_equal @user, Rubygem.last.ownerships.first.user
assert_equal 2, Rubygem.last.versions.size
assert_equal "Successfully registered new gem: test (1.0.0)", @response.body
end
@@ -307,15 +317,18 @@ class RubygemsControllerTest < ActionController::TestCase
context "On POST to create for someone else's gem" do
setup do
@other_user = Factory(:email_confirmed_user)
- @rubygem = Factory(:rubygem, :user => @other_user, :name => "test")
+ create_gem(@other_user)
+ stub(Rubygem).find { @gem }
+
@request.env["RAW_POST_DATA"] = gem_file("test-1.0.0.gem").read
post :create
end
should_respond_with 403
should_assign_to(:_current_user) { @user }
should "not allow new version to be saved" do
- assert_equal @other_user, Rubygem.last.user
- assert_equal 1, Rubygem.last.versions.size
+ other_gem = Rubygem.find(@gem.id)
+ assert_equal @other_user, other_gem.ownerships.first.user
+ assert_equal 1, other_gem.versions.size
assert_equal "You do not have permission to push to this gem.", @response.body
end
end
@@ -0,0 +1,12 @@
+require 'test_helper'
+
+class OwnershipTest < ActiveSupport::TestCase
+ should "be valid with factory" do
+ assert_valid Factory.build(:ownership)
+ end
+ should_belong_to :rubygem
+ should_have_index :rubygem_id
+ should_belong_to :user
+ should_have_index :user_id
+
+end
View
@@ -6,11 +6,43 @@ class RubygemTest < ActiveSupport::TestCase
@rubygem = Factory(:rubygem)
end
- should_belong_to :user
+ should_have_many :owners, :through => :ownerships
+ should_have_many :ownerships
should_have_many :versions, :dependent => :destroy
should_have_one :linkset, :dependent => :destroy
should_validate_uniqueness_of :name
+ context "with a user" do
+ setup do
+ @user = Factory(:user)
+ end
+
+ should "be owned by a user in approved ownership" do
+ ownership = Factory(:ownership, :user => @user, :rubygem => @rubygem, :approved => true)
+ assert @rubygem.owned_by?(@user)
+ end
+
+ should "be not owned by a user in unapproved ownership" do
+ ownership = Factory(:ownership, :user => @user, :rubygem => @rubygem)
+ assert !@rubygem.owned_by?(@user)
+ end
+
+ should "be not owned by a user without ownership" do
+ other_user = Factory(:user)
+ ownership = Factory(:ownership, :user => other_user, :rubygem => @rubygem)
+ assert !@rubygem.owned_by?(@user)
+ end
+
+ should "be not owned if no ownerships" do
+ assert @rubygem.ownerships.empty?
+ assert !@rubygem.owned_by?(@user)
+ end
+
+ should "be not owned if no user" do
+ assert !@rubygem.owned_by?(nil)
+ end
+ end
+
should "create token" do
assert_not_nil @rubygem.token
end
View
@@ -1,7 +1,8 @@
require File.dirname(__FILE__) + '/../test_helper'
class UserTest < ActiveSupport::TestCase
- should_have_many :rubygems
+ should_have_many :rubygems, :through => :ownerships
+ should_have_many :ownerships
should "create api key" do
user = Factory(:user)

0 comments on commit 42ebc52

Please sign in to comment.