diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 72007840e..93b7ab7c0 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -53,12 +53,6 @@ Lint/ParenthesesAsGroupedExpression: - 'spec/models/member_spec.rb' - 'spec/views/plantings/show.html.haml_spec.rb' -# Offense count: 1 -# Cop supports --auto-correct. -Lint/StringConversionInInterpolation: - Exclude: - - 'spec/views/seeds/index.rss.haml_spec.rb' - # Offense count: 15 # Cop supports --auto-correct. # Configuration parameters: IgnoreEmptyBlocks, AllowUnusedKeywordArguments. diff --git a/.travis.yml b/.travis.yml index 0fc8e1e54..3aefec912 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,21 +11,26 @@ env: global: secure: "Z5TpM2jEX4UCvNePnk/LwltQX48U2u9BRc+Iypr1x9QW2o228QJhPIOH39a8RMUrepGnkQIq9q3ZRUn98RfrJz1yThtlNFL3NmzdQ57gKgjGwfpa0e4Dwj/ZJqV2D84tDGjvdVYLP7zzaYZxQcwk/cgNpzKf/jq97HLNP7CYuf4=" rvm: -- 2.3.1 + - 2.3.1 before_install: - export PATH=$PWD/travis_phantomjs/phantomjs-2.1.1-linux-x86_64/bin:$PATH - - if [ $(phantomjs --version) != '2.1.1' ]; then rm -rf $PWD/travis_phantomjs; mkdir -p $PWD/travis_phantomjs; fi - - if [ $(phantomjs --version) != '2.1.1' ]; then wget https://assets.membergetmember.co/software/phantomjs-2.1.1-linux-x86_64.tar.bz2 -O $PWD/travis_phantomjs/phantomjs-2.1.1-linux-x86_64.tar.bz2; fi - - if [ $(phantomjs --version) != '2.1.1' ]; then tar -xvf $PWD/travis_phantomjs/phantomjs-2.1.1-linux-x86_64.tar.bz2 -C $PWD/travis_phantomjs; fi + - > + if [ $(phantomjs --version) != '2.1.1' ]; then + PHANTOM_URL=https://bitbucket.org/ariya/phantomjs/downloads/phantomjs-2.1.1-linux-x86_64.tar.bz2; + rm -rf $PWD/travis_phantomjs; + mkdir -p $PWD/travis_phantomjs; + wget $PHANTOM_URL -O $PWD/travis_phantomjs/phantomjs-2.1.1-linux-x86_64.tar.bz2; + tar -xvf $PWD/travis_phantomjs/phantomjs-2.1.1-linux-x86_64.tar.bz2 -C $PWD/travis_phantomjs; + fi - phantomjs --version before_script: -- psql -c 'create database growstuff_test;' -U postgres + - bundle exec rake db:create db:migrate db:test:prepare script: -- bundle exec rubocop --display-cop-names --rails -- script/gemfile_check -- bundle exec script/check_contributors_md -- bundle exec rake db:migrate --trace -- bundle exec rspec spec/ + - bundle exec rubocop --display-cop-names --rails + - script/gemfile_check + - bundle exec script/check_contributors_md + - bundle exec rake db:migrate --trace + - bundle exec rspec spec/ services: - elasticsearch before_deploy: @@ -38,6 +43,7 @@ deploy: repo: Growstuff/growstuff app: dev: growstuff-staging + master: growstuff-prod travis_deploy: tranquil-basin-3130 travis_containers: tranquil-basin-3130 run: diff --git a/.yamllint b/.yamllint new file mode 100644 index 000000000..06876caa2 --- /dev/null +++ b/.yamllint @@ -0,0 +1,7 @@ +extends: relaxed + +rules: + # 80 chars should be enough, but don't fail if a line is longer + line-length: + max: 150 + level: warning diff --git a/Gemfile b/Gemfile index 0a445dc62..e752f553a 100644 --- a/Gemfile +++ b/Gemfile @@ -110,14 +110,17 @@ group :development, :test do gem 'capybara' # integration tests gem 'capybara-email' # integration tests for email gem 'capybara-screenshot' # for test debugging - gem 'poltergeist', '~> 1.6' # for headless JS testing + gem 'poltergeist' # for headless JS testing gem 'i18n-tasks' # adds tests for finding missing and unused translations gem 'selenium-webdriver' - gem "codeclimate-test-reporter", group: :test, require: nil gem "active_merchant-paypal-bogus-gateway" gem 'rubocop', require: false end +group :test do + gem 'codeclimate-test-reporter', require: false +end + group :travis do gem 'heroku-api' end diff --git a/Gemfile.lock b/Gemfile.lock index 8f6230ac2..618de9759 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -24,13 +24,13 @@ GEM actionpack active_merchant-paypal-bogus-gateway (0.1.0) activemerchant - active_utils (3.2.2) + active_utils (3.2.3) activesupport (>= 3.2, < 5.1.0) i18n activejob (4.2.7.1) activesupport (= 4.2.7.1) globalid (>= 0.3.0) - activemerchant (1.60.0) + activemerchant (1.61.0) activesupport (>= 3.2.14, < 5.1) builder (>= 2.1.2, < 4.0.0) i18n (>= 0.6.9) @@ -48,7 +48,8 @@ GEM minitest (~> 5.1) thread_safe (~> 0.3, >= 0.3.4) tzinfo (~> 1.1) - addressable (2.4.0) + addressable (2.5.0) + public_suffix (~> 2.0, >= 2.0.2) arel (6.0.3) ast (2.3.0) autoprefixer-rails (6.4.0.2) @@ -70,11 +71,11 @@ GEM bootstrap-sass (3.3.7) autoprefixer-rails (>= 5.2.1) sass (>= 3.3.4) - bootstrap_form (2.5.0) + bootstrap_form (2.5.2) builder (3.2.2) - byebug (9.0.5) + byebug (9.0.6) cancancan (1.15.0) - capybara (2.8.0) + capybara (2.10.1) addressable mime-types (>= 1.16) nokogiri (>= 1.3.3) @@ -84,7 +85,7 @@ GEM capybara-email (2.5.0) capybara (~> 2.4) mail - capybara-screenshot (1.0.13) + capybara-screenshot (1.0.14) capybara (>= 1.0, < 3) launchy childprocess (0.5.9) @@ -123,10 +124,10 @@ GEM sass-rails (>= 4.0.3) concurrent-ruby (1.0.2) connection_pool (2.2.0) - coveralls (0.8.15) + coveralls (0.8.16) json (>= 1.8, < 3) simplecov (~> 0.12.0) - term-ansicolor (~> 1.3) + term-ansicolor (~> 1.3.0) thor (~> 0.19.1) tins (>= 1.6.0, < 2) csv_shaper (1.3.0) @@ -160,7 +161,7 @@ GEM faraday multi_json erubis (2.7.0) - excon (0.51.0) + excon (0.54.0) execjs (2.7.0) factory_girl (4.7.0) activesupport (>= 3.0.0) @@ -222,7 +223,7 @@ GEM httparty (0.14.0) multi_xml (>= 0.5.2) i18n (0.7.0) - i18n-tasks (0.9.5) + i18n-tasks (0.9.6) activesupport (>= 4.0.2) ast (>= 2.1.0) easy_translate (>= 0.5.0) @@ -242,12 +243,12 @@ GEM railties (>= 3.2) sprockets-rails json (1.8.3) - jwt (1.5.4) + jwt (1.5.6) kaminari (0.17.0) actionpack (>= 3.0.0) activesupport (>= 3.0.0) kgio (2.10.0) - kramdown (1.12.0) + kramdown (1.13.1) launchy (2.4.3) addressable (~> 2.3) leaflet-markercluster-rails (0.7.0) @@ -271,15 +272,14 @@ GEM mime-types-data (3.2016.0521) mimemagic (0.3.2) mini_portile2 (2.1.0) - minitest (5.9.0) + minitest (5.9.1) multi_json (1.11.3) multi_xml (0.5.5) multipart-post (2.0.0) nenv (0.3.0) - newrelic_rpm (3.16.1.320) - nokogiri (1.6.8) + newrelic_rpm (3.17.1.326) + nokogiri (1.6.8.1) mini_portile2 (~> 2.1.0) - pkg-config (~> 1.1.7) notiffany (0.1.1) nenv (~> 0.1) shellany (~> 0.0) @@ -314,13 +314,12 @@ GEM cocaine (~> 0.5.5) mime-types mimemagic (~> 0.3.0) - parser (2.3.1.2) + parser (2.3.2.0) ast (~> 2.2) - pg (0.18.4) - pkg-config (1.1.7) + pg (0.19.0) plupload-rails (1.2.1) rails (>= 3.1) - poltergeist (1.10.0) + poltergeist (1.11.0) capybara (~> 2.1) cliver (~> 0.3.1) websocket-driver (>= 0.2.0) @@ -329,9 +328,10 @@ GEM coderay (~> 1.1.0) method_source (~> 0.8.1) slop (~> 3.4) + public_suffix (2.0.4) quiet_assets (1.1.0) railties (>= 3.1, < 5.0) - rack (1.6.4) + rack (1.6.5) rack-protection (1.5.3) rack rack-test (0.6.3) @@ -370,11 +370,11 @@ GEM thor (>= 0.18.1, < 2.0) rainbow (2.1.0) raindrops (0.17.0) - rake (11.2.2) - rb-fsevent (0.9.7) + rake (11.3.0) + rb-fsevent (0.9.8) rb-inotify (0.9.7) ffi (>= 0.5.0) - redis (3.3.1) + redis (3.3.2) responders (2.3.0) railties (>= 4.2.0, < 5.1) rspec (3.5.0) @@ -385,7 +385,7 @@ GEM activemodel (>= 3.0) activesupport (>= 3.0) rspec-mocks (>= 2.99, < 4.0) - rspec-core (3.5.2) + rspec-core (3.5.4) rspec-support (~> 3.5.0) rspec-expectations (3.5.0) diff-lcs (>= 1.2.0, < 2.0) @@ -393,7 +393,7 @@ GEM rspec-mocks (3.5.0) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.5.0) - rspec-rails (3.5.1) + rspec-rails (3.5.2) actionpack (>= 3.0) activesupport (>= 3.0) railties (>= 3.0) @@ -411,7 +411,7 @@ GEM ruby-progressbar (1.8.1) ruby-units (2.0.1) ruby_dep (1.4.0) - ruby_parser (3.8.2) + ruby_parser (3.8.3) sexp_processor (~> 4.1) rubyzip (1.2.0) sass (3.4.22) @@ -453,12 +453,13 @@ GEM sprockets (>= 3.0.0) term-ansicolor (1.3.2) tins (~> 1.0) - terminal-table (1.6.0) - thor (0.19.1) + terminal-table (1.7.3) + unicode-display_width (~> 1.1.1) + thor (0.19.3) thread (0.2.2) thread_safe (0.3.5) tilt (2.0.5) - tins (1.12.0) + tins (1.13.0) tzinfo (1.2.2) thread_safe (~> 0.1) uglifier (2.7.2) @@ -540,7 +541,7 @@ DEPENDENCIES omniauth-flickr (>= 0.0.15) omniauth-twitter pg - poltergeist (~> 1.6) + poltergeist pry quiet_assets rails (~> 4.2.1) diff --git a/app/models/crop.rb b/app/models/crop.rb index 47829f463..7cf02115c 100644 --- a/app/models/crop.rb +++ b/app/models/crop.rb @@ -42,7 +42,7 @@ class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength ## Wikipedia urls are only necessary when approving a crop validates :en_wikipedia_url, format: { - with: /\Ahttps?:\/\/en\.wikipedia\.org\/wiki/, + with: /\Ahttps?:\/\/en\.wikipedia\.org\/wiki\/[[:alnum:]%_]+\z/, message: 'is not a valid English Wikipedia URL' }, if: :approved? @@ -152,13 +152,7 @@ class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength # key: sunniness (eg. 'sun') # value: count of how many times it's been used by plantings def sunniness - sunniness = Hash.new(0) - plantings.each do |p| - if !p.sunniness.blank? - sunniness[p.sunniness] += 1 - end - end - return sunniness + count_uses_of_property 'sunniness' end # crop.planted_from @@ -166,13 +160,7 @@ class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength # key: propagation method (eg. 'seed') # value: count of how many times it's been used by plantings def planted_from - planted_from = Hash.new(0) - plantings.each do |p| - if !p.planted_from.blank? - planted_from[p.planted_from] += 1 - end - end - return planted_from + count_uses_of_property 'planted_from' end # crop.popular_plant_parts @@ -273,43 +261,25 @@ class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength logger.warn("Warning: no scientific name (not even on parent crop) for #{self}") end + cropbot = Member.find_by_login_name('cropbot') + if names_to_add.size > 0 - cropbot = Member.find_by_login_name('cropbot') raise "cropbot account not found: run rake db:seed" unless cropbot - names_to_add.each do |n| - if self.scientific_names.exists?(scientific_name: n) - logger.warn("Warning: skipping duplicate scientific name #{n} for #{self}") - else - - self.scientific_names.create( - scientific_name: n, - creator_id: cropbot.id - ) - end - end + add_names_to_list(names_to_add, 'scientific', 'scientific_name') end end def add_alternate_names_from_csv(alternate_names) + cropbot = Member.find_by_login_name('cropbot') + names_to_add = [] + if !alternate_names.blank? # i.e. we actually passed something in, which isn't a given - cropbot = Member.find_by_login_name('cropbot') raise "cropbot account not found: run rake db:seed" unless cropbot names_to_add = alternate_names.split(%r{,\s*}) - - names_to_add.each do |n| - if self.alternate_names.exists?(name: n) - logger.warn("Warning: skipping duplicate alternate name #{n} for #{self}") - else - self.alternate_names.create( - name: n, - creator_id: cropbot.id - ) - end - end - + add_names_to_list(names_to_add, 'alternate', 'name') end end @@ -364,6 +334,40 @@ class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength end end + private + + def add_names_to_list(names_to_add, list_name, col_name) + names_to_add.each do |n| + if name_already_exists(list_name, col_name, n) + logger.warn("Warning: skipping duplicate #{list_name} name #{n} for #{self}") + else + create_crop_in_list(list_name, col_name, n) + end + end + end + + def create_crop_in_list(list_name, col_name, name) + cropbot = Member.find_by_login_name('cropbot') + create_hash = {} + create_hash['creator_id'] = "#{cropbot.id}" + create_hash["#{col_name}"] = name + self.send("#{list_name}_names").create(create_hash) + end + + def name_already_exists(list_name, col_name, name) + self.send("#{list_name}_names").exists?(["#{col_name} LIKE ?", name]) + end + + def count_uses_of_property(col_name) + data = Hash.new(0) + plantings.each do |p| + if !p.send("#{col_name}").blank? + data[p.send("#{col_name}")] += 1 + end + end + data + end + # Custom validations def approval_status_cannot_be_changed_again diff --git a/app/models/garden.rb b/app/models/garden.rb index 79ebe0260..ec4aea94f 100644 --- a/app/models/garden.rb +++ b/app/models/garden.rb @@ -33,7 +33,7 @@ class Garden < ActiveRecord::Base validates :name, format: { - with: /\S/ + with: /\A\w+[\w ]+\z/ }, length: { maximum: 255 } diff --git a/app/views/gardens/_thumbnail.html.haml b/app/views/gardens/_thumbnail.html.haml index 8c00d25ec..bc08be246 100644 --- a/app/views/gardens/_thumbnail.html.haml +++ b/app/views/gardens/_thumbnail.html.haml @@ -1,7 +1,7 @@ .panel.panel-success .panel-heading %h3.panel-title - = link_to "#{garden.owner.login_name}'s garden", garden.owner + = link_to "#{garden.owner.login_name}'s garden", garden - if can? :edit, garden %a.pull-right{:href => edit_garden_path(garden), :role => "button", :id => "edit_garden_glyphicon"} %span.glyphicon.glyphicon-pencil{:title => "Edit"} diff --git a/spec/models/crop_spec.rb b/spec/models/crop_spec.rb index b497b6222..675a90a7f 100644 --- a/spec/models/crop_spec.rb +++ b/spec/models/crop_spec.rb @@ -82,11 +82,43 @@ describe Crop do @crop.plantings.size.should eq 1 end - it 'validates en_wikipedia_url' do - @crop = FactoryGirl.build(:tomato, en_wikipedia_url: 'this is not valid') - @crop.should_not be_valid - @crop = FactoryGirl.build(:tomato, en_wikipedia_url: 'http://en.wikipedia.org/wiki/SomePage') - @crop.should be_valid + context "wikipedia url" do + subject { FactoryGirl.build(:tomato, en_wikipedia_url: wikipedia_url) } + + context 'not a url' do + let(:wikipedia_url) { 'this is not valid' } + it { expect(subject).not_to be_valid } + end + + context 'http url' do + let(:wikipedia_url) { 'http://en.wikipedia.org/wiki/SomePage' } + it { expect(subject).to be_valid } + end + + context 'with ssl' do + let(:wikipedia_url) { 'https://en.wikipedia.org/wiki/SomePage' } + it { expect(subject).to be_valid } + end + + context 'with utf8 macrons' do + let(:wikipedia_url) { 'https://en.wikipedia.org/wiki/Māori' } + it { expect(subject).to be_valid } + end + + context 'urlencoded' do + let(:wikipedia_url) { 'https://en.wikipedia.org/wiki/M%C4%81ori' } + it { expect(subject).to be_valid } + end + + context 'with new lines in url' do + let(:wikipedia_url) { 'http://en.wikipedia.org/wiki/SomePage\n\nBrendaRocks' } + it { expect(subject).not_to be_valid } + end + + context "with script tags in url" do + let(:wikipedia_url) { 'http://en.wikipedia.org/wiki/SomePage' } + it { expect(subject).not_to be_valid } + end end context 'varieties' do diff --git a/spec/models/garden_spec.rb b/spec/models/garden_spec.rb index b56ac2056..4ffb1bfc5 100644 --- a/spec/models/garden_spec.rb +++ b/spec/models/garden_spec.rb @@ -22,11 +22,26 @@ describe Garden do garden.should_not be_valid end + it "allows numbers" do + garden = FactoryGirl.build(:garden, name: "100 vines of 2 kamo-kamo") + garden.should_not be_valid + end + + it "allows macrons" do + garden = FactoryGirl.build(:garden, name: "Kūmara and pūha patch") + garden.should_not be_valid + end + it "doesn't allow a name with only spaces" do garden = FactoryGirl.build(:garden, name: " ") garden.should_not be_valid end + it "doesn't allow a new lines in garden names" do + garden = FactoryGirl.build(:garden, name: "My garden\nI am a 1337 hacker") + garden.should_not be_valid + end + it "should have an owner" do garden.owner.should be_an_instance_of Member end diff --git a/spec/views/seeds/index.rss.haml_spec.rb b/spec/views/seeds/index.rss.haml_spec.rb index 2de2fff1f..a90394084 100644 --- a/spec/views/seeds/index.rss.haml_spec.rb +++ b/spec/views/seeds/index.rss.haml_spec.rb @@ -38,7 +38,7 @@ describe 'seeds/index.rss.haml' do end it 'shows the plant_before date' do - rendered.should have_content "Plant before: #{@seed.plant_before.to_s}" + rendered.should have_content "Plant before: #{@seed.plant_before}" end it 'mentions that one seed is tradable' do