diff --git a/app/models/rubygem.rb b/app/models/rubygem.rb index f239589328e..ea5e4e09e29 100644 --- a/app/models/rubygem.rb +++ b/app/models/rubygem.rb @@ -33,12 +33,16 @@ def latest def validate if name =~ /^[\d]+$/ - errors.add "Name must include at least one letter." - elsif name =~ /[^\d\w_\-\.]/ - errors.add "Name can only include letters, numbers, dashes, and underscores." + errors.add :name, "must include at least one letter" + elsif name =~ /[^\d\w\-\.]/ + errors.add :name, "can only include letters, numbers, dashes, and underscores" end end + def all_errors + (errors.full_messages + linkset.errors.full_messages).join(", ") + end + def self.total_count with_versions.count end diff --git a/features/push.feature b/features/push.feature index 3f481736036..406228e383a 100644 --- a/features/push.feature +++ b/features/push.feature @@ -39,3 +39,10 @@ Feature: Push Gems Then I should see "BGem" And I should see "2.0.0" And I should see "3.0.0" + + Scenario: User pushes gem with bad url + Given I am signed up and confirmed as "email@person.com/password" + And I have an api key for "email@person.com/password" + And I have a gem "CGem" with version "1.0.0" and homepage "badurl.com" + When I push the gem "CGem-1.0.0.gem" with my api key + Then I should see "Home does not appear to be a valid URL" diff --git a/features/step_definitions/api_steps.rb b/features/step_definitions/api_steps.rb index 8d1b2aaceef..bb1b524960f 100644 --- a/features/step_definitions/api_steps.rb +++ b/features/step_definitions/api_steps.rb @@ -14,7 +14,6 @@ path = File.join(TEST_DIR, name) visit api_v1_rubygems_path, :post, File.open(path).read - assert_match /Successfully registered/, response.body end When /^I delete the gem "([^\"]*)" with my api key$/ do |arg1| diff --git a/features/step_definitions/gem_steps.rb b/features/step_definitions/gem_steps.rb index a1276f2ae30..65554ba91fd 100644 --- a/features/step_definitions/gem_steps.rb +++ b/features/step_definitions/gem_steps.rb @@ -20,18 +20,27 @@ Factory(:version, :rubygem => rubygem, :number => version_number) end -def build_gem(name, version, summary = "Gemcutter", platform = "ruby") - builder = Gem::Builder.new(build_gemspec(name, version, summary, platform)) +Given /^I have a gem "([^\"]*)" with version "([^\"]*)" and homepage "([^\"]*)"$/ do |name, version, homepage| + gemspec = new_gemspec(name, version, "Gemcutter", "ruby") + gemspec.homepage = homepage + build_gemspec(gemspec) +end + +def build_gemspec(gemspec) + builder = Gem::Builder.new(gemspec) builder.ui = Gem::SilentUI.new builder.build end -def build_gemspec(name, version, summary, platform) - Gem::Specification.new do |s| +def build_gem(name, version, summary = "Gemcutter", platform = "ruby") + build_gemspec(new_gemspec(name, version, summary, platform)) +end + +def new_gemspec(name, version, summary, platform) + gemspec = Gem::Specification.new do |s| s.name = "#{name}" s.platform = "#{platform}" s.version = "#{version}" - s.required_rubygems_version = Gem::Requirement.new(">= 0") if s.respond_to? :required_rubygems_version= s.authors = ["John Doe"] s.date = "#{Time.now.strftime('%Y-%m-%d')}" s.description = "#{summary}" @@ -43,4 +52,10 @@ def build_gemspec(name, version, summary, platform) s.summary = "#{summary}" s.test_files = [] end + + def gemspec.validate + "not validating on purpose" + end + + gemspec end diff --git a/lib/gemcutter.rb b/lib/gemcutter.rb index 353cbfc059e..411bcac5995 100644 --- a/lib/gemcutter.rb +++ b/lib/gemcutter.rb @@ -36,7 +36,7 @@ def save enqueue_web_hook_jobs notify("Successfully registered gem: #{self.version.to_title}", 200) else - notify("There was a problem saving your gem: #{rubygem.errors.full_messages}", 403) + notify("There was a problem saving your gem: #{rubygem.all_errors}", 403) end end diff --git a/test/unit/gemcutter_test.rb b/test/unit/gemcutter_test.rb index 4a1cc2df6c8..e630d36dd35 100644 --- a/test/unit/gemcutter_test.rb +++ b/test/unit/gemcutter_test.rb @@ -193,10 +193,10 @@ class GemcutterTest < ActiveSupport::TestCase @spec = gem_spec(:version => @version) @ownerships = "ownerships" - stub(@rubygem).errors.stub!.full_messages + stub(@rubygem).all_errors stub(@rubygem).save stub(@rubygem).ownerships { @ownerships } - stub(@rubygem).web_hook_jobs{[]} + stub(@rubygem).web_hook_jobs{ [] } stub(@cutter).version { @version } stub(@version).to_title { "latest version" } stub(@version).id { 1337 } @@ -205,7 +205,7 @@ class GemcutterTest < ActiveSupport::TestCase stub(Delayed::Job).enqueue end - # FIXME: These tests all SUCK. + # FIXME: These tests all SUCK!!!! context "saving the rubygem" do before_should "process if successfully saved" do mock(@cutter).update { true } diff --git a/test/unit/rubygem_test.rb b/test/unit/rubygem_test.rb index 0cd59513bf5..2e65aee6f02 100644 --- a/test/unit/rubygem_test.rb +++ b/test/unit/rubygem_test.rb @@ -90,9 +90,21 @@ class RubygemTest < ActiveSupport::TestCase should "not accept #{bad_name} as a name" do @rubygem.name = bad_name assert ! @rubygem.valid? + assert_match /Name/, @rubygem.all_errors end end + should "return linkset errors in #all_errors" do + @specification = gem_specification_from_gem_fixture('test-0.0.0') + @specification.homepage = "badurl.com" + + assert_raise ActiveRecord::RecordInvalid do + @rubygem.update_linkset!(@specification) + end + + assert_match /Home/, @rubygem.all_errors + end + context "with a user" do setup do @user = Factory(:user)